You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by jx...@apache.org on 2020/04/07 01:20:41 UTC

[helix] 12/12: Add REST and JAVA API to modify existing cloudconfig (#898)

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

commit ec2f3d983acfc2ae4605a8b9452ea6c335a95b80
Author: Ali Reza Zamani Zadeh Najari <an...@linkedin.com>
AuthorDate: Tue Mar 17 13:07:46 2020 -0700

    Add REST and JAVA API to modify existing cloudconfig (#898)
    
    The new APIs are added to update/delete cloudconfig entries.
    Tests have been modified accordingly.
---
 .../main/java/org/apache/helix/ConfigAccessor.java | 42 +++++++++++++++-
 .../AzureCloudInstanceInformationProcessor.java    |  8 ++--
 .../java/org/apache/helix/model/CloudConfig.java   |  2 +-
 .../java/org/apache/helix/TestConfigAccessor.java  | 56 ++++++++++++++++++++--
 ...TestAzureCloudInstanceInformationProcessor.java |  8 ++--
 .../server/resources/helix/ClusterAccessor.java    | 13 +++--
 .../helix/rest/server/TestClusterAccessor.java     | 50 ++++++++++++++++++-
 7 files changed, 157 insertions(+), 22 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
index c0afde4..8ae7fd5 100644
--- a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
+++ b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
@@ -605,7 +605,47 @@ public class ConfigAccessor {
       return null;
     }
 
-    return new CloudConfig.Builder(record).build();
+    return new CloudConfig(record);
+  }
+
+  /**
+   * Delete cloud config fields (not the whole config)
+   * @param clusterName
+   * @param cloudConfig
+   */
+  public void deleteCloudConfigFields(String clusterName, CloudConfig cloudConfig) {
+    if (!ZKUtil.isClusterSetup(clusterName, _zkClient)) {
+      throw new HelixException("fail to delete cloud config. cluster: " + clusterName + " is NOT setup.");
+    }
+
+    HelixConfigScope scope =
+        new HelixConfigScopeBuilder(ConfigScopeProperty.CLOUD).forCluster(clusterName).build();
+    remove(scope, cloudConfig.getRecord());
+  }
+
+  /**
+   * Update cloud config
+   * @param clusterName
+   * @param cloudConfig
+   */
+  public void updateCloudConfig(String clusterName, CloudConfig cloudConfig) {
+    updateCloudConfig(clusterName, cloudConfig, false);
+  }
+
+  private void updateCloudConfig(String clusterName, CloudConfig cloudConfig, boolean overwrite) {
+    if (!ZKUtil.isClusterSetup(clusterName, _zkClient)) {
+      throw new HelixException("Fail to update cloud config. cluster: " + clusterName + " is NOT setup.");
+    }
+
+    HelixConfigScope scope =
+        new HelixConfigScopeBuilder(ConfigScopeProperty.CLOUD).forCluster(clusterName).build();
+    String zkPath = scope.getZkPath();
+
+    if (overwrite) {
+      ZKUtil.createOrReplace(_zkClient, zkPath, cloudConfig.getRecord(), true);
+    } else {
+      ZKUtil.createOrUpdate(_zkClient, zkPath, cloudConfig.getRecord(), true, true);
+    }
   }
 
   /**
diff --git a/helix-core/src/main/java/org/apache/helix/cloud/azure/AzureCloudInstanceInformationProcessor.java b/helix-core/src/main/java/org/apache/helix/cloud/azure/AzureCloudInstanceInformationProcessor.java
index 464ff55..f0f75f7 100644
--- a/helix-core/src/main/java/org/apache/helix/cloud/azure/AzureCloudInstanceInformationProcessor.java
+++ b/helix-core/src/main/java/org/apache/helix/cloud/azure/AzureCloudInstanceInformationProcessor.java
@@ -145,15 +145,15 @@ public class AzureCloudInstanceInformationProcessor
         String azureTopology = AzureConstants.AZURE_TOPOLOGY;
         String[] parts = azureTopology.trim().split("/");
         //The hostname will be filled in by each participant
-        String domain = parts[0] + "=" + platformFaultDomain + "," + parts[1] + "=";
+        String domain = parts[1] + "=" + platformFaultDomain + "," + parts[2] + "=";
 
         AzureCloudInstanceInformation.Builder builder = new AzureCloudInstanceInformation.Builder();
-        builder.setInstanceName(vmName).setFaultDomain(domain)
-            .setInstanceSetName(vmssName);
+        builder.setInstanceName(vmName).setFaultDomain(domain).setInstanceSetName(vmssName);
         azureCloudInstanceInformation = builder.build();
       }
     } catch (IOException e) {
-      throw new HelixException(String.format("Error in parsing cloud instance information: {}", response, e));
+      throw new HelixException(
+          String.format("Error in parsing cloud instance information: {}", response, e));
     }
     return azureCloudInstanceInformation;
   }
diff --git a/helix-core/src/main/java/org/apache/helix/model/CloudConfig.java b/helix-core/src/main/java/org/apache/helix/model/CloudConfig.java
index 0fd622b..0c6062d 100644
--- a/helix-core/src/main/java/org/apache/helix/model/CloudConfig.java
+++ b/helix-core/src/main/java/org/apache/helix/model/CloudConfig.java
@@ -66,7 +66,7 @@ public class CloudConfig extends HelixProperty {
    * The constructor from the ZNRecord.
    * @param record
    */
-  private CloudConfig(ZNRecord record) {
+  public CloudConfig(ZNRecord record) {
     super(CLOUD_CONFIG_KW);
     _record.setSimpleFields(record.getSimpleFields());
     _record.setListFields(record.getListFields());
diff --git a/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java b/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java
index 4452c23..e44585f 100644
--- a/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java
+++ b/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java
@@ -300,7 +300,7 @@ public class TestConfigAccessor extends ZkUnitTestBase {
     _clusterSetup.addCluster(clusterName, false, cloudConfigInit);
 
     // Read CloudConfig from Zookeeper and check the content
-    ConfigAccessor _configAccessor = new ConfigAccessor(_gZkClient);
+    ConfigAccessor _configAccessor = new ConfigAccessor(ZK_ADDR);
     CloudConfig cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
     Assert.assertTrue(cloudConfigFromZk.isCloudEnabled());
     Assert.assertEquals(cloudConfigFromZk.getCloudID(), "TestCloudID");
@@ -310,10 +310,10 @@ public class TestConfigAccessor extends ZkUnitTestBase {
     Assert.assertEquals(cloudConfigFromZk.getCloudProvider(), CloudProvider.CUSTOMIZED.name());
 
     // Change the processor name and check if processor name has been changed in Zookeeper.
-    cloudConfigInitBuilder.setCloudInfoProcessorName("TestProcessor2");
-    cloudConfigInit = cloudConfigInitBuilder.build();
-    ZKHelixAdmin admin = new ZKHelixAdmin(_gZkClient);
-    admin.addCloudConfig(clusterName, cloudConfigInit);
+    CloudConfig.Builder cloudConfigToUpdateBuilder = new CloudConfig.Builder();
+    cloudConfigToUpdateBuilder.setCloudInfoProcessorName("TestProcessor2");
+    CloudConfig cloudConfigToUpdate = cloudConfigToUpdateBuilder.build();
+    _configAccessor.updateCloudConfig(clusterName, cloudConfigToUpdate);
 
     cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
     Assert.assertTrue(cloudConfigFromZk.isCloudEnabled());
@@ -323,4 +323,50 @@ public class TestConfigAccessor extends ZkUnitTestBase {
     Assert.assertEquals(cloudConfigFromZk.getCloudInfoProcessorName(), "TestProcessor2");
     Assert.assertEquals(cloudConfigFromZk.getCloudProvider(), CloudProvider.CUSTOMIZED.name());
   }
+
+  @Test
+  public void testDeleteCloudConfig() throws Exception {
+    ClusterSetup _clusterSetup = new ClusterSetup(ZK_ADDR);
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+
+    CloudConfig.Builder cloudConfigInitBuilder = new CloudConfig.Builder();
+    cloudConfigInitBuilder.setCloudEnabled(true);
+    cloudConfigInitBuilder.setCloudID("TestCloudID");
+    List<String> sourceList = new ArrayList<String>();
+    sourceList.add("TestURL");
+    cloudConfigInitBuilder.setCloudInfoSources(sourceList);
+    cloudConfigInitBuilder.setCloudInfoProcessorName("TestProcessor");
+    cloudConfigInitBuilder.setCloudProvider(CloudProvider.AZURE);
+    CloudConfig cloudConfigInit = cloudConfigInitBuilder.build();
+
+    _clusterSetup.addCluster(clusterName, false, cloudConfigInit);
+
+    // Read CloudConfig from Zookeeper and check the content
+    ConfigAccessor _configAccessor = new ConfigAccessor(ZK_ADDR);
+    CloudConfig cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
+    Assert.assertTrue(cloudConfigFromZk.isCloudEnabled());
+    Assert.assertEquals(cloudConfigFromZk.getCloudID(), "TestCloudID");
+    List<String> listUrlFromZk = cloudConfigFromZk.getCloudInfoSources();
+    Assert.assertEquals(listUrlFromZk.get(0), "TestURL");
+    Assert.assertEquals(cloudConfigFromZk.getCloudInfoProcessorName(), "TestProcessor");
+    Assert.assertEquals(cloudConfigFromZk.getCloudProvider(), CloudProvider.AZURE.name());
+
+    // Change the processor name and check if processor name has been changed in Zookeeper.
+    CloudConfig.Builder cloudConfigBuilderToDelete = new CloudConfig.Builder();
+    cloudConfigBuilderToDelete.setCloudInfoProcessorName("TestProcessor");
+    cloudConfigBuilderToDelete.setCloudID("TestCloudID");
+    CloudConfig cloudConfigToDelete = cloudConfigBuilderToDelete.build();
+
+    _configAccessor.deleteCloudConfigFields(clusterName, cloudConfigToDelete);
+
+    cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
+    Assert.assertTrue(cloudConfigFromZk.isCloudEnabled());
+    Assert.assertNull(cloudConfigFromZk.getCloudID());
+    listUrlFromZk = cloudConfigFromZk.getCloudInfoSources();
+    Assert.assertEquals(listUrlFromZk.get(0), "TestURL");
+    Assert.assertNull(cloudConfigFromZk.getCloudInfoProcessorName());
+    Assert.assertEquals(cloudConfigFromZk.getCloudProvider(), CloudProvider.AZURE.name());
+  }
 }
diff --git a/helix-core/src/test/java/org/apache/helix/cloud/TestAzureCloudInstanceInformationProcessor.java b/helix-core/src/test/java/org/apache/helix/cloud/TestAzureCloudInstanceInformationProcessor.java
index 10ba42d..350c9a9 100644
--- a/helix-core/src/test/java/org/apache/helix/cloud/TestAzureCloudInstanceInformationProcessor.java
+++ b/helix-core/src/test/java/org/apache/helix/cloud/TestAzureCloudInstanceInformationProcessor.java
@@ -55,12 +55,12 @@ public class TestAzureCloudInstanceInformationProcessor extends MockHttpClient {
     // Verify the response from mock http client
     AzureCloudInstanceInformation azureCloudInstanceInformation =
         processor.parseCloudInstanceInformation(response);
-    Assert.assertEquals(azureCloudInstanceInformation
-        .get(CloudInstanceInformation.CloudInstanceField.FAULT_DOMAIN.name()), "2");
     Assert.assertEquals(
         azureCloudInstanceInformation
-            .get(CloudInstanceInformation.CloudInstanceField.INSTANCE_SET_NAME.name()),
-        "test-helix");
+            .get(CloudInstanceInformation.CloudInstanceField.FAULT_DOMAIN.name()),
+        "faultDomain=2," + "hostname=");
+    Assert.assertEquals(azureCloudInstanceInformation
+        .get(CloudInstanceInformation.CloudInstanceField.INSTANCE_SET_NAME.name()), "test-helix");
     Assert.assertEquals(
         azureCloudInstanceInformation
             .get(CloudInstanceInformation.CloudInstanceField.INSTANCE_NAME.name()),
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
index 3966d7c..95fb534 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/ClusterAccessor.java
@@ -733,6 +733,7 @@ public class ClusterAccessor extends AbstractHelixResource {
       return notFound();
     }
 
+    ConfigAccessor configAccessor = new ConfigAccessor(zkClient);
     // Here to update cloud config
     Command command;
     if (commandStr == null || commandStr.isEmpty()) {
@@ -745,22 +746,24 @@ public class ClusterAccessor extends AbstractHelixResource {
       }
     }
 
-    HelixAdmin admin = getHelixAdmin();
-
     ZNRecord record;
+    CloudConfig cloudConfig;
     try {
       record = toZNRecord(content);
+      cloudConfig = new CloudConfig(record);
     } catch (IOException e) {
       LOG.error("Failed to deserialize user's input " + content + ", Exception: " + e);
       return badRequest("Input is not a vaild ZNRecord!");
     }
     try {
       switch (command) {
+      case delete: {
+        configAccessor.deleteCloudConfigFields(clusterId, cloudConfig);
+      }
+      break;
       case update: {
         try {
-          CloudConfig cloudConfig = new CloudConfig.Builder(record).build();
-          admin.removeCloudConfig(clusterId);
-          admin.addCloudConfig(clusterId, cloudConfig);
+          configAccessor.updateCloudConfig(clusterId, cloudConfig);
         } catch (HelixException ex) {
           LOG.error("Error in updating a CloudConfig to cluster: " + clusterId, ex);
           return badRequest(ex.getMessage());
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java
index 78a7a2c..9e862a8 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestClusterAccessor.java
@@ -871,7 +871,7 @@ public class TestClusterAccessor extends AbstractTestClass {
     String className = TestHelper.getTestClassName();
     String methodName = TestHelper.getTestMethodName();
     String clusterName = className + "_" + methodName;
-    
+
     ZNRecord record = new ZNRecord("testZnode");
     record.setBooleanField(CloudConfig.CloudConfigProperty.CLOUD_ENABLED.name(), true);
     record.setSimpleField(CloudConfig.CloudConfigProperty.CLOUD_ID.name(), "TestCloudID");
@@ -888,7 +888,6 @@ public class TestClusterAccessor extends AbstractTestClass {
     CloudConfig cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
     Assert.assertNotNull(cloudConfigFromZk);
     String urlBase = "clusters/" + clusterName + "/cloudconfig/";
-
     delete(urlBase, Response.Status.OK.getStatusCode());
 
     // Read CloudConfig from Zookeeper and make sure it has been removed
@@ -899,7 +898,54 @@ public class TestClusterAccessor extends AbstractTestClass {
     System.out.println("End test :" + TestHelper.getTestMethodName());
   }
 
+
   @Test(dependsOnMethods = "testDeleteCloudConfig")
+  public void testPartialDeleteCloudConfig() throws IOException {
+    System.out.println("Start test :" + TestHelper.getTestMethodName());
+    String className = TestHelper.getTestClassName();
+    String methodName = TestHelper.getTestMethodName();
+    String clusterName = className + "_" + methodName;
+
+
+    ZNRecord record = new ZNRecord(clusterName);
+    record.setBooleanField(CloudConfig.CloudConfigProperty.CLOUD_ENABLED.name(), true);
+    record.setSimpleField(CloudConfig.CloudConfigProperty.CLOUD_PROVIDER.name(),
+        CloudProvider.AZURE.name());
+    record.setSimpleField(CloudConfig.CloudConfigProperty.CLOUD_ID.name(), "TestCloudID");
+    record.setSimpleField(CloudConfig.CloudConfigProperty.CLOUD_INFO_PROCESSOR_NAME.name(), "TestProcessor");
+    _gSetupTool.addCluster(clusterName, true, new CloudConfig.Builder(record).build());
+
+    String urlBase = "clusters/" + clusterName +"/cloudconfig/";
+    Map<String, String> map = new HashMap<>();
+    map.put("addCloudConfig", "true");
+    put("clusters/" + clusterName, map,
+        Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE),
+        Response.Status.CREATED.getStatusCode());
+    // Read CloudConfig from Zookeeper and make sure it has been created
+    ConfigAccessor _configAccessor = new ConfigAccessor(ZK_ADDR);
+    CloudConfig cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
+    Assert.assertNotNull(cloudConfigFromZk);
+
+    record = new ZNRecord(clusterName);
+    Map<String, String> map1 = new HashMap<>();
+    map1.put("command",  Command.delete.name());
+    record.setSimpleField(CloudConfig.CloudConfigProperty.CLOUD_ID.name(), "TestCloudID");
+    record.setSimpleField(CloudConfig.CloudConfigProperty.CLOUD_PROVIDER.name(), CloudProvider.AZURE.name());
+    post(urlBase, map1, Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE),
+        Response.Status.OK.getStatusCode());
+
+    // Read CloudConfig from Zookeeper and make sure it has been removed
+    _configAccessor = new ConfigAccessor(ZK_ADDR);
+    cloudConfigFromZk = _configAccessor.getCloudConfig(clusterName);
+    Assert.assertNull(cloudConfigFromZk.getCloudID());
+    Assert.assertNull(cloudConfigFromZk.getCloudProvider());
+    Assert.assertTrue(cloudConfigFromZk.isCloudEnabled());
+    Assert.assertEquals(cloudConfigFromZk.getCloudInfoProcessorName(),"TestProcessor");
+
+    System.out.println("End test :" + TestHelper.getTestMethodName());
+  }
+
+  @Test(dependsOnMethods = "testPartialDeleteCloudConfig")
   public void testUpdateCloudConfig() throws IOException {
     System.out.println("Start test :" + TestHelper.getTestMethodName());
     _gSetupTool.addCluster("TestCloud", true);