ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bhuvnesh2...@apache.org
Subject ambari git commit: AMBARI-15254: Ambari config update does not handle removals properly (ljain via bhuvnesh2703)
Date Thu, 03 Mar 2016 20:03:10 GMT
Repository: ambari
Updated Branches:
  refs/heads/branch-2.2 b3c3d66a5 -> 91b224e0f


AMBARI-15254: Ambari config update does not handle removals properly (ljain via bhuvnesh2703)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/91b224e0
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/91b224e0
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/91b224e0

Branch: refs/heads/branch-2.2
Commit: 91b224e0f8f3956fd4c7bb08ef21999ec044b7a3
Parents: b3c3d66
Author: Bhuvnesh Chaudhary <bchaudhary@pivotal.io>
Authored: Thu Mar 3 12:03:00 2016 -0800
Committer: Bhuvnesh Chaudhary <bchaudhary@pivotal.io>
Committed: Thu Mar 3 12:03:00 2016 -0800

----------------------------------------------------------------------
 .../ambari/server/state/ConfigHelper.java       | 65 +++++++++++---------
 .../ambari/server/state/ConfigHelperTest.java   | 26 ++++++++
 2 files changed, 62 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/91b224e0/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
index ac36ea9..b21cbbd 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
@@ -683,42 +683,49 @@ public class ConfigHelper {
                                String authenticatedUserName,
                                String serviceVersionNote) throws AmbariException {
 
-    if((configType != null) && (updates != null) && !updates.isEmpty()) {
-      Config oldConfig = cluster.getDesiredConfigByType(configType);
-      Map<String, String> oldConfigProperties;
-      Map<String, String> properties = new HashMap<String, String>();
-      Map<String, Map<String, String>> propertiesAttributes =
-        new HashMap<String, Map<String, String>>();
-
-      if (oldConfig == null) {
-        oldConfigProperties = null;
-      } else {
-        oldConfigProperties = oldConfig.getProperties();
-        if (oldConfigProperties != null) {
-          properties.putAll(oldConfigProperties);
-        }
-        if (oldConfig.getPropertiesAttributes() != null) {
-          propertiesAttributes.putAll(oldConfig.getPropertiesAttributes());
-        }
+    // Nothing to update or remove
+    if (configType == null ||
+      (updates == null || updates.isEmpty()) &&
+      (removals == null || removals.isEmpty())) {
+      return;
+    }
+
+    Config oldConfig = cluster.getDesiredConfigByType(configType);
+    Map<String, String> oldConfigProperties;
+    Map<String, String> properties = new HashMap<String, String>();
+    Map<String, Map<String, String>> propertiesAttributes =
+      new HashMap<String, Map<String, String>>();
+
+    if (oldConfig == null) {
+      oldConfigProperties = null;
+    } else {
+      oldConfigProperties = oldConfig.getProperties();
+      if (oldConfigProperties != null) {
+        properties.putAll(oldConfigProperties);
       }
+      if (oldConfig.getPropertiesAttributes() != null) {
+        propertiesAttributes.putAll(oldConfig.getPropertiesAttributes());
+      }
+    }
 
+    if (updates != null) {
       properties.putAll(updates);
+    }
 
-      // Remove properties that need to be removed.
-      if(removals != null) {
-        for (String propertyName : removals) {
-          properties.remove(propertyName);
-          for (Map<String, String> attributesMap: propertiesAttributes.values()) {
-            attributesMap.remove(propertyName);
-          }
+    // Remove properties that need to be removed.
+    if (removals != null) {
+      for (String propertyName : removals) {
+        properties.remove(propertyName);
+        for (Map<String, String> attributesMap: propertiesAttributes.values()) {
+          attributesMap.remove(propertyName);
         }
       }
+    }
 
-      if ((oldConfigProperties == null)
-        || !Maps.difference(oldConfigProperties, properties).areEqual()) {
-        createConfigType(cluster, controller, configType, properties,
-          propertiesAttributes, authenticatedUserName, serviceVersionNote);
-      }
+    if ((oldConfigProperties == null)
+      || !Maps.difference(oldConfigProperties, properties).areEqual()) {
+      createConfigType(cluster, controller, configType, properties,
+        propertiesAttributes, authenticatedUserName, serviceVersionNote);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/ambari/blob/91b224e0/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java
index 4c9bd2e..86b119e 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java
@@ -650,6 +650,32 @@ public class ConfigHelperTest {
     }
 
     @Test
+    public void testUpdateConfigTypeRemovals() throws Exception {
+      Config currentConfig = cluster.getDesiredConfigByType("oozie-site");
+      Map<String, String> properties = currentConfig.getProperties();
+      // Config tag before update
+      Assert.assertEquals("version1", currentConfig.getTag());
+      // Properties before update
+      Assert.assertEquals("simple", properties.get("oozie.authentication.type"));
+      Assert.assertEquals("false", properties.get("oozie.service.HadoopAccessorService.kerberos.enabled"));
+
+      List<String> removals = new ArrayList<String>();
+      removals.add("oozie.authentication.type");
+
+      configHelper.updateConfigType(cluster, managementController, "oozie-site", null, removals,
"admin", "Test note");
+
+      Config updatedConfig = cluster.getDesiredConfigByType("oozie-site");
+      // Config tag updated
+      Assert.assertFalse("version1".equals(updatedConfig.getTag()));
+      // Property removed
+      properties = updatedConfig.getProperties();
+      Assert.assertFalse(properties.containsKey("oozie.authentication.type"));
+      // Property unchanged
+      Assert.assertTrue(properties.containsKey("oozie.service.HadoopAccessorService.kerberos.enabled"));
+      Assert.assertEquals("false", properties.get("oozie.service.HadoopAccessorService.kerberos.enabled"));
+    }
+
+    @Test
     public void testCalculateIsStaleConfigs() throws Exception {
 
       Map<String, HostConfig> schReturn = new HashMap<String, HostConfig>();


Mime
View raw message