Fix bin/solr script for bugs in -p, -V, and healthcheck tool

* Re-introduce support for -p and -port * Now supporting the deprecated -V (versus -v) * fix a bug in HealthcheckTool itself and how we pass parameters in --------- Co-authored-by: Christos Malliaridis <[email protected]>
apache · Sep 28, 2024 · 21a6b4c · 21a6b4c
1 parent 19cb424
commit 21a6b4c
Show file tree

Hide file tree

Showing 11 changed files with 146 additions and 36 deletions.
diff --git a/solr/bin/solr b/solr/bin/solr
@@ -946,7 +946,7 @@ if [ "$SCRIPT_CMD" == "healthcheck" ]; then
     exit 1
   fi
 
-  run_tool healthcheck -zkHost "$ZK_HOST" -collection "$HEALTHCHECK_COLLECTION" $VERBOSE
+  run_tool healthcheck -z "$ZK_HOST" -c "$HEALTHCHECK_COLLECTION" $VERBOSE
 
   exit $?
 fi

diff --git a/solr/bin/solr.cmd b/solr/bin/solr.cmd
@@ -1548,7 +1548,7 @@ echo ZK_HOST: !ZK_HOST!
 "%JAVA%" %SOLR_SSL_OPTS% %AUTHC_OPTS% %SOLR_ZK_CREDS_AND_ACLS% -Dsolr.install.dir="%SOLR_TIP%" ^
   -Dlog4j.configurationFile="file:///%DEFAULT_SERVER_DIR%\resources\log4j2-console.xml" ^
   -classpath "%DEFAULT_SERVER_DIR%\solr-webapp\webapp\WEB-INF\lib\*;%DEFAULT_SERVER_DIR%\lib\ext\*" ^
-  org.apache.solr.cli.SolrCLI healthcheck -collection !HEALTHCHECK_COLLECTION! -zkHost !ZK_HOST! %HEALTHCHECK_VERBOSE%
+  org.apache.solr.cli.SolrCLI healthcheck -c !HEALTHCHECK_COLLECTION! -z !ZK_HOST! %HEALTHCHECK_VERBOSE%
 goto done
 
 :run_solrcli

diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java
@@ -45,6 +45,7 @@
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.EnvUtils;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.core.ConfigSetService;
 import org.noggit.CharArr;
@@ -148,16 +149,36 @@ public List<Option> getOptions() {
             .required(false)
             .desc("Configuration name; default is the collection name.")
             .build(),
+        Option.builder("p")
+            .longOpt("port")
+            .deprecated(
+                DeprecatedAttributes.builder()
+                    .setForRemoval(true)
+                    .setSince("9.7")
+                    .setDescription("Use --solr-url instead")
+                    .get())
+            .argName("PORT")
+            .hasArg()
+            .required(false)
+            .desc("Port of a local Solr instance where you want to create the new core.")
+            .build(),
         SolrCLI.OPTION_SOLRURL,
         SolrCLI.OPTION_SOLRURL_DEPRECATED,
         SolrCLI.OPTION_ZKHOST,
-        SolrCLI.OPTION_ZKHOST_DEPRECATED);
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_VERBOSE_DEPRECATED);
   }
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
 
+    // If a port is provided, inject it into the environment variables so
+    // that {@link SolrCLI#getDefaultSolrUrl()} can use it instead of the default 8983.
+    if (cli.hasOption("p")) {
+      EnvUtils.setProperty("jetty.port", cli.getOptionValue("p"));
+    }
+
     try (var solrClient = SolrCLI.getSolrClient(cli)) {
       if (SolrCLI.isCloudMode(solrClient)) {
         createCollection(cli);
@@ -224,12 +245,12 @@ protected void createCore(CommandLine cli, SolrClient solrClient) throws Excepti
 
     try {
       CoreAdminResponse res = CoreAdminRequest.createCore(coreName, coreName, solrClient);
-      if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+      if (isVerbose()) {
         echo(res.jsonStr());
         echo("\n");
-      } else {
-        echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
       }
+      echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
+
     } catch (Exception e) {
       /* create-core failed, cleanup the copied configset before propagating the error. */
       PathUtils.deleteDirectory(coreInstanceDir);
@@ -349,25 +370,25 @@ protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine cli
           "Failed to create collection '" + collectionName + "' due to: " + sse.getMessage());
     }
 
-    if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+    if (isVerbose()) {
       // pretty-print the response to stdout
       CharArr arr = new CharArr();
       new JSONWriter(arr, 2).write(response.asMap());
       echo(arr.toString());
-    } else {
-      String endMessage =
-          String.format(
-              Locale.ROOT,
-              "Created collection '%s' with %d shard(s), %d replica(s)",
-              collectionName,
-              numShards,
-              replicationFactor);
-      if (confName != null && !confName.trim().isEmpty()) {
-        endMessage += String.format(Locale.ROOT, " with config-set '%s'", confName);
-      }
+    }
 
-      echo(endMessage);
+    String endMessage =
+        String.format(
+            Locale.ROOT,
+            "Created collection '%s' with %d shard(s), %d replica(s)",
+            collectionName,
+            numShards,
+            replicationFactor);
+    if (confName != null && !confName.trim().isEmpty()) {
+      endMessage += String.format(Locale.ROOT, " with config-set '%s'", confName);
     }
+
+    echo(endMessage);
   }
 
   private Path getFullConfDir(Path solrInstallDir, Path confDirName) {

diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
@@ -22,6 +22,7 @@
 import java.io.PrintStream;
 import java.lang.invoke.MethodHandles;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -40,6 +41,7 @@
 import org.apache.solr.client.solrj.request.CoreAdminRequest;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
 import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.EnvUtils;
 import org.apache.solr.common.util.NamedList;
 import org.noggit.CharArr;
 import org.noggit.JSONWriter;
@@ -122,16 +124,36 @@ public List<Option> getOptions() {
             .desc(
                 "Skip safety checks when deleting the configuration directory used by a collection.")
             .build(),
+        Option.builder("p")
+            .longOpt("port")
+            .deprecated(
+                DeprecatedAttributes.builder()
+                    .setForRemoval(true)
+                    .setSince("9.7")
+                    .setDescription("Use --solr-url instead")
+                    .get())
+            .argName("PORT")
+            .hasArg()
+            .required(false)
+            .desc("Port of a local Solr instance where you want to create the new core.")
+            .build(),
         SolrCLI.OPTION_SOLRURL,
         SolrCLI.OPTION_SOLRURL_DEPRECATED,
         SolrCLI.OPTION_ZKHOST,
-        SolrCLI.OPTION_ZKHOST_DEPRECATED);
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_VERBOSE_DEPRECATED);
   }
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
 
+    // If a port is provided, inject it into the environment variables so
+    // that {@link SolrCLI#getDefaultSolrUrl()} can use it instead of the default 8983.
+    if (cli.hasOption("p")) {
+      EnvUtils.setProperty("jetty.port", cli.getOptionValue("p"));
+    }
+
     try (var solrClient = SolrCLI.getSolrClient(cli)) {
       Map<String, Object> systemInfo =
           solrClient
@@ -248,21 +270,22 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli
       }
     }
 
-    if (response != null) {
-      // pretty-print the response to stdout
-      CharArr arr = new CharArr();
-      new JSONWriter(arr, 2).write(response.asMap());
-      echo(arr.toString());
-      echo("\n");
+    if (isVerbose()) {
+      if (response != null) {
+        // pretty-print the response to stdout
+        CharArr arr = new CharArr();
+        new JSONWriter(arr, 2).write(response.asMap());
+        echo(arr.toString());
+      }
     }
 
-    echo("Deleted collection '" + collectionName + "' using CollectionAdminRequest");
+    echo(String.format(Locale.ROOT, "\nDeleted collection '%s'", collectionName));
   }
 
   protected void deleteCore(CommandLine cli, SolrClient solrClient) throws Exception {
     String coreName = cli.getOptionValue(NAME);
 
-    echo("\nDeleting core '" + coreName + "' using CoreAdminRequest\n");
+    echoIfVerbose("\nDeleting core '" + coreName + "'", cli);
 
     NamedList<Object> response;
     try {
@@ -281,5 +304,6 @@ protected void deleteCore(CommandLine cli, SolrClient solrClient) throws Excepti
       echoIfVerbose((String) response.get("response"), cli);
       echoIfVerbose("\n", cli);
     }
+    echo(String.format(Locale.ROOT, "\nDeleted core '%s'", coreName));
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
@@ -28,12 +28,14 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Option;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.impl.CloudHttp2SolrClient;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.Http2SolrClient;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.cloud.ClusterState;
@@ -92,8 +94,8 @@ public void runImpl(CommandLine cli) throws Exception {
       CLIO.err("Healthcheck tool only works in Solr Cloud mode.");
       System.exit(1);
     }
+    echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
     try (CloudHttp2SolrClient cloudSolrClient = SolrCLI.getCloudHttp2SolrClient(zkHost)) {
-      echoIfVerbose("\nConnecting to ZooKeeper at " + zkHost + " ...", cli);
       cloudSolrClient.connect();
       runCloudTool(cloudSolrClient, cli);
     }
@@ -170,7 +172,15 @@ protected void runCloudTool(CloudSolrClient cloudSolrClient, CommandLine cli) th
           q = new SolrQuery("*:*");
           q.setRows(0);
           q.set(DISTRIB, "false");
-          try (var solrClientForCollection = SolrCLI.getSolrClient(coreUrl)) {
+
+          // SolrCLI.getSolrClient converts the coreUrl back into a root Solr url, but we need to
+          // talk to specific core so manually creating the client.
+          // try (var solrClientForCollection = SolrCLI.getSolrClient(coreUrl)) {
+          try (var solrClientForCollection =
+              new Http2SolrClient.Builder(coreUrl)
+                  .withMaxConnectionsPerHost(32)
+                  .withKeyStoreReloadInterval(-1, TimeUnit.SECONDS)
+                  .build()) {
             qr = solrClientForCollection.query(q);
             numDocs = qr.getResults().getNumFound();
             try (var solrClient = SolrCLI.getSolrClient(replicaCoreProps.getBaseUrl())) {

diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
@@ -150,6 +150,17 @@ public class SolrCLI implements CLIO {
           .required(false)
           .desc("Enable verbose command output.")
           .build();
+  public static final Option OPTION_VERBOSE_DEPRECATED =
+      Option.builder("V")
+          .deprecated(
+              DeprecatedAttributes.builder()
+                  .setForRemoval(true)
+                  .setSince("9.7")
+                  .setDescription("Use -v instead")
+                  .get())
+          .required(false)
+          .desc("Enable verbose command output.")
+          .build();
   public static final Option OPTION_HELP =
       Option.builder("h").longOpt("help").required(false).desc("Print this message.").build();
 
@@ -294,7 +305,8 @@ protected static void checkSslStoreSysProp(String solrInstallDir, String key) {
   }
 
   public static void raiseLogLevelUnlessVerbose(CommandLine cli) {
-    if (!cli.hasOption(OPTION_VERBOSE.getOpt())) {
+    if (!(cli.hasOption(OPTION_VERBOSE.getOpt())
+        || cli.hasOption(OPTION_VERBOSE_DEPRECATED.getOpt()))) {
       StartupLoggingUtils.changeLogLevel("WARN");
     }
   }
@@ -519,6 +531,10 @@ public static boolean exceptionIsAuthRelated(Exception exc) {
    * <p>Note that from Solr 10.0 Solr will use / as base URL to support /api and other contexts, so
    * handling of hostContext differs between Solr 9.x and 10.x.
    *
+   * <p>Note that this should not be used with individual core specific urls such as
+   * "http://127.0.0.1:51428/solr/bob_shard1_replica_n1/" as it removes the core name by converting
+   * the url to "http://127.0.0.1:51428/" before creating the client.
+   *
    * @param solrUrl the base URL of Solr
    * @return a SolrClient initialized with correct hostContext, default '/solr'
    */

diff --git a/solr/core/src/java/org/apache/solr/cli/ToolBase.java b/solr/core/src/java/org/apache/solr/cli/ToolBase.java
@@ -34,18 +34,25 @@ protected ToolBase(PrintStream stdout) {
   }
 
   protected void echoIfVerbose(final String msg, CommandLine cli) {
-    if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+    if (verbose) {
       echo(msg);
     }
   }
 
+  /** Is this tool being run in a verbose mode? */
+  protected boolean isVerbose() {
+    return verbose;
+  }
+
   protected void echo(final String msg) {
     stdout.println(msg);
   }
 
   @Override
   public int runTool(CommandLine cli) throws Exception {
-    verbose = cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt());
+    verbose =
+        cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())
+            || cli.hasOption(SolrCLI.OPTION_VERBOSE_DEPRECATED);
 
     int toolExitStatus = 0;
     try {

diff --git a/solr/packaging/test/test_create.bats b/solr/packaging/test/test_create.bats
@@ -34,8 +34,31 @@ teardown() {
   assert_output --partial "Created new core 'COLL_NAME'"
 }
 
+@test "create_core works" {
+  run solr start
+  run solr create_core -c COLL_NAME
+  assert_output --partial "Created new core 'COLL_NAME'"
+}
+
 @test "create for cloud mode" {
   run solr start -c
   run solr create -c COLL_NAME
   assert_output --partial "Created collection 'COLL_NAME'"
 }
+
+@test "ensure -p port parameter is supported" {
+  solr start -p ${SOLR2_PORT}
+  solr assert --not-started http://localhost:${SOLR_PORT} --timeout 5000
+  solr assert --started http://localhost:${SOLR2_PORT} --timeout 5000
+
+  run solr create -c COLL_NAME -p ${SOLR2_PORT}
+  assert_output --partial "Created new core 'COLL_NAME'"
+}
+
+@test "ensure -V verbose parameter is supported" {
+  solr start 
+  solr assert --started http://localhost:${SOLR_PORT} --timeout 5000
+
+  run solr create -c COLL_NAME -V
+  assert_output --partial "Creating new core 'COLL_NAME' using CoreAdminRequest"
+}
diff --git a/solr/packaging/test/test_delete_collection.bats b/solr/packaging/test/test_delete_collection.bats
@@ -77,3 +77,11 @@ teardown() {
   solr delete -c "COLL_NAME" --delete-config false
   assert config_exists "COLL_NAME"
 }
+
+@test "ensure -p port parameter is supported" {  
+  run solr create -c COLL_NAME -p ${SOLR_PORT}
+  assert_output --partial "Created collection 'COLL_NAME'"
+
+  run solr delete -c COLL_NAME --delete-config false -p ${SOLR_PORT} -V
+  assert_output --partial "Deleted collection 'COLL_NAME'"
+}
diff --git a/solr/packaging/test/test_healthcheck.bats b/solr/packaging/test/test_healthcheck.bats
@@ -29,8 +29,9 @@ teardown() {
 }
 
 @test "healthcheck on cloud solr" {
-  solr start -c -e films
-  run solr healthcheck -c films
+  solr start -c -e films -p ${SOLR_PORT}
+  run solr healthcheck -c films -zkhost localhost:${ZK_PORT} -V
   refute_output --partial 'error'
+  assert_output --partial '"collection":"films"'
 
 }
diff --git a/solr/packaging/test/test_zk.bats b/solr/packaging/test/test_zk.bats
@@ -89,7 +89,7 @@ teardown() {
   touch myfile2.txt
   run solr zk cp myfile2.txt zk:myfile2.txt -z localhost:${ZK_PORT}
   sleep 1
-  run solr zk ls / -z localhost:${ZK_PORT}
+  run solr zk ls / -z localhost:${ZK_PORT} -V
   assert_output --partial "myfile2.txt"
 
   touch myfile3.txt