Repository: ambari Updated Branches: refs/heads/trunk 8f2032e59 -> d14f04444
AMBARI-11399 - Merged Stack Configurations Don't Use Overrides On Upgrade (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/d14f0444 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/d14f0444 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/d14f0444 Branch: refs/heads/trunk Commit: d14f044445a2f768ed0e8375279821c3a7daab5c Parents: 8f2032e Author: Jonathan Hurley <[email protected]> Authored: Tue May 26 16:54:51 2015 -0400 Committer: Jonathan Hurley <[email protected]> Committed: Tue May 26 18:39:30 2015 -0400 ---------------------------------------------------------------------- .../internal/UpgradeResourceProvider.java | 114 ++++++++++++------- .../ambari/server/state/ConfigHelper.java | 53 +++++++++ .../internal/UpgradeResourceProviderTest.java | 112 +++++++++++++++++- .../ambari/server/state/ConfigHelperTest.java | 16 +-- 4 files changed, 240 insertions(+), 55 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/d14f0444/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java index a644d78..2a0252e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java @@ -77,7 +77,7 @@ import org.apache.ambari.server.stack.MasterHostResolver; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.ConfigHelper; -import org.apache.ambari.server.state.PropertyInfo; +import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.ServiceInfo; import org.apache.ambari.server.state.StackId; import org.apache.ambari.server.state.StackInfo; @@ -637,7 +637,10 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider * then this will perform the following: * <ul> * <li>Upgrade: Create new configurations that are a merge between the current - * stack and the desired stack.</li> + * stack and the desired stack. If a value has changed between stacks, then + * the target stack value should be taken unless the cluster's value differs + * from the old stack. This can occur if a property has been customized after + * installation.</li> * <li>Downgrade: Reset the latest configurations from the cluster's original * stack. The new configurations that were created on upgrade must be left * intact until all components have been reverted, otherwise heartbeats will @@ -651,7 +654,8 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider * the version * @throws AmbariException */ - private void processConfigurations(Cluster cluster, String version, Direction direction) throws AmbariException { + void processConfigurations(Cluster cluster, String version, Direction direction) + throws AmbariException { RepositoryVersionEntity targetRve = s_repoVersionDAO.findMaxByVersion(version); if (null == targetRve) { LOG.info("Could not find version entity for {}; not setting new configs", @@ -680,52 +684,74 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider break; } - - Map<String, Map<String, String>> clusterConfigs = null; + Map<String, Map<String, String>> newConfigurationsByType = null; ConfigHelper configHelper = getManagementController().getConfigHelper(); if (direction == Direction.UPGRADE) { - clusterConfigs = new HashMap<String, Map<String, String>>(); - - // !!! stack - Set<org.apache.ambari.server.state.PropertyInfo> pi = s_metaProvider.get().getStackProperties( - targetStack.getStackName(), targetStack.getStackVersion()); - - for (PropertyInfo stackProperty : pi) { - String type = ConfigHelper.fileNameToConfigType(stackProperty.getFilename()); - - if (!clusterConfigs.containsKey(type)) { - clusterConfigs.put(type, new HashMap<String, String>()); + // populate a map of default configurations for the old stack (this is + // used when determining if a property has been customized and should be + // overriden with the new stack value) + Map<String, Map<String, String>> oldStackDefaultConfigurationsByType = configHelper.getDefaultProperties( + currentStackId, cluster); + + // populate a map with default configurations from the new stack + newConfigurationsByType = configHelper.getDefaultProperties(targetStackId, cluster); + + // now that the map has been populated with the default configurations + // from the stack/service, overlay the existing configurations on top + Map<String, DesiredConfig> existingDesiredConfigurationsByType = cluster.getDesiredConfigs(); + for (Map.Entry<String, DesiredConfig> existingEntry : existingDesiredConfigurationsByType.entrySet()) { + String configurationType = existingEntry.getKey(); + + // NPE sanity, althought shouldn't even happen since we are iterating + // over the desired configs to start with + Config currentClusterConfig = cluster.getDesiredConfigByType(configurationType); + if (null == currentClusterConfig) { + continue; } - clusterConfigs.get(type).put(stackProperty.getName(), - stackProperty.getValue()); - } - - // !!! by service - for (String serviceName : cluster.getServices().keySet()) { - pi = s_metaProvider.get().getServiceProperties( - targetStack.getStackName(), targetStack.getStackVersion(), - serviceName); + // get the existing configurations + Map<String,String> existingConfigurations = currentClusterConfig.getProperties(); - // !!! use new stack as the basis - for (PropertyInfo stackProperty : pi) { - String type = ConfigHelper.fileNameToConfigType(stackProperty.getFilename()); - - if (!clusterConfigs.containsKey(type)) { - clusterConfigs.put(type, new HashMap<String, String>()); - } - - clusterConfigs.get(type).put(stackProperty.getName(), - stackProperty.getValue()); + // if the new stack configurations don't have the type, then simple add + // all of the existing in + Map<String, String> newDefaultConfigurations = newConfigurationsByType.get(configurationType); + if (null == newDefaultConfigurations) { + newConfigurationsByType.put(configurationType, existingConfigurations); + continue; } - } - // !!! overlay the currently defined values per type - for (Map.Entry<String, Map<String, String>> entry : clusterConfigs.entrySet()) { - Config config = cluster.getDesiredConfigByType(entry.getKey()); - if (null != config) { - entry.getValue().putAll(config.getProperties()); + // for every existing configuration, see if an entry exists; if it does + // not exist, then put it in the map, otherwise we'll have to compare + // the existing value to the original stack value to see if its been + // customized + for (Map.Entry<String, String> existingConfigurationEntry : existingConfigurations.entrySet()) { + String existingConfigurationKey = existingConfigurationEntry.getKey(); + String existingConfigurationValue = existingConfigurationEntry.getValue(); + + // if there is already an entry, we now have to try to determine if + // the value was customized after stack installation + if (newDefaultConfigurations.containsKey(existingConfigurationKey)) { + String newDefaultConfigurationValue = newDefaultConfigurations.get(existingConfigurationKey); + if (!StringUtils.equals(existingConfigurationValue, newDefaultConfigurationValue)) { + // the new default is different from the existing cluster value; + // only override the default value if the existing value differs + // from the original stack + Map<String, String> configurationTypeDefaultConfigurations = oldStackDefaultConfigurationsByType.get(configurationType); + if (null != configurationTypeDefaultConfigurations) { + String oldDefaultValue = configurationTypeDefaultConfigurations.get(existingConfigurationKey); + if (!StringUtils.equals(existingConfigurationValue, oldDefaultValue)) { + // at this point, we've determined that there is a difference + // between default values between stacks, but the value was + // also customized, so keep the customized value + newDefaultConfigurations.put(existingConfigurationKey, existingConfigurationValue); + } + } + } + } else { + // there is no entry in the map, so add the existing key/value pair + newDefaultConfigurations.put(existingConfigurationKey, existingConfigurationValue); + } } } } else { @@ -738,9 +764,9 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider targetStack.getStackVersion()), true); // !!! configs must be created after setting the stack version - if (null != clusterConfigs) { - configHelper.createConfigTypes(cluster, getManagementController(), - clusterConfigs, getManagementController().getAuthName(), + if (null != newConfigurationsByType) { + configHelper.createConfigTypes(cluster, getManagementController(), newConfigurationsByType, + getManagementController().getAuthName(), "Configuration created for Upgrade"); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/d14f0444/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 4e2fbcd..ce5b634 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 @@ -867,6 +867,59 @@ public class ConfigHelper { } } + /** + * Gets the default properties from the specified stack and services when a + * cluster is first installed. + * + * @param stack + * the stack to pull stack-values from (not {@code null}) + * @param cluster + * the cluster to use when determining which services default + * configurations to include (not {@code null}). + * @return a mapping of configuration type to map of key/value pairs for the + * default configurations. + * @throws AmbariException + */ + public Map<String, Map<String, String>> getDefaultProperties(StackId stack, Cluster cluster) + throws AmbariException { + Map<String, Map<String, String>> defaultPropertiesByType = new HashMap<String, Map<String, String>>(); + + // populate the stack (non-service related) properties first + Set<org.apache.ambari.server.state.PropertyInfo> stackConfigurationProperties = ambariMetaInfo.getStackProperties( + stack.getStackName(), stack.getStackVersion()); + + for (PropertyInfo stackDefaultProperty : stackConfigurationProperties) { + String type = ConfigHelper.fileNameToConfigType(stackDefaultProperty.getFilename()); + + if (!defaultPropertiesByType.containsKey(type)) { + defaultPropertiesByType.put(type, new HashMap<String, String>()); + } + + defaultPropertiesByType.get(type).put(stackDefaultProperty.getName(), + stackDefaultProperty.getValue()); + } + + // for every installed service, populate the default service properties + for (String serviceName : cluster.getServices().keySet()) { + Set<org.apache.ambari.server.state.PropertyInfo> serviceConfigurationProperties = ambariMetaInfo.getServiceProperties( + stack.getStackName(), stack.getStackVersion(), serviceName); + + // !!! use new stack as the basis + for (PropertyInfo serviceDefaultProperty : serviceConfigurationProperties) { + String type = ConfigHelper.fileNameToConfigType(serviceDefaultProperty.getFilename()); + + if (!defaultPropertiesByType.containsKey(type)) { + defaultPropertiesByType.put(type, new HashMap<String, String>()); + } + + defaultPropertiesByType.get(type).put(serviceDefaultProperty.getName(), + serviceDefaultProperty.getValue()); + } + } + + return defaultPropertiesByType; + } + private boolean calculateIsStaleConfigs(ServiceComponentHost sch) throws AmbariException { if (sch.isRestartRequired()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/d14f0444/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java index 12e6efe..e533736 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java @@ -68,6 +68,7 @@ import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.ConfigHelper; import org.apache.ambari.server.state.ConfigImpl; +import org.apache.ambari.server.state.DesiredConfig; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.HostState; import org.apache.ambari.server.state.RepositoryVersionState; @@ -79,6 +80,7 @@ import org.apache.ambari.server.state.stack.upgrade.Direction; import org.apache.ambari.server.topology.TopologyManager; import org.apache.ambari.server.utils.StageUtils; import org.apache.ambari.server.view.ViewRegistry; +import org.easymock.Capture; import org.easymock.EasyMock; import org.junit.After; import org.junit.Assert; @@ -111,11 +113,18 @@ public class UpgradeResourceProviderTest { public void before() throws Exception { // setup the config helper for placeholder resolution configHelper = EasyMock.createNiceMock(ConfigHelper.class); + expect( configHelper.getPlaceholderValueFromDesiredConfigurations( EasyMock.anyObject(Cluster.class), EasyMock.eq("{{foo/bar}}"))).andReturn( "placeholder-rendered-properly").anyTimes(); + expect( + configHelper.getDefaultProperties(EasyMock.anyObject(StackId.class), + EasyMock.anyObject(Cluster.class))).andReturn( + new HashMap<String, Map<String, String>>()).anyTimes(); + + EasyMock.replay(configHelper); // create an injector which will inject the mocks @@ -692,9 +701,109 @@ public class UpgradeResourceProviderTest { } } } + } + + /** + * Tests merging configurations between existing and new stack values on + * upgrade. + * + * @throws Exception + */ + @Test + public void testMergeConfigurations() throws Exception { + StackId stack211 = new StackId("HDP-2.1.1"); + StackId stack220 = new StackId("HDP-2.2.0"); + + Map<String, Map<String, String>> stack211Configs = new HashMap<String, Map<String, String>>(); + Map<String, String> stack211FooType = new HashMap<String, String>(); + Map<String, String> stack211BarType = new HashMap<String, String>(); + Map<String, String> stack211BazType = new HashMap<String, String>(); + stack211Configs.put("foo-site", stack211FooType); + stack211Configs.put("bar-site", stack211BarType); + stack211Configs.put("baz-site", stack211BazType); + stack211FooType.put("1", "one"); + stack211FooType.put("11", "one-one"); + stack211BarType.put("2", "two"); + stack211BazType.put("3", "three"); + + Map<String, Map<String, String>> stack220Configs = new HashMap<String, Map<String, String>>(); + Map<String, String> stack220FooType = new HashMap<String, String>(); + Map<String, String> stack220BazType = new HashMap<String, String>(); + stack220Configs.put("foo-site", stack220FooType); + stack220Configs.put("baz-site", stack220BazType); + stack220FooType.put("1", "one-new"); + stack220FooType.put("111", "one-one-one"); + stack220BazType.put("3", "three-new"); + + Map<String, String> clusterFooType = new HashMap<String, String>(); + Map<String, String> clusterBarType = new HashMap<String, String>(); + Map<String, String> clusterBazType = new HashMap<String, String>(); + + Config fooConfig = EasyMock.createNiceMock(Config.class); + Config barConfig = EasyMock.createNiceMock(Config.class); + Config bazConfig = EasyMock.createNiceMock(Config.class); + + clusterFooType.put("1", "one"); + clusterFooType.put("11", "one-one"); + clusterBarType.put("2", "two"); + clusterBazType.put("3", "three-changed"); + + expect(fooConfig.getProperties()).andReturn(clusterFooType); + expect(barConfig.getProperties()).andReturn(clusterBarType); + expect(bazConfig.getProperties()).andReturn(clusterBazType); + + Map<String, DesiredConfig> desiredConfigurations = new HashMap<String, DesiredConfig>(); + desiredConfigurations.put("foo-site", null); + desiredConfigurations.put("bar-site", null); + desiredConfigurations.put("baz-site", null); + + Cluster cluster = EasyMock.createNiceMock(Cluster.class); + expect(cluster.getCurrentStackVersion()).andReturn(stack211); + expect(cluster.getDesiredStackVersion()).andReturn(stack220); + expect(cluster.getDesiredConfigs()).andReturn(desiredConfigurations); + expect(cluster.getDesiredConfigByType("foo-site")).andReturn(fooConfig); + expect(cluster.getDesiredConfigByType("bar-site")).andReturn(barConfig); + expect(cluster.getDesiredConfigByType("baz-site")).andReturn(bazConfig); + + // setup the config helper for placeholder resolution + EasyMock.reset(configHelper); + expect( + configHelper.getDefaultProperties(EasyMock.eq(stack211), EasyMock.anyObject(Cluster.class))).andReturn( + stack211Configs).anyTimes(); + + expect( + configHelper.getDefaultProperties(EasyMock.eq(stack220), EasyMock.anyObject(Cluster.class))).andReturn( + stack220Configs).anyTimes(); + + Capture<Map<String, Map<String, String>>> expectedConfigurationsCapture = new Capture<Map<String, Map<String, String>>>(); + + configHelper.createConfigTypes(EasyMock.anyObject(Cluster.class), + EasyMock.anyObject(AmbariManagementController.class), + EasyMock.capture(expectedConfigurationsCapture), + EasyMock.anyObject(String.class), EasyMock.anyObject(String.class)); + + EasyMock.expectLastCall().once(); + EasyMock.replay(configHelper, cluster, fooConfig, barConfig, bazConfig); + UpgradeResourceProvider upgradeResourceProvider = createProvider(amc); + + upgradeResourceProvider.processConfigurations(cluster, "2.2.0.0", Direction.UPGRADE); + + Map<String, Map<String, String>> expectedConfigurations = expectedConfigurationsCapture.getValue(); + Map<String, String> expectedFooType = expectedConfigurations.get("foo-site"); + Map<String, String> expectedBarType = expectedConfigurations.get("bar-site"); + Map<String, String> expectedBazType = expectedConfigurations.get("baz-site"); + + // the really important values are one-new and three-changed; one-new + // indicates that the new stack value is changed since it was not customized + // while three-changed represents that the customized value was preserved + // even though the stack value changed + assertEquals("one-new", expectedFooType.get("1")); + assertEquals("one-one", expectedFooType.get("11")); + assertEquals("two", expectedBarType.get("2")); + assertEquals("three-changed", expectedBazType.get("3")); } /** @@ -717,7 +826,4 @@ public class UpgradeResourceProviderTest { binder.bind(ConfigHelper.class).toInstance(configHelper); } } - - - } http://git-wip-us.apache.org/repos/asf/ambari/blob/d14f0444/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 7fb8f66..1376d29 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 @@ -377,7 +377,7 @@ public class ConfigHelperTest { } @Test - public void testCloneAttributesMap_sourceIsNull() throws Exception { + public void testCloneAttributesMapSourceIsNull() throws Exception { // init Map<String, Map<String, String>> targetAttributesMap = new HashMap<String, Map<String, String>>(); Map<String, String> attributesValues = new HashMap<String, String>(); @@ -405,7 +405,7 @@ public class ConfigHelperTest { } @Test - public void testCloneAttributesMap_targetIsNull() throws Exception { + public void testCloneAttributesMapTargetIsNull() throws Exception { // init Map<String, Map<String, String>> targetAttributesMap = null; Map<String, Map<String, String>> sourceAttributesMap = new HashMap<String, Map<String, String>>(); @@ -469,7 +469,7 @@ public class ConfigHelperTest { } @Test - public void testMergeAttributes_noAttributeOverrides() throws Exception { + public void testMergeAttributesWithNoAttributeOverrides() throws Exception { Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>(); Map<String, String> persistedFinalAttrs = new HashMap<String, String>(); persistedFinalAttrs.put("a", "true"); @@ -496,7 +496,7 @@ public class ConfigHelperTest { } @Test - public void testMergeAttributes_nullAttributes() throws Exception { + public void testMergeAttributesWithNullAttributes() throws Exception { Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>(); Map<String, String> persistedFinalAttrs = new HashMap<String, String>(); persistedFinalAttrs.put("a", "true"); @@ -524,7 +524,7 @@ public class ConfigHelperTest { } @Test - public void testMergeAttributes_nullProperties() throws Exception { + public void testMergeAttributesWithNullProperties() throws Exception { Map<String, Map<String, String>> persistedAttributes = new HashMap<String, Map<String, String>>(); Map<String, String> persistedFinalAttrs = new HashMap<String, String>(); persistedFinalAttrs.put("a", "true"); @@ -620,7 +620,7 @@ public class ConfigHelperTest { } @Test - public void testGetServiceProperties_SimpleInvocation() throws Exception { + public void testGetServicePropertiesSimpleInvocation() throws Exception { Cluster mockCluster = createStrictMock(Cluster.class); StackId mockStackVersion = createStrictMock(StackId.class); AmbariMetaInfo mockAmbariMetaInfo = injector.getInstance(AmbariMetaInfo.class); @@ -654,7 +654,7 @@ public class ConfigHelperTest { } @Test - public void testGetServiceProperties_DoNoRemoveExcluded() throws Exception { + public void testGetServicePropertiesDoNoRemoveExcluded() throws Exception { StackId mockStackVersion = createStrictMock(StackId.class); AmbariMetaInfo mockAmbariMetaInfo = injector.getInstance(AmbariMetaInfo.class); ServiceInfo mockServiceInfo = createStrictMock(ServiceInfo.class); @@ -685,7 +685,7 @@ public class ConfigHelperTest { } @Test - public void testGetServiceProperties_RemoveExcluded() throws Exception { + public void testGetServicePropertiesRemoveExcluded() throws Exception { StackId mockStackVersion = createStrictMock(StackId.class); AmbariMetaInfo mockAmbariMetaInfo = injector.getInstance(AmbariMetaInfo.class); ServiceInfo mockServiceInfo = createStrictMock(ServiceInfo.class);
