diff --git a/changelog/unreleased/SOLR-18320-skip-zk-for-configset-upload.yml b/changelog/unreleased/SOLR-18320-skip-zk-for-configset-upload.yml new file mode 100644 index 000000000000..fa9d69fa2f78 --- /dev/null +++ b/changelog/unreleased/SOLR-18320-skip-zk-for-configset-upload.yml @@ -0,0 +1,7 @@ +title: bin/solr create no longer requires a direct ZooKeeper connection to upload a configset; it now uses the Configsets V2 API. +type: changed +authors: + - name: Eric Pugh +links: + - name: SOLR-18320 + url: https://issues.apache.org/jira/browse/SOLR-18320 diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java index dae7174605ad..20cf3b5820e5 100644 --- a/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java +++ b/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java @@ -31,7 +31,14 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** Supports zk upconfig command in the bin/solr script. */ +/** + * Supports zk upconfig command in the bin/solr script. + * + *

Unlike {@link CreateTool}, this uploads directly to ZooKeeper rather than going through the + * Configsets V2 API. That's intentional: this command is meant to work against a bare ZooKeeper + * ensemble with no Solr node running yet (e.g. provisioning configs before the cluster starts), and + * the V2 API requires an HTTP endpoint on an already-running node to PUT the zip to. + */ @SuppressWarnings("UnnecessarilyFullyQualified") @picocli.CommandLine.Command( name = "upconfig", diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java index 58149b974757..9e58a157ab2e 100644 --- a/solr/core/src/java/org/apache/solr/cli/CreateTool.java +++ b/solr/core/src/java/org/apache/solr/cli/CreateTool.java @@ -16,6 +16,7 @@ */ package org.apache.solr.cli; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -34,12 +35,13 @@ import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.client.solrj.request.CollectionsApi; +import org.apache.solr.client.solrj.request.ConfigsetsApi; import org.apache.solr.client.solrj.request.CoresApi; import org.apache.solr.client.solrj.request.SystemInfoRequest; import org.apache.solr.client.solrj.response.SystemInfoResponse; -import org.apache.solr.cloud.ZkConfigSetService; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.EnvUtils; +import org.apache.solr.common.util.Utils; import org.apache.solr.core.ConfigSetService; /** Supports create command in the bin/solr script. */ @@ -317,24 +319,22 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para int replicationFactor = params.replicationFactor; String confName = params.confName; - boolean configExistsInZk = + boolean configExists = confName != null && !confName.trim().isEmpty() - && ZkStateReader.from(cloudSolrClient).getZkClient().exists("/configs/" + confName); + && new ConfigsetsApi.ListConfigSet() + .process(cloudSolrClient) + .configSets + .contains(confName); - if (configExistsInZk) { + if (configExists) { echo("Re-using existing configuration directory " + confName); - } else { // if (confdir != null && !confdir.trim().isEmpty()) { + } else { if (confName == null || confName.trim().isEmpty()) { confName = params.name; } - // TODO: This should be done using the configSet API. This would let us remove - // the direct dependency on ZooKeeper APIs. Unlike the bin/solr zk comamnds that - // work directly with ZooKeeper. final Path configsetsDirPath = CLIUtils.getConfigSetsDir(solrInstallDirPath); - ConfigSetService configSetService = - new ZkConfigSetService(ZkStateReader.from(cloudSolrClient).getZkClient()); Path confPath = ConfigSetService.getConfigsetPath(params.confDir, configsetsDirPath.toString()); @@ -343,10 +343,11 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para + confPath.toAbsolutePath() + " for config " + confName - + " to ZooKeeper at " - + cloudSolrClient.getClusterStateProvider().getQuorumHosts()); - // We will trust the config since we have the Zookeeper Address - configSetService.uploadConfig(confName, confPath); + + " using the Configsets V2 API"); + var uploadReq = + new ConfigsetsApi.UploadConfigSet( + confName, new ByteArrayInputStream(zipConfigSet(confPath))); + uploadReq.process(cloudSolrClient); } // since creating a collection is a heavy-weight operation, check for existence first @@ -387,6 +388,11 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para echo(endMessage); } + /** Zips the contents of a configset directory for upload. */ + static byte[] zipConfigSet(Path confPath) throws IOException { + return Utils.zipDirectory(confPath); + } + private Path getFullConfDir(Path solrInstallDir, Path confDirName) { return CLIUtils.getConfigSetsDir(solrInstallDir).resolve(confDirName); } diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java b/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java index 729aaf00d914..76d12758db01 100644 --- a/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java +++ b/solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java @@ -21,20 +21,14 @@ import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.StreamingOutput; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.InputStream; -import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.SimpleFileVisitor; -import java.nio.file.attribute.BasicFileAttributes; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; import org.apache.commons.io.file.PathUtils; import org.apache.solr.client.api.endpoint.ConfigsetsApi; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.StrUtils; +import org.apache.solr.common.util.Utils; import org.apache.solr.core.ConfigSetService; import org.apache.solr.core.CoreContainer; import org.apache.solr.jersey.PermissionName; @@ -86,48 +80,12 @@ public static Response buildZipResponse(ConfigSetService configSetService, Strin */ public static byte[] zipConfigSet(ConfigSetService configSetService, String configSetName) throws IOException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); Path tmpDirectory = Files.createTempDirectory("configset-download-"); try { configSetService.downloadConfig(configSetName, tmpDirectory); - try (ZipOutputStream zipOut = new ZipOutputStream(baos)) { - Files.walkFileTree( - tmpDirectory, - new SimpleFileVisitor<>() { - @Override - public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) - throws IOException { - if (Files.isHidden(dir)) { - return FileVisitResult.SKIP_SUBTREE; - } - String dirName = tmpDirectory.relativize(dir).toString(); - if (!dirName.isEmpty()) { - if (!dirName.endsWith("/")) { - dirName += "/"; - } - zipOut.putNextEntry(new ZipEntry(dirName)); - zipOut.closeEntry(); - } - return FileVisitResult.CONTINUE; - } - - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) - throws IOException { - if (!Files.isHidden(file)) { - try (InputStream fis = Files.newInputStream(file)) { - ZipEntry zipEntry = new ZipEntry(tmpDirectory.relativize(file).toString()); - zipOut.putNextEntry(zipEntry); - fis.transferTo(zipOut); - } - } - return FileVisitResult.CONTINUE; - } - }); - } + return Utils.zipDirectory(tmpDirectory); } finally { PathUtils.deleteDirectory(tmpDirectory); } - return baos.toByteArray(); } } diff --git a/solr/core/src/test/org/apache/solr/cli/CreateToolTest.java b/solr/core/src/test/org/apache/solr/cli/CreateToolTest.java index e1df239b51bb..a814ba529dec 100644 --- a/solr/core/src/test/org/apache/solr/cli/CreateToolTest.java +++ b/solr/core/src/test/org/apache/solr/cli/CreateToolTest.java @@ -17,6 +17,13 @@ package org.apache.solr.cli; +import java.io.ByteArrayInputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.Set; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.util.SecurityJson; import org.junit.BeforeClass; @@ -57,4 +64,52 @@ public void testCreateCollectionWithBasicAuth() throws Exception { assertEquals(0, runTool(args, CreateTool.class)); } + + @Test + public void testCreateCollectionUploadsNewConfigSet() throws Exception { + String[] args = { + "create", + "-c", + "testCreateCollectionUploadsNewConfigSet", + "-d", + configset("cloud-minimal").toString(), + "-n", + "cloud-minimal-uploaded", + "-z", + cluster.getZkClient().getZkServerAddress(), + "--credentials", + SecurityJson.USER_PASS, + "--verbose" + }; + + assertEquals(0, CLITestHelper.runTool(args, CreateTool.class)); + assertTrue(cluster.getZkClient().exists("/configs/cloud-minimal-uploaded")); + } + + @Test + public void testZipConfigSetSkipsHiddenFilesAndIncludesDirectoryEntries() throws Exception { + Path confDir = createTempDir("zipConfigSetTest"); + Files.writeString(confDir.resolve("solrconfig.xml"), ""); + Files.writeString(confDir.resolve(".hidden-file"), "should not be zipped"); + Path langDir = Files.createDirectory(confDir.resolve("lang")); + Files.writeString(langDir.resolve("stopwords.txt"), "the\na\n"); + Path hiddenDir = Files.createDirectory(confDir.resolve(".hiddenDir")); + Files.writeString(hiddenDir.resolve("nope.txt"), "should not be zipped either"); + + byte[] zipBytes = CreateTool.zipConfigSet(confDir); + + Set entryNames = new HashSet<>(); + try (ZipInputStream zipIn = new ZipInputStream(new ByteArrayInputStream(zipBytes))) { + ZipEntry entry; + while ((entry = zipIn.getNextEntry()) != null) { + entryNames.add(entry.getName()); + } + } + + assertTrue(entryNames.contains("solrconfig.xml")); + assertTrue(entryNames.contains("lang/")); + assertTrue(entryNames.contains("lang/stopwords.txt")); + assertFalse(entryNames.contains(".hidden-file")); + assertTrue(entryNames.stream().noneMatch(name -> name.startsWith(".hiddenDir"))); + } } diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index 71cdbb59c23c..810cf0861328 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -43,6 +43,11 @@ import java.nio.BufferOverflowException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.AbstractMap; @@ -69,6 +74,8 @@ import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.LinkedHashMapWriter; import org.apache.solr.common.MapWriter; @@ -1045,4 +1052,53 @@ public static ByteBuffer toByteArray(InputStream is) throws IOException { return bos.getByteBuffer(); } } + + /** + * Zips the contents of {@code rootPath} into an in-memory archive. Hidden files and directories + * (as determined by {@link Files#isHidden}) are skipped, directory entries are written for + * non-empty subdirectories, and zip entry names are normalized to use {@code /} separators + * regardless of platform. + * + * @param rootPath the directory to zip + * @return the zipped bytes + */ + public static byte[] zipDirectory(Path rootPath) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ZipOutputStream zipOut = new ZipOutputStream(baos)) { + Files.walkFileTree( + rootPath, + new SimpleFileVisitor<>() { + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) + throws IOException { + if (Files.isHidden(dir)) { + return FileVisitResult.SKIP_SUBTREE; + } + String dirName = rootPath.relativize(dir).toString().replace('\\', '/'); + if (!dirName.isEmpty()) { + if (!dirName.endsWith("/")) { + dirName += "/"; + } + zipOut.putNextEntry(new ZipEntry(dirName)); + zipOut.closeEntry(); + } + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) + throws IOException { + if (Files.isHidden(file)) { + return FileVisitResult.CONTINUE; + } + String entryName = rootPath.relativize(file).toString().replace('\\', '/'); + zipOut.putNextEntry(new ZipEntry(entryName)); + Files.copy(file, zipOut); + zipOut.closeEntry(); + return FileVisitResult.CONTINUE; + } + }); + } + return baos.toByteArray(); + } } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/RemoteSolrBackend.java b/solr/test-framework/src/java/org/apache/solr/cloud/RemoteSolrBackend.java index 78cb06ecee0a..3ef526a01762 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/RemoteSolrBackend.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/RemoteSolrBackend.java @@ -20,8 +20,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; import org.apache.solr.SolrBackend; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; @@ -30,6 +28,7 @@ import org.apache.solr.client.solrj.request.ConfigSetAdminRequest; import org.apache.solr.common.util.IOUtils; import org.apache.solr.common.util.URLUtil; +import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; /** @@ -68,7 +67,7 @@ public SolrClient getSolrClient() { public void uploadConfigSet(Path configDir, String name) throws SolrServerException, IOException { Path tempZip = Files.createTempFile("configset-", ".zip"); try { - zipDirectory(configDir, tempZip); + Files.write(tempZip, Utils.zipDirectory(configDir)); new ConfigSetAdminRequest.Upload() .setConfigSetName(name) .setUploadFile(tempZip, "application/zip") @@ -83,24 +82,6 @@ public boolean hasConfigSet(String name) throws SolrServerException, IOException return new ConfigSetAdminRequest.List().process(adminClient).getConfigSets().contains(name); } - /** Packages {@code sourceDir} contents into a zip file. */ - private static void zipDirectory(Path sourceDir, Path targetZip) throws IOException { - try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(targetZip))) { - Files.walk(sourceDir) - .filter(Files::isRegularFile) - .forEach( - file -> { - try { - zos.putNextEntry(new ZipEntry(sourceDir.relativize(file).toString())); - Files.copy(file, zos); - zos.closeEntry(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }); - } - } - @Override public void createCollection(CollectionAdminRequest.Create create) throws SolrServerException, IOException {