[lucene-solr] branch jira/solr-14599_1 updated: SOLR-14599: Fixing nocommits

[is...@apache.org]

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

ishan pushed a commit to branch jira/solr-14599_1
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/jira/solr-14599_1 by this push:
     new 54f1cc0  SOLR-14599: Fixing nocommits
54f1cc0 is described below

commit 54f1cc0a9cd68f08817357fa98c7792f96bbb7bd
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Tue Jun 30 13:41:04 2020 +0530

    SOLR-14599: Fixing nocommits
---
 .../apache/solr/packagemanager/PackageManager.java | 38 +++++++++++++---------
 .../solr/packagemanager/RepositoryManager.java     | 12 +++----
 2 files changed, 29 insertions(+), 21 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 3e995dd..ca576ed 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
@@ -40,6 +40,7 @@ import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.Pair;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.packagemanager.SolrPackage.Command;
 import org.apache.solr.packagemanager.SolrPackage.Manifest;
@@ -189,12 +190,26 @@ public class PackageManager implements Closeable {
 
     
     // Install plugins of type "collection"
-    boolean collectionSuccess = deployCollectionPackage(packageInstance, pegToLatest, isUpdate, noprompt, collections,
+    Pair<List<String>, List<String>> deployResult = deployCollectionPackage(packageInstance, pegToLatest, isUpdate, noprompt, collections,
         shouldDeployClusterPlugins, overrides);
-    return cluasterSuccess && collectionSuccess;
+    List<String> deployedCollections = deployResult.first();
+    List<String> previouslyDeployedOnCollections = deployResult.second();
+    
+    // Verify
+    boolean verifySuccess = true;
+    // Verify that package was successfully deployed
+    verifySuccess = verify(packageInstance, deployedCollections, shouldDeployClusterPlugins, overrides);
+    if (verifySuccess) {
+      PackageUtils.printGreen("Deployed on " + deployedCollections + " and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
+    }
+
+    return cluasterSuccess && previouslyDeployedOnCollections.isEmpty() && verifySuccess;
   }
 
-  private boolean deployCollectionPackage(SolrPackageInstance packageInstance, boolean pegToLatest, boolean isUpdate,
+  /**
+   * @return list of collections on which packages deployed on
+   */
+  private Pair<List<String>, List<String>> deployCollectionPackage(SolrPackageInstance packageInstance, boolean pegToLatest, boolean isUpdate,
       boolean noprompt, List<String> collections, boolean shouldDeployClusterPlugins, String[] overrides) {
     List<String> previouslyDeployed =  new ArrayList<>(); // collections where package is already deployed in
     for (String collection: collections) {
@@ -290,21 +305,12 @@ public class PackageManager implements Closeable {
       }
     }
 
-    List<String> deployedCollections = collections.stream().filter(c -> !previouslyDeployed.contains(c)).collect(Collectors.toList());
-
-    boolean success = true;
-    if (deployedCollections.isEmpty() == false) {
-      // Verify that package was successfully deployed
-      success = verify(packageInstance, deployedCollections, shouldDeployClusterPlugins, overrides);
-      if (success) {
-        PackageUtils.printGreen("Deployed on " + deployedCollections + " and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
-      }
-    }
     if (previouslyDeployed.isEmpty() == false) {
       PackageUtils.printRed("Already Deployed on " + previouslyDeployed + ", package: " + packageInstance.name + ", version: " + packageInstance.version);
     }
-    success &= previouslyDeployed.isEmpty();
-    return success;
+
+    List<String> deployedCollections = collections.stream().filter(c -> !previouslyDeployed.contains(c)).collect(Collectors.toList());
+    return new Pair(deployedCollections, previouslyDeployed);
   }
 
   private boolean deployClusterPackage(SolrPackageInstance packageInstance, boolean isUpdate, boolean noprompt,
@@ -430,6 +436,8 @@ public class PackageManager implements Closeable {
   /**
    * Given a package and list of collections, verify if the package is installed
    * in those collections. It uses the verify command of every plugin in the package (if defined).
+   * 
+   * @param overrides are needed only when shouldDeployClusterPlugins is true, since collection level plugins will get their overrides from ZK (collection params API)
    */
   public boolean verify(SolrPackageInstance pkg, List<String> collections, boolean shouldDeployClusterPlugins, String overrides[]) {
     boolean success = true;
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 9ce4645..beabc7a 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/RepositoryManager.java
@@ -306,10 +306,10 @@ public class RepositoryManager {
     String latestVersion = getLastPackageRelease(packageName).version;
 
     Map<String, String> collectionsDeployedIn = packageManager.getDeployedCollections(packageName);
-    List<String> peggedToLatest = collectionsDeployedIn.keySet().stream().
+    List<String> collectionsPeggedToLatest = collectionsDeployedIn.keySet().stream().
         filter(collection -> collectionsDeployedIn.get(collection).equals(PackagePluginHolder.LATEST)).collect(Collectors.toList());
-    if (!peggedToLatest.isEmpty()) {
-      PackageUtils.printGreen("Collections that will be affected (since they are configured to use $LATEST): "+peggedToLatest);
+    if (!collectionsPeggedToLatest.isEmpty()) {
+      PackageUtils.printGreen("Collections that will be affected (since they are configured to use $LATEST): "+collectionsPeggedToLatest);
     }
 
     if (version == null || version.equals(PackageUtils.LATEST)) {
@@ -318,11 +318,11 @@ public class RepositoryManager {
       installPackage(packageName, version);
     }
 
-    if (peggedToLatest.isEmpty() == false) {
+    if (collectionsPeggedToLatest.isEmpty() == false) {
       SolrPackageInstance updatedPackage = packageManager.getPackageInstance(packageName, PackageUtils.LATEST);
-      boolean res = packageManager.verify(updatedPackage, peggedToLatest, true, new String[] {}); // nocommit: get the overrides here
+      boolean res = packageManager.verify(updatedPackage, collectionsPeggedToLatest, false, new String[] {}); // Cluster level plugins don't work with peggedToLatest functionality
       PackageUtils.printGreen("Verifying version " + updatedPackage.version + 
-          " on " + peggedToLatest + ", result: " + res);
+          " on " + collectionsPeggedToLatest + ", result: " + res);
       if (!res) throw new SolrException(ErrorCode.BAD_REQUEST, "Failed verification after deployment");
     }
   }