dsmiley commented on code in PR #1785:
URL: https://github.com/apache/solr/pull/1785#discussion_r1272681973


##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -43,24 +68,295 @@ public String getName() {
 
   @Override
   public List<Option> getOptions() {
-    return CreateCollectionTool.CREATE_COLLECTION_OPTIONS;
+    return List.of(
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_SOLRURL,
+        Option.builder("c")
+            .longOpt("name")
+            .argName("NAME")
+            .hasArg()
+            .required(true)
+            .desc("Name of collection or core to create.")
+            .build(),
+        Option.builder("s")
+            .longOpt("shards")
+            .argName("#")
+            .hasArg()
+            .required(false)
+            .desc("Number of shards; default is 1.")
+            .build(),
+        Option.builder("rf")
+            .longOpt("replicationFactor")
+            .argName("#")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Number of copies of each document across the collection 
(replicas per shard); default is 1.")
+            .build(),
+        Option.builder("d")
+            .longOpt("confdir")
+            .argName("NAME")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Configuration directory to copy when creating the new 
collection; default is "
+                    + SolrCLI.DEFAULT_CONFIG_SET
+                    + '.')
+            .build(),
+        Option.builder("n")
+            .longOpt("confname")
+            .argName("NAME")
+            .hasArg()
+            .required(false)
+            .desc("Configuration name; default is the collection name.")
+            .build(),
+        SolrCLI.OPTION_VERBOSE);
   }
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
     String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
 
-    ToolBase tool;
     try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
-      NamedList<Object> systemInfo =
-          solrClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, 
SYSTEM_INFO_PATH));
-      if ("solrcloud".equals(systemInfo.get("mode"))) {
-        tool = new CreateCollectionTool(stdout);
+      if (SolrCLI.isCloudMode(solrClient)) {
+        createCollection(cli);
+      } else {
+        createCore(cli, solrClient);
+      }
+    }
+  }
+
+  protected void createCore(CommandLine cli, SolrClient solrClient) throws 
Exception {
+    String coreName = cli.getOptionValue("name");
+    String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
+
+    final String solrInstallDir = System.getProperty("solr.install.dir");
+    final String confDirName = cli.getOptionValue("confdir", 
SolrCLI.DEFAULT_CONFIG_SET);
+
+    // we allow them to pass a directory instead of a configset name
+    Path configsetDir = Paths.get(confDirName);
+    Path solrInstallDirPath = Paths.get(solrInstallDir);
+
+    if (!Files.isDirectory(configsetDir)) {
+      ensureConfDirExists(configsetDir, solrInstallDirPath);
+    }
+    printDefaultConfigsetWarningIfNecessary(cli);
+
+    String coreRootDirectory; // usually same as solr home, but not always
+
+    Map<String, Object> systemInfo =
+        solrClient
+            .request(new GenericSolrRequest(SolrRequest.METHOD.GET, 
CommonParams.SYSTEM_INFO_PATH))
+            .asMap();
+
+    // convert raw JSON into user-friendly output
+    coreRootDirectory = (String) systemInfo.get("core_root");
+
+    if (SolrCLI.safeCheckCoreExists(solrUrl, coreName)) {
+      throw new IllegalArgumentException(
+          "\nCore '"
+              + coreName
+              + "' already exists!\nChecked core existence using Core API 
command");
+    }
+
+    Path coreInstanceDir = Paths.get(coreRootDirectory, coreName);
+    Path confDir = getFullConfDir(configsetDir, 
solrInstallDirPath).resolve("conf");
+    if (!Files.isDirectory(coreInstanceDir)) {
+      Files.createDirectories(coreInstanceDir);
+      if (!Files.isDirectory(coreInstanceDir)) {
+        throw new IOException(
+            "Failed to create new core instance directory: " + 
coreInstanceDir.toAbsolutePath());
+      }
+
+      FileUtils.copyDirectoryToDirectory(confDir.toFile(), 
coreInstanceDir.toFile());
+
+      echoIfVerbose(
+          "\nCopying configuration to new core instance directory:\n"
+              + coreInstanceDir.toAbsolutePath(),
+          cli);
+    }
+
+    echoIfVerbose("\nCreating new core '" + coreName + "' using 
CoreAdminRequest", cli);
+
+    try {
+      CoreAdminResponse res = CoreAdminRequest.createCore(coreName, coreName, 
solrClient);
+      if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+        echo(res.jsonStr());
+        echo("\n");
       } else {
-        tool = new CreateCoreTool(stdout);
+        echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
       }
-      tool.runImpl(cli);
+    } catch (Exception e) {
+      /* create-core failed, cleanup the copied configset before propagating 
the error. */
+      PathUtils.deleteDirectory(coreInstanceDir);
+      throw e;
+    }
+  }
+
+  protected void createCollection(CommandLine cli) throws Exception {
+    String zkHost = SolrCLI.getZkHost(cli);
+    try (CloudSolrClient cloudSolrClient =
+        new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), 
Optional.empty())
+            .withInternalClientBuilder(
+                new Http2SolrClient.Builder()
+                    .withIdleTimeout(30, TimeUnit.SECONDS)
+                    .withConnectionTimeout(15, TimeUnit.SECONDS))
+            .build()) {
+      echoIfVerbose("Connecting to ZooKeeper at " + zkHost, cli);
+      cloudSolrClient.connect();
+      createCollection(cloudSolrClient, cli);
+    }
+  }
+
+  protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine 
cli)
+      throws Exception {
+
+    String collectionName = cli.getOptionValue("name");
+    final String solrInstallDir = System.getProperty("solr.install.dir");
+    String confName = cli.getOptionValue("confname");
+    String confDir = cli.getOptionValue("confdir", "_default");
+    ensureConfDirExists(Paths.get(confDir), Paths.get(solrInstallDir));
+    printDefaultConfigsetWarningIfNecessary(cli);
+
+    Set<String> liveNodes = cloudSolrClient.getClusterState().getLiveNodes();
+    if (liveNodes.isEmpty())
+      throw new IllegalStateException(
+          "No live nodes found! Cannot create a collection until "
+              + "there is at least 1 live node in the cluster.");
+
+    String solrUrl = cli.getOptionValue("solrUrl");
+    if (solrUrl == null) {
+      String firstLiveNode = liveNodes.iterator().next();
+      solrUrl = 
ZkStateReader.from(cloudSolrClient).getBaseUrlForNodeName(firstLiveNode);
+    }
+
+    // build a URL to create the collection
+    int numShards = Integer.parseInt(cli.getOptionValue("shards", 
String.valueOf(1)));
+    int replicationFactor =
+        Integer.parseInt(cli.getOptionValue("replicationFactor", 
String.valueOf(1)));
+
+    final String configsetsDir = solrInstallDir + "/server/solr/configsets";

Review Comment:
   Seeing string concatenation here vs. working with Path API.  Also, this 
looks redundant; with logic inside `getFullConfDir` suggesting that that method 
should maybe take configsetsDir instead?



##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -43,24 +68,295 @@ public String getName() {
 
   @Override
   public List<Option> getOptions() {
-    return CreateCollectionTool.CREATE_COLLECTION_OPTIONS;
+    return List.of(
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_SOLRURL,
+        Option.builder("c")
+            .longOpt("name")
+            .argName("NAME")
+            .hasArg()
+            .required(true)
+            .desc("Name of collection or core to create.")
+            .build(),
+        Option.builder("s")
+            .longOpt("shards")
+            .argName("#")
+            .hasArg()
+            .required(false)
+            .desc("Number of shards; default is 1.")
+            .build(),
+        Option.builder("rf")
+            .longOpt("replicationFactor")
+            .argName("#")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Number of copies of each document across the collection 
(replicas per shard); default is 1.")
+            .build(),
+        Option.builder("d")
+            .longOpt("confdir")
+            .argName("NAME")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Configuration directory to copy when creating the new 
collection; default is "
+                    + SolrCLI.DEFAULT_CONFIG_SET
+                    + '.')
+            .build(),
+        Option.builder("n")
+            .longOpt("confname")
+            .argName("NAME")
+            .hasArg()
+            .required(false)
+            .desc("Configuration name; default is the collection name.")
+            .build(),
+        SolrCLI.OPTION_VERBOSE);
   }
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
     String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
 
-    ToolBase tool;
     try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
-      NamedList<Object> systemInfo =
-          solrClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, 
SYSTEM_INFO_PATH));
-      if ("solrcloud".equals(systemInfo.get("mode"))) {
-        tool = new CreateCollectionTool(stdout);
+      if (SolrCLI.isCloudMode(solrClient)) {
+        createCollection(cli);
+      } else {
+        createCore(cli, solrClient);
+      }
+    }
+  }
+
+  protected void createCore(CommandLine cli, SolrClient solrClient) throws 
Exception {
+    String coreName = cli.getOptionValue("name");
+    String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
+
+    final String solrInstallDir = System.getProperty("solr.install.dir");
+    final String confDirName = cli.getOptionValue("confdir", 
SolrCLI.DEFAULT_CONFIG_SET);
+
+    // we allow them to pass a directory instead of a configset name
+    Path configsetDir = Paths.get(confDirName);
+    Path solrInstallDirPath = Paths.get(solrInstallDir);
+
+    if (!Files.isDirectory(configsetDir)) {
+      ensureConfDirExists(configsetDir, solrInstallDirPath);
+    }
+    printDefaultConfigsetWarningIfNecessary(cli);
+
+    String coreRootDirectory; // usually same as solr home, but not always
+
+    Map<String, Object> systemInfo =
+        solrClient
+            .request(new GenericSolrRequest(SolrRequest.METHOD.GET, 
CommonParams.SYSTEM_INFO_PATH))
+            .asMap();
+
+    // convert raw JSON into user-friendly output
+    coreRootDirectory = (String) systemInfo.get("core_root");
+
+    if (SolrCLI.safeCheckCoreExists(solrUrl, coreName)) {
+      throw new IllegalArgumentException(
+          "\nCore '"
+              + coreName
+              + "' already exists!\nChecked core existence using Core API 
command");
+    }
+
+    Path coreInstanceDir = Paths.get(coreRootDirectory, coreName);
+    Path confDir = getFullConfDir(configsetDir, 
solrInstallDirPath).resolve("conf");
+    if (!Files.isDirectory(coreInstanceDir)) {
+      Files.createDirectories(coreInstanceDir);
+      if (!Files.isDirectory(coreInstanceDir)) {
+        throw new IOException(
+            "Failed to create new core instance directory: " + 
coreInstanceDir.toAbsolutePath());
+      }
+
+      FileUtils.copyDirectoryToDirectory(confDir.toFile(), 
coreInstanceDir.toFile());
+
+      echoIfVerbose(
+          "\nCopying configuration to new core instance directory:\n"
+              + coreInstanceDir.toAbsolutePath(),
+          cli);
+    }
+
+    echoIfVerbose("\nCreating new core '" + coreName + "' using 
CoreAdminRequest", cli);
+
+    try {
+      CoreAdminResponse res = CoreAdminRequest.createCore(coreName, coreName, 
solrClient);
+      if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+        echo(res.jsonStr());
+        echo("\n");
       } else {
-        tool = new CreateCoreTool(stdout);
+        echo(String.format(Locale.ROOT, "\nCreated new core '%s'", coreName));
       }
-      tool.runImpl(cli);
+    } catch (Exception e) {
+      /* create-core failed, cleanup the copied configset before propagating 
the error. */
+      PathUtils.deleteDirectory(coreInstanceDir);
+      throw e;
+    }
+  }
+
+  protected void createCollection(CommandLine cli) throws Exception {
+    String zkHost = SolrCLI.getZkHost(cli);
+    try (CloudSolrClient cloudSolrClient =
+        new CloudHttp2SolrClient.Builder(Collections.singletonList(zkHost), 
Optional.empty())
+            .withInternalClientBuilder(
+                new Http2SolrClient.Builder()
+                    .withIdleTimeout(30, TimeUnit.SECONDS)
+                    .withConnectionTimeout(15, TimeUnit.SECONDS))
+            .build()) {
+      echoIfVerbose("Connecting to ZooKeeper at " + zkHost, cli);
+      cloudSolrClient.connect();
+      createCollection(cloudSolrClient, cli);
+    }
+  }
+
+  protected void createCollection(CloudSolrClient cloudSolrClient, CommandLine 
cli)
+      throws Exception {
+
+    String collectionName = cli.getOptionValue("name");
+    final String solrInstallDir = System.getProperty("solr.install.dir");
+    String confName = cli.getOptionValue("confname");
+    String confDir = cli.getOptionValue("confdir", "_default");
+    ensureConfDirExists(Paths.get(confDir), Paths.get(solrInstallDir));
+    printDefaultConfigsetWarningIfNecessary(cli);
+
+    Set<String> liveNodes = cloudSolrClient.getClusterState().getLiveNodes();
+    if (liveNodes.isEmpty())
+      throw new IllegalStateException(
+          "No live nodes found! Cannot create a collection until "
+              + "there is at least 1 live node in the cluster.");
+
+    String solrUrl = cli.getOptionValue("solrUrl");
+    if (solrUrl == null) {
+      String firstLiveNode = liveNodes.iterator().next();
+      solrUrl = 
ZkStateReader.from(cloudSolrClient).getBaseUrlForNodeName(firstLiveNode);
+    }
+
+    // build a URL to create the collection
+    int numShards = Integer.parseInt(cli.getOptionValue("shards", 
String.valueOf(1)));
+    int replicationFactor =
+        Integer.parseInt(cli.getOptionValue("replicationFactor", 
String.valueOf(1)));
+
+    final String configsetsDir = solrInstallDir + "/server/solr/configsets";
+
+    boolean configExistsInZk =
+        confName != null
+            && !confName.trim().isEmpty()
+            && ZkStateReader.from(cloudSolrClient)
+                .getZkClient()
+                .exists("/configs/" + confName, true);
+
+    if (CollectionAdminParams.SYSTEM_COLL.equals(collectionName)) {
+      // do nothing
+    } else if (configExistsInZk) {
+      echo("Re-using existing configuration directory " + confName);
+    } else { // if (confdir != null && !confdir.trim().isEmpty()) {
+      if (confName == null || confName.trim().isEmpty()) {
+        confName = collectionName;
+      }
+      Path confPath = ConfigSetService.getConfigsetPath(confDir, 
configsetsDir);
+
+      echoIfVerbose(
+          "Uploading "
+              + confPath.toAbsolutePath()
+              + " for config "
+              + confName
+              + " to ZooKeeper at "
+              + cloudSolrClient.getClusterStateProvider().getQuorumHosts(),
+          cli);
+      ZkMaintenanceUtils.uploadToZK(
+          ZkStateReader.from(cloudSolrClient).getZkClient(),
+          confPath,
+          ZkMaintenanceUtils.CONFIGS_ZKNODE + "/" + confName,
+          ZkMaintenanceUtils.UPLOAD_FILENAME_EXCLUDE_PATTERN);
+    }
+
+    // since creating a collection is a heavy-weight operation, check for 
existence first
+    if (SolrCLI.safeCheckCollectionExists(solrUrl, collectionName)) {
+      throw new IllegalStateException(
+          "\nCollection '"
+              + collectionName
+              + "' already exists!\nChecked collection existence using 
CollectionAdminRequest");
+    }
+
+    // doesn't seem to exist ... try to create
+    echoIfVerbose(
+        "\nCreating new collection '" + collectionName + "' using 
CollectionAdminRequest", cli);
+
+    NamedList<Object> response;
+    try {
+      response =
+          cloudSolrClient.request(
+              CollectionAdminRequest.createCollection(
+                  collectionName, confName, numShards, replicationFactor));
+    } catch (SolrServerException sse) {
+      throw new Exception(
+          "Failed to create collection '" + collectionName + "' due to: " + 
sse.getMessage());
+    }
+
+    if (cli.hasOption(SolrCLI.OPTION_VERBOSE.getOpt())) {
+      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);
+    }
+  }
+
+  private Path getFullConfDir(Path confDirName, Path solrInstallDir) {
+    Path configSetsPath = Paths.get("server/solr/configsets/");
+    return solrInstallDir.resolve(configSetsPath).resolve(confDirName);
+  }
+
+  private void ensureConfDirExists(Path confDirName, Path solrInstallDir) {

Review Comment:
   nitpick but do consider parameter order.  I think solrInstallDir should come 
first on this method and the previous method because confDirName is surely 
beneath it



##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -43,24 +68,295 @@ public String getName() {
 
   @Override
   public List<Option> getOptions() {
-    return CreateCollectionTool.CREATE_COLLECTION_OPTIONS;
+    return List.of(
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_SOLRURL,
+        Option.builder("c")
+            .longOpt("name")
+            .argName("NAME")
+            .hasArg()
+            .required(true)
+            .desc("Name of collection or core to create.")
+            .build(),
+        Option.builder("s")
+            .longOpt("shards")
+            .argName("#")
+            .hasArg()
+            .required(false)
+            .desc("Number of shards; default is 1.")
+            .build(),
+        Option.builder("rf")
+            .longOpt("replicationFactor")
+            .argName("#")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Number of copies of each document across the collection 
(replicas per shard); default is 1.")
+            .build(),
+        Option.builder("d")
+            .longOpt("confdir")
+            .argName("NAME")
+            .hasArg()
+            .required(false)
+            .desc(
+                "Configuration directory to copy when creating the new 
collection; default is "
+                    + SolrCLI.DEFAULT_CONFIG_SET
+                    + '.')
+            .build(),
+        Option.builder("n")
+            .longOpt("confname")
+            .argName("NAME")
+            .hasArg()
+            .required(false)
+            .desc("Configuration name; default is the collection name.")
+            .build(),
+        SolrCLI.OPTION_VERBOSE);
   }
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
     SolrCLI.raiseLogLevelUnlessVerbose(cli);
     String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
 
-    ToolBase tool;
     try (var solrClient = SolrCLI.getSolrClient(solrUrl)) {
-      NamedList<Object> systemInfo =
-          solrClient.request(new GenericSolrRequest(SolrRequest.METHOD.GET, 
SYSTEM_INFO_PATH));
-      if ("solrcloud".equals(systemInfo.get("mode"))) {
-        tool = new CreateCollectionTool(stdout);
+      if (SolrCLI.isCloudMode(solrClient)) {
+        createCollection(cli);
+      } else {
+        createCore(cli, solrClient);
+      }
+    }
+  }
+
+  protected void createCore(CommandLine cli, SolrClient solrClient) throws 
Exception {
+    String coreName = cli.getOptionValue("name");
+    String solrUrl = cli.getOptionValue("solrUrl", SolrCLI.DEFAULT_SOLR_URL);
+
+    final String solrInstallDir = System.getProperty("solr.install.dir");
+    final String confDirName = cli.getOptionValue("confdir", 
SolrCLI.DEFAULT_CONFIG_SET);
+
+    // we allow them to pass a directory instead of a configset name
+    Path configsetDir = Paths.get(confDirName);
+    Path solrInstallDirPath = Paths.get(solrInstallDir);
+
+    if (!Files.isDirectory(configsetDir)) {
+      ensureConfDirExists(configsetDir, solrInstallDirPath);
+    }
+    printDefaultConfigsetWarningIfNecessary(cli);
+
+    String coreRootDirectory; // usually same as solr home, but not always
+
+    Map<String, Object> systemInfo =
+        solrClient
+            .request(new GenericSolrRequest(SolrRequest.METHOD.GET, 
CommonParams.SYSTEM_INFO_PATH))
+            .asMap();
+
+    // convert raw JSON into user-friendly output
+    coreRootDirectory = (String) systemInfo.get("core_root");
+
+    if (SolrCLI.safeCheckCoreExists(solrUrl, coreName)) {
+      throw new IllegalArgumentException(
+          "\nCore '"
+              + coreName
+              + "' already exists!\nChecked core existence using Core API 
command");
+    }
+
+    Path coreInstanceDir = Paths.get(coreRootDirectory, coreName);
+    Path confDir = getFullConfDir(configsetDir, 
solrInstallDirPath).resolve("conf");
+    if (!Files.isDirectory(coreInstanceDir)) {
+      Files.createDirectories(coreInstanceDir);
+      if (!Files.isDirectory(coreInstanceDir)) {
+        throw new IOException(
+            "Failed to create new core instance directory: " + 
coreInstanceDir.toAbsolutePath());
+      }
+
+      FileUtils.copyDirectoryToDirectory(confDir.toFile(), 
coreInstanceDir.toFile());

Review Comment:
   Why are we dealing with "File" again?



-- 
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

Reply via email to