This is an automated email from the ASF dual-hosted git repository. jiajunwang pushed a commit to branch helix-0.9.x in repository https://gitbox.apache.org/repos/asf/helix.git
commit 980fbd1487d27b9f2cf223d4938b1c429e5fbc4f Author: Ali Reza Zamani Zadeh Najari <[email protected]> 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 f1e3ffa..da6edda 100644 --- a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java +++ b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java @@ -582,7 +582,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 79c7330..89fcd43 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 @@ -64,7 +64,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 d59beb6..02b5a45 100644 --- a/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java +++ b/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java @@ -216,7 +216,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"); @@ -226,10 +226,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()); @@ -239,4 +239,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 4f5bb20..f8e1d86 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 @@ -604,6 +604,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()) { @@ -616,22 +617,24 @@ public class ClusterAccessor extends AbstractHelixResource { } } - HelixAdmin admin = getHelixAdmin(); - ZNRecord record; + CloudConfig cloudConfig; try { record = toZNRecord(content); + cloudConfig = new CloudConfig(record); } catch (IOException e) { _logger.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) { _logger.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 5bb29ce..bcf0d0c 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 @@ -796,7 +796,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"); @@ -813,7 +813,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 @@ -824,7 +823,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);
