malliaridis commented on code in PR #2712:
URL: https://github.com/apache/solr/pull/2712#discussion_r1799446120
##########
solr/packaging/test/test_status.bats:
##########
@@ -37,18 +37,27 @@ teardown() {
solr stop
run solr status
assert_output --partial "No Solr nodes are running."
+}
+@test "status with --solr-url from user" {
+ solr start
+ run solr status --solr-url http://localhost:${SOLR_PORT}
+ assert_output --partial "Found 1 Solr nodes:"
+ assert_output --partial "running on port ${SOLR_PORT}"
+ solr stop
}
-@test "status shell script ignores passed in --solr-url cli parameter from
user" {
+@test "status with --port from user" {
solr start
- run solr status --solr-url http://localhost:9999
+ run solr status --port ${SOLR_PORT}
assert_output --partial "Found 1 Solr nodes:"
assert_output --partial "running on port ${SOLR_PORT}"
+ solr stop
}
Review Comment:
For the mentioned scenario, this would be the negative test that should fail.
```suggestion
@test "status with invalid --solr-url from user" {
solr start
run solr status --solr-url http://invalidhost:${SOLR_PORT}
assert_output --partial "No Solr nodes are running."
solr stop
}
```
##########
solr/core/src/java/org/apache/solr/cli/StatusTool.java:
##########
@@ -65,37 +75,132 @@ public String getName() {
.desc("Wait up to the specified number of seconds to see Solr
running.")
.build();
+ public static final Option OPTION_PORT =
+ Option.builder("p")
+ .longOpt("port")
+ .argName("PORT")
+ .required(false)
+ .hasArg()
+ .desc("Port on localhost to check status for")
+ .build();
+
+ public static final Option OPTION_SHORT =
+ Option.builder()
+ .longOpt("short")
+ .argName("SHORT")
+ .required(false)
+ .desc("Short format. Prints one URL per line for running instances")
+ .build();
+
@Override
public List<Option> getOptions() {
- return List.of(
- // The solr-url option is not exposed to the end user, and is
- // created by the bin/solr script and passed into this command
directly,
- // therefore we don't use the SolrCLI.OPTION_SOLRURL.
- Option.builder()
- .argName("URL")
- .longOpt("solr-url")
- .hasArg()
- .required(false)
- .desc("Property set by calling scripts, not meant for user
configuration.")
- .build(),
- OPTION_MAXWAITSECS);
+ return List.of(OPTION_SOLRURL, OPTION_MAXWAITSECS, OPTION_PORT,
OPTION_SHORT);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
- // Override the default help behaviour to put out a customized message
that only list user
- // settable Options.
- if ((cli.getOptions().length == 0 && cli.getArgs().length == 0)
- || cli.hasOption("h")
- || cli.hasOption("help")) {
- final Options options = new Options();
- options.addOption(OPTION_MAXWAITSECS);
- new HelpFormatter().printHelp("status", options);
- return;
+ String solrUrl = cli.getOptionValue(OPTION_SOLRURL);
+ boolean urlProvided = solrUrl != null;
+ Integer port =
+ cli.hasOption(OPTION_PORT) ?
Integer.parseInt(cli.getOptionValue(OPTION_PORT)) : null;
+ boolean shortFormat = cli.hasOption(OPTION_SHORT);
+
+ if (port != null && solrUrl != null) {
+ throw new IllegalArgumentException("Only one of port or url can be
specified");
+ }
+
+ if (port != null) {
+ Optional<SolrProcess> proc = processMgr.processForPort(port);
+ if (proc.isEmpty()) {
+ CLIO.err("Could not find a running Solr on port " + port);
+ System.exit(1);
+ } else {
+ solrUrl = proc.get().getLocalUrl();
+ }
+ }
+
+ if (solrUrl == null) {
+ if (!shortFormat) {
+ CLIO.out("Looking for running processes...");
+ }
+ Collection<SolrProcess> procs = processMgr.getAllRunning();
+ if (!shortFormat) {
+ CLIO.out(String.format(Locale.ROOT, "\nFound %s Solr nodes: ",
procs.size()));
+ }
+ if (!procs.isEmpty()) {
+ for (SolrProcess process : procs) {
+ if (shortFormat) {
+ CLIO.out(process.getLocalUrl());
+ } else {
+ printProcessStatus(process, cli);
+ }
+ }
+ } else {
+ if (!shortFormat) {
+ CLIO.out("\nNo Solr nodes are running.\n");
+ }
+ }
+ } else {
+ // We have a solrUrl
+ int urlPort = portFromUrl(solrUrl);
+ if (shortFormat) {
+ if (processMgr.isRunningWithPort(urlPort)) {
+ CLIO.out(solrUrl);
+ }
+ } else {
+ Optional<SolrProcess> process = processMgr.processForPort(urlPort);
+
+ if (process.isEmpty() && urlProvided) {
+ // Try harder to get status if the URL was provided, even if Solr is
not a separate OS
+ // process
+ try {
+ getStatus(solrUrl, cli.getOptionValue(OPTION_CREDENTIALS));
+ process = Optional.of(new SolrProcess(-1, urlPort,
solrUrl.contains("https")));
+ } catch (Exception e) {
+ /* Ignore */
+ }
+ }
+
+ if (process.isPresent()) {
+ CLIO.out(String.format(Locale.ROOT, "\nFound %s Solr nodes: ", 1));
+ printProcessStatus(process.get(), cli);
+ } else {
+ CLIO.out("\nNo Solr running on port " + urlPort + ".");
+ }
+ }
Review Comment:
In a scenario where the developer is troubleshooting a production
environment with the same CLI, but is also running Solr on the same port in the
local environment, would this return the status of the local environment, or
the production environment?
I believe that if the user is explicit with a solr-url we should always use
this instead of the process manager.
##########
solr/core/src/java/org/apache/solr/cli/StatusTool.java:
##########
@@ -65,37 +75,132 @@ public String getName() {
.desc("Wait up to the specified number of seconds to see Solr
running.")
.build();
+ public static final Option OPTION_PORT =
+ Option.builder("p")
+ .longOpt("port")
+ .argName("PORT")
+ .required(false)
+ .hasArg()
+ .desc("Port on localhost to check status for")
+ .build();
+
+ public static final Option OPTION_SHORT =
+ Option.builder()
+ .longOpt("short")
+ .argName("SHORT")
+ .required(false)
+ .desc("Short format. Prints one URL per line for running instances")
+ .build();
+
@Override
public List<Option> getOptions() {
- return List.of(
- // The solr-url option is not exposed to the end user, and is
- // created by the bin/solr script and passed into this command
directly,
- // therefore we don't use the SolrCLI.OPTION_SOLRURL.
- Option.builder()
- .argName("URL")
- .longOpt("solr-url")
- .hasArg()
- .required(false)
- .desc("Property set by calling scripts, not meant for user
configuration.")
- .build(),
- OPTION_MAXWAITSECS);
+ return List.of(OPTION_SOLRURL, OPTION_MAXWAITSECS, OPTION_PORT,
OPTION_SHORT);
}
@Override
public void runImpl(CommandLine cli) throws Exception {
- // Override the default help behaviour to put out a customized message
that only list user
- // settable Options.
- if ((cli.getOptions().length == 0 && cli.getArgs().length == 0)
- || cli.hasOption("h")
- || cli.hasOption("help")) {
- final Options options = new Options();
- options.addOption(OPTION_MAXWAITSECS);
- new HelpFormatter().printHelp("status", options);
- return;
+ String solrUrl = cli.getOptionValue(OPTION_SOLRURL);
+ boolean urlProvided = solrUrl != null;
+ Integer port =
+ cli.hasOption(OPTION_PORT) ?
Integer.parseInt(cli.getOptionValue(OPTION_PORT)) : null;
+ boolean shortFormat = cli.hasOption(OPTION_SHORT);
+
+ if (port != null && solrUrl != null) {
+ throw new IllegalArgumentException("Only one of port or url can be
specified");
+ }
+
+ if (port != null) {
+ Optional<SolrProcess> proc = processMgr.processForPort(port);
+ if (proc.isEmpty()) {
+ CLIO.err("Could not find a running Solr on port " + port);
+ System.exit(1);
+ } else {
+ solrUrl = proc.get().getLocalUrl();
+ }
+ }
+
+ if (solrUrl == null) {
+ if (!shortFormat) {
+ CLIO.out("Looking for running processes...");
+ }
+ Collection<SolrProcess> procs = processMgr.getAllRunning();
+ if (!shortFormat) {
+ CLIO.out(String.format(Locale.ROOT, "\nFound %s Solr nodes: ",
procs.size()));
+ }
+ if (!procs.isEmpty()) {
+ for (SolrProcess process : procs) {
+ if (shortFormat) {
+ CLIO.out(process.getLocalUrl());
+ } else {
+ printProcessStatus(process, cli);
+ }
+ }
+ } else {
+ if (!shortFormat) {
+ CLIO.out("\nNo Solr nodes are running.\n");
+ }
+ }
+ } else {
+ // We have a solrUrl
+ int urlPort = portFromUrl(solrUrl);
+ if (shortFormat) {
+ if (processMgr.isRunningWithPort(urlPort)) {
+ CLIO.out(solrUrl);
+ }
+ } else {
+ Optional<SolrProcess> process = processMgr.processForPort(urlPort);
+
+ if (process.isEmpty() && urlProvided) {
+ // Try harder to get status if the URL was provided, even if Solr is
not a separate OS
+ // process
+ try {
+ getStatus(solrUrl, cli.getOptionValue(OPTION_CREDENTIALS));
+ process = Optional.of(new SolrProcess(-1, urlPort,
solrUrl.contains("https")));
+ } catch (Exception e) {
+ /* Ignore */
+ }
+ }
+
+ if (process.isPresent()) {
+ CLIO.out(String.format(Locale.ROOT, "\nFound %s Solr nodes: ", 1));
+ printProcessStatus(process.get(), cli);
+ } else {
+ CLIO.out("\nNo Solr running on port " + urlPort + ".");
+ }
+ }
}
+ }
+ private void printProcessStatus(SolrProcess process, CommandLine cli) throws
Exception {
int maxWaitSecs = Integer.parseInt(cli.getOptionValue("max-wait-secs",
"0"));
- String solrUrl = SolrCLI.normalizeSolrUrl(cli);
+ boolean shortFormat = cli.hasOption(OPTION_SHORT);
+ String pidUrl = process.getLocalUrl();
+ CLIO.out(
+ String.format(
+ Locale.ROOT,
+ "\nSolr process %s running on port %s",
+ process.getPid(),
+ process.getPort()));
+ if (shortFormat) {
+ CLIO.out(pidUrl);
+ } else {
+ statusFromRunningSolr(pidUrl, cli, maxWaitSecs);
+ }
+ CLIO.out("");
+ }
+
+ private Integer portFromUrl(String solrUrl) {
+ try {
+ return new URI(solrUrl).getPort();
Review Comment:
For a URL like `https://domain.localhost/` this would return `-1`, instead
of `443` that a user might expect for the `https` scheme, or `8983` (default
Solr port) . Is this fine?
##########
gradle/testing/randomization/policies/solr-tests.policy:
##########
@@ -250,6 +252,8 @@ grant {
// expanded to a wildcard if set, allows all networking everywhere
permission java.net.SocketPermission "${solr.internal.network.permission}",
"accept,listen,connect,resolve";
+
+ permission java.io.FilePermission "${java.home}${/}-", "execute";
Review Comment:
This line seems related to the errors I get from running `gradlew check` on
Windows. Do I have to set a specific variable in the `gradle.properties` or
somewhere else? The error is:
```
ERROR: access denied ("java.io.FilePermission" "<<ALL FILES>>" "execute")
```
11 tests fail with this error (only on Windows).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
