You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by bh...@apache.org on 2016/03/02 23:27:08 UTC
ambari git commit: AMBARI-15254: Ambari config update does not handle
removals properly (ljain via bhuvnesh2703)
Repository: ambari
Updated Branches:
refs/heads/trunk e347aef42 -> f707cb50f
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/f707cb50
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/f707cb50
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/f707cb50
Branch: refs/heads/trunk
Commit: f707cb50fd0f5616c74ff8b2f15917a6eda608c2
Parents: e347aef
Author: Bhuvnesh Chaudhary <bc...@pivotal.io>
Authored: Wed Mar 2 14:33:07 2016 -0800
Committer: Bhuvnesh Chaudhary <bc...@pivotal.io>
Committed: Wed Mar 2 14:33:07 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/f707cb50/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 6172fad..77e36c8 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
@@ -684,42 +684,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/f707cb50/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 9fe0fc3..f887c3a 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
@@ -659,6 +659,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>();