[helix] branch master updated: Do not proceed with cluster creation if addCluster() fails. (#2068)

[jxue]

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

jxue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new b129bd664 Do not proceed with cluster creation if addCluster() fails. 
(#2068)
b129bd664 is described below

commit b129bd664faae24f21aeb6c9e076153cc88e3635
Author: Komal Desai <98000016+desaiko...@users.noreply.github.com>
AuthorDate: Wed Apr 27 15:42:08 2022 -0700

    Do not proceed with cluster creation if addCluster() fails. (#2068)
    
    In Helix-Tools - ClusterSetup::addCluster() doesn't check return value
    of HelixAdmin::addCluster() method. It proceeds even when the call
    returns failure.
    Check the return status of addCluster() and throw an exception if cluster 
creation fails.
---
 .../java/org/apache/helix/tools/ClusterSetup.java  |  6 ++++-
 .../org/apache/helix/tools/TestClusterSetup.java   | 27 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 
b/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
index e01828c28..0d9a4f1ac 100644
--- a/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
+++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
@@ -223,7 +223,11 @@ public class ClusterSetup {
 
   public void addCluster(String clusterName, boolean overwritePrevious, 
CloudConfig cloudConfig)
       throws HelixException {
-    _admin.addCluster(clusterName, overwritePrevious);
+    if (!_admin.addCluster(clusterName, overwritePrevious)) {
+      String error = "Cluster creation failed for " + clusterName;
+      _logger.error(error);
+      throw new HelixException(error);
+    }
     for (BuiltInStateModelDefinitions def : 
BuiltInStateModelDefinitions.values()) {
       addStateModelDef(clusterName, def.getStateModelDefinition().getId(),
           def.getStateModelDefinition(), overwritePrevious);
diff --git 
a/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java 
b/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java
index 44cacc86f..8d0135fbd 100644
--- a/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java
+++ b/helix-core/src/test/java/org/apache/helix/tools/TestClusterSetup.java
@@ -26,6 +26,7 @@ import java.util.List;
 
 import org.apache.helix.BaseDataAccessor;
 import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
 import org.apache.helix.HelixDataAccessor;
 import org.apache.helix.HelixException;
 import org.apache.helix.PropertyKey;
@@ -53,6 +54,10 @@ import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class TestClusterSetup extends ZkUnitTestBase {
   protected static final String CLUSTER_NAME = "TestClusterSetup";
@@ -519,6 +524,28 @@ public class TestClusterSetup extends ZkUnitTestBase {
     Assert.assertEquals(cloudConfigFromZk.getCloudProvider(), 
CloudProvider.CUSTOMIZED.name());
   }
 
+  @Test(dependsOnMethods = "testAddClusterWithInvalidCloudConfig",
+      expectedExceptions = HelixException.class)
+  public void testAddClusterWithFailure() throws Exception {
+    HelixAdmin admin = mock(HelixAdmin.class);
+    when(admin.addCluster(anyString(), 
anyBoolean())).thenReturn(Boolean.FALSE);
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+
+    CloudConfig.Builder cloudConfigInitBuilder = new CloudConfig.Builder();
+    cloudConfigInitBuilder.setCloudEnabled(true);
+    List<String> sourceList = new ArrayList<String>();
+    sourceList.add("TestURL");
+    cloudConfigInitBuilder.setCloudInfoSources(sourceList);
+    cloudConfigInitBuilder.setCloudProvider(CloudProvider.CUSTOMIZED);
+
+    CloudConfig cloudConfigInit = cloudConfigInitBuilder.build();
+
+    // Since setCloudInfoProcessorName is missing, this add cluster call will 
throw an exception
+    _clusterSetup.addCluster(clusterName, false, cloudConfigInit);
+  }
+
   @Test(dependsOnMethods = "testAddClusterWithValidCloudConfig")
   public void testAddClusterAzureProvider() throws Exception {
     String className = TestHelper.getTestClassName();