You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2019/12/14 08:45:30 UTC
[lucene-solr] 01/01: SOLR-13662: Fixes to package manager
This is an automated email from the ASF dual-hosted git repository.
ishan pushed a commit to branch jira/solr-13662-fixes
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit ca3788cd6ca647ea2d4bbdefaf12027af10510dc
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Sat Dec 14 14:12:43 2019 +0530
SOLR-13662: Fixes to package manager
* Better logging and error reporting
* Fixing deploy command to handle previously undeployed packages
* Test now uses @LogLevel annotation
* Deploy command had a hard coded collection name by mistake, fix it
---
.../apache/solr/packagemanager/PackageManager.java | 58 ++++++++++++++++++----
.../solr/packagemanager/RepositoryManager.java | 12 +++--
.../src/java/org/apache/solr/util/PackageTool.java | 18 ++++---
.../apache/solr/cloud/PackageManagerCLITest.java | 9 +---
4 files changed, 67 insertions(+), 30 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
index 73f50dc..493e00a 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
@@ -121,25 +121,44 @@ public class PackageManager implements Closeable {
Map<String, SolrPackageInstance> ret = new HashMap<String, SolrPackageInstance>();
for (String packageName: packages.keySet()) {
if (Strings.isNullOrEmpty(packageName) == false) { // There can be an empty key, storing the version here
+ if (packages.get(packageName) == null) { // This means the package was undeployed from this package before
+ continue;
+ }
ret.put(packageName, getPackageInstance(packageName, packages.get(packageName)));
}
}
return ret;
}
+ private void ensureCollectionsExist(List<String> collections) {
+ try {
+ List<String> existingCollections = zkClient.getChildren("/collections", null, true);
+ for (String c: collections) {
+ if (existingCollections.contains(c) == false) {
+ throw new SolrException(ErrorCode.BAD_REQUEST, "Collection " + c + " doesn't exist.");
+ }
+ }
+ } catch (KeeperException | InterruptedException e) {
+ throw new SolrException(ErrorCode.SERVER_ERROR, "Unable to fetch list of collections from ZK.");
+ }
+ }
+
private boolean deployPackage(SolrPackageInstance packageInstance, boolean pegToLatest, boolean isUpdate, boolean noprompt,
List<String> collections, String overrides[]) {
- for (String collection: collections) {
+ List<String> alreadyDeployed = new ArrayList<>(); // collections where package is already deployed in
+ for (String collection: collections) {
SolrPackageInstance deployedPackage = getPackagesDeployed(collection).get(packageInstance.name);
if (packageInstance.equals(deployedPackage)) {
if (!pegToLatest) {
PackageUtils.printRed("Package " + packageInstance + " already deployed on "+collection);
+ alreadyDeployed.add(collection);
continue;
}
} else {
if (deployedPackage != null && !isUpdate) {
PackageUtils.printRed("Package " + deployedPackage + " already deployed on "+collection+". To update to "+packageInstance+", pass --update parameter.");
+ alreadyDeployed.add(collection);
continue;
}
}
@@ -148,7 +167,7 @@ public class PackageManager implements Closeable {
// Get package params
try {
- boolean packageParamsExist = ((Map)PackageUtils.getJson(solrClient.getHttpClient(), solrBaseUrl + "/api/collections/abc/config/params/packages", Map.class)
+ boolean packageParamsExist = ((Map)PackageUtils.getJson(solrClient.getHttpClient(), solrBaseUrl + "/api/collections/"+collection+"/config/params/packages", Map.class)
.getOrDefault("response", Collections.emptyMap())).containsKey("params");
SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params",
getMapper().writeValueAsString(Collections.singletonMap(packageParamsExist? "update": "set",
@@ -220,12 +239,21 @@ public class PackageManager implements Closeable {
}
}
- // Verify that package was successfully deployed
- boolean success = verify(packageInstance, collections);
- if (success) {
- PackageUtils.printGreen("Deployed and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
+ List<String> deployedCollections = new ArrayList<String>();
+ for (String c: collections) if (alreadyDeployed.contains(c) == false) deployedCollections.add(c);
+
+ boolean success = true;
+ if (deployedCollections.isEmpty() == false) {
+ // Verify that package was successfully deployed
+ success = verify(packageInstance, deployedCollections);
+ if (success) {
+ PackageUtils.printGreen("Deployed on " + deployedCollections + " and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
+ }
}
- return success;
+ if (alreadyDeployed.isEmpty() == false) {
+ PackageUtils.printRed("Already Deployed on " + alreadyDeployed + ", package: " + packageInstance.name + ", version: " + packageInstance.version);
+ }
+ return alreadyDeployed.isEmpty()==false? false: success;
}
private Map<String,String> getCollectionParameterOverrides(SolrPackageInstance packageInstance, boolean isUpdate,
@@ -260,7 +288,7 @@ public class PackageManager implements Closeable {
public boolean verify(SolrPackageInstance pkg, List<String> collections) {
boolean success = true;
for (Plugin plugin: pkg.plugins) {
- PackageUtils.printGreen(plugin.verifyCommand);
+ // PackageUtils.printGreen(plugin.verifyCommand);
for (String collection: collections) {
Map<String, String> collectionParameterOverrides = getPackageParams(pkg.name, collection);
Command cmd = plugin.verifyCommand;
@@ -322,6 +350,8 @@ public class PackageManager implements Closeable {
*/
public void deploy(String packageName, String version, String[] collections, String[] parameters,
boolean isUpdate, boolean noprompt) throws SolrException {
+ ensureCollectionsExist(Arrays.asList(collections));
+
boolean pegToLatest = PackageUtils.LATEST.equals(version); // User wants to peg this package's version to the latest installed (for auto-update, i.e. no explicit deploy step)
SolrPackageInstance packageInstance = getPackageInstance(packageName, version);
if (packageInstance == null) {
@@ -342,11 +372,17 @@ public class PackageManager implements Closeable {
}
/**
- * Undeploys a packge from given collections.
+ * Undeploys a package from given collections.
*/
public void undeploy(String packageName, String[] collections) throws SolrException {
+ ensureCollectionsExist(Arrays.asList(collections));
+
for (String collection: collections) {
SolrPackageInstance deployedPackage = getPackagesDeployed(collection).get(packageName);
+ if (deployedPackage == null) {
+ PackageUtils.printRed("Package "+packageName+" not deployed on collection "+collection);
+ continue;
+ }
Map<String, String> collectionParameterOverrides = getPackageParams(packageName, collection);
// Run the uninstall command for all plugins
@@ -374,13 +410,13 @@ public class PackageManager implements Closeable {
// Set the package version in the collection's parameters
try {
- SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params", "{set: {PKG_VERSIONS: {"+packageName+": null}}}");
+ SolrCLI.postJsonToSolr(solrClient, "/api/collections/" + collection + "/config/params", "{set: {PKG_VERSIONS: {"+packageName+": null}}}"); // Is it better to "unset"? If so, build support in params API for "unset"
SolrCLI.postJsonToSolr(solrClient, "/api/cluster/package", "{\"refresh\": \"" + packageName + "\"}");
} catch (Exception ex) {
throw new SolrException(ErrorCode.SERVER_ERROR, ex);
}
- // TODO: Also better to remove the package parameters
+ // TODO: Also better to remove the package parameters PKG_VERSION etc.
}
}
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
index 5dd503e..66b59f5 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
@@ -308,10 +308,12 @@ public class RepositoryManager {
installPackage(packageName, version);
}
- SolrPackageInstance updatedPackage = packageManager.getPackageInstance(packageName, PackageUtils.LATEST);
- boolean res = packageManager.verify(updatedPackage, peggedToLatest);
- PackageUtils.printGreen("Verifying version " + updatedPackage.version +
- " on " + peggedToLatest + ", result: " + res);
- if (!res) throw new SolrException(ErrorCode.BAD_REQUEST, "Failed verification after deployment");
+ if (peggedToLatest.isEmpty() == false) {
+ SolrPackageInstance updatedPackage = packageManager.getPackageInstance(packageName, PackageUtils.LATEST);
+ boolean res = packageManager.verify(updatedPackage, peggedToLatest);
+ PackageUtils.printGreen("Verifying version " + updatedPackage.version +
+ " on " + peggedToLatest + ", result: " + res);
+ if (!res) throw new SolrException(ErrorCode.BAD_REQUEST, "Failed verification after deployment");
+ }
}
}
diff --git a/solr/core/src/java/org/apache/solr/util/PackageTool.java b/solr/core/src/java/org/apache/solr/util/PackageTool.java
index 8464ec1..f7744c9 100644
--- a/solr/core/src/java/org/apache/solr/util/PackageTool.java
+++ b/solr/core/src/java/org/apache/solr/util/PackageTool.java
@@ -118,9 +118,13 @@ public class PackageTool extends SolrCLI.ToolBase {
} else {
String packageName = cli.getArgs()[1];
Map<String, String> deployedCollections = packageManager.getDeployedCollections(packageName);
- PackageUtils.printGreen("Collections on which package " + packageName + " was deployed:");
- for (String collection: deployedCollections.keySet()) {
- PackageUtils.printGreen("\t" + collection + "("+packageName+":"+deployedCollections.get(collection)+")");
+ if (deployedCollections.isEmpty() == false) {
+ PackageUtils.printGreen("Collections on which package " + packageName + " was deployed:");
+ for (String collection: deployedCollections.keySet()) {
+ PackageUtils.printGreen("\t" + collection + "("+packageName+":"+deployedCollections.get(collection)+")");
+ }
+ } else {
+ PackageUtils.printGreen("Package "+packageName+" not deployed on any collection.");
}
}
break;
@@ -130,7 +134,7 @@ public class PackageTool extends SolrCLI.ToolBase {
String packageName = parsedVersion.first();
String version = parsedVersion.second();
repositoryManager.install(packageName, version);
- PackageUtils.printGreen(repositoryManager.toString() + " installed.");
+ PackageUtils.printGreen(packageName + " installed.");
break;
}
case "deploy":
@@ -140,7 +144,7 @@ public class PackageTool extends SolrCLI.ToolBase {
String version = parsedVersion.second();
boolean noprompt = cli.hasOption('y');
boolean isUpdate = cli.hasOption("update") || cli.hasOption('u');
- packageManager.deploy(packageName, version, PackageUtils.validateCollections(cli.getOptionValues("collections")), cli.getOptionValues("param"), isUpdate, noprompt);
+ packageManager.deploy(packageName, version, PackageUtils.validateCollections(cli.getOptionValue("collections").split(",")), cli.getOptionValues("param"), isUpdate, noprompt);
break;
}
case "undeploy":
@@ -150,7 +154,7 @@ public class PackageTool extends SolrCLI.ToolBase {
throw new SolrException(ErrorCode.BAD_REQUEST, "Only package name expected, without a version. Actual: " + cli.getArgList().get(1));
}
String packageName = parsedVersion.first();
- packageManager.undeploy(packageName, cli.getOptionValues("collections"));
+ packageManager.undeploy(packageName, cli.getOptionValue("collections").split(","));
break;
}
case "help":
@@ -228,7 +232,7 @@ public class PackageTool extends SolrCLI.ToolBase {
OptionBuilder
.withArgName("COLLECTIONS")
- .hasArgs()
+ .hasArg()
.isRequired(false)
.withDescription("List of collections. Run './solr package help' for more details.")
.create("collections"),
diff --git a/solr/core/src/test/org/apache/solr/cloud/PackageManagerCLITest.java b/solr/core/src/test/org/apache/solr/cloud/PackageManagerCLITest.java
index 4c08aad..38b66b6 100644
--- a/solr/core/src/test/org/apache/solr/cloud/PackageManagerCLITest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/PackageManagerCLITest.java
@@ -20,11 +20,9 @@ package org.apache.solr.cloud;
import java.lang.invoke.MethodHandles;
import java.util.Arrays;
-import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.core.config.Configurator;
-import org.apache.lucene.util.SuppressForbidden;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.core.TestSolrConfigHandler;
+import org.apache.solr.util.LogLevel;
import org.apache.solr.util.PackageTool;
import org.apache.solr.util.SolrCLI;
import org.eclipse.jetty.server.Handler;
@@ -39,6 +37,7 @@ import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+@LogLevel("org.apache=INFO")
public class PackageManagerCLITest extends SolrCloudTestCase {
// Note for those who want to modify the jar files used in the packages used in this test:
@@ -70,13 +69,9 @@ public class PackageManagerCLITest extends SolrCloudTestCase {
}
@Test
- @SuppressForbidden(reason = "Need to turn off logging, and SLF4J doesn't seem to provide for a way.")
public void testPackageManager() throws Exception {
PackageTool tool = new PackageTool();
- // Enable the logger for this test. Need to do this since the tool disables logger.
- Configurator.setRootLevel(Level.INFO);
-
String solrUrl = cluster.getJettySolrRunner(0).getBaseUrl().toString();
run(tool, new String[] {"-solrUrl", solrUrl, "list-installed"});