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:29 UTC

[lucene-solr] branch jira/solr-13662-fixes created (now ca3788c)

This is an automated email from the ASF dual-hosted git repository.

ishan pushed a change to branch jira/solr-13662-fixes
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


      at ca3788c  SOLR-13662: Fixes to package manager

This branch includes the following new commits:

     new ca3788c  SOLR-13662: Fixes to package manager

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[lucene-solr] 01/01: SOLR-13662: Fixes to package manager

Posted by is...@apache.org.
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"});