AMBARI-15336. Blueprints: NullPointerException when unncessary config types found with %HOSTGROUP% tags. (Sebastian Toader via magyari_sandor)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/664ccd18 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/664ccd18 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/664ccd18 Branch: refs/heads/AMBARI-13364 Commit: 664ccd185a3ba7ffb3489e49f527e7aa902e8cd0 Parents: 112d385 Author: Sandor Magyari <smagy...@hortonworks.com> Authored: Wed Mar 9 13:32:43 2016 +0100 Committer: Sandor Magyari <smagy...@hortonworks.com> Committed: Wed Mar 9 14:03:04 2016 +0100 ---------------------------------------------------------------------- .../topology/ClusterConfigurationRequest.java | 41 +++++---- .../ClusterConfigurationRequestTest.java | 88 +++++++++++++++++--- .../ClusterInstallWithoutStartTest.java | 4 +- .../server/topology/TopologyManagerTest.java | 4 +- 4 files changed, 107 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/664ccd18/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java index c9120de..063fd12 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java @@ -75,7 +75,7 @@ public class ClusterConfigurationRequest { // set initial configuration (not topology resolved) this.configurationProcessor = new BlueprintConfigurationProcessor(clusterTopology); this.stackAdvisorBlueprintProcessor = stackAdvisorBlueprintProcessor; - removeOrphanConfigTypes(clusterTopology); + removeOrphanConfigTypes(); if (setInitial) { setConfigurationsOnCluster(clusterTopology, TopologyManager.INITIAL_CONFIG_TAG, Collections.<String>emptySet()); } @@ -84,24 +84,35 @@ public class ClusterConfigurationRequest { /** * Remove config-types, if there is no any services related to them (except cluster-env and global). */ - private void removeOrphanConfigTypes(ClusterTopology clusterTopology) { + private void removeOrphanConfigTypes() { Configuration configuration = clusterTopology.getConfiguration(); + removeOrphanConfigTypes(configuration); + + Map<String, HostGroupInfo> hostGroupInfoMap = clusterTopology.getHostGroupInfo(); + if (MapUtils.isNotEmpty(hostGroupInfoMap)) { + for (Map.Entry<String, HostGroupInfo> hostGroupInfo : hostGroupInfoMap.entrySet()) { + configuration = hostGroupInfo.getValue().getConfiguration(); + + if (configuration != null) { + removeOrphanConfigTypes(configuration); + } + } + } + } + + /** + * Remove config-types from the given configuration if there is no any services related to them (except cluster-env and global). + */ + private void removeOrphanConfigTypes(Configuration configuration) { + Blueprint blueprint = clusterTopology.getBlueprint(); + Collection<String> configTypes = configuration.getAllConfigTypes(); for (String configType : configTypes) { - if (!configType.equals("cluster-env") && !configType.equals("global")) { - String service = clusterTopology.getBlueprint().getStack().getServiceForConfigType(configType); - if (!clusterTopology.getBlueprint().getServices().contains(service)) { + if (!"cluster-env".equals(configType) && !"global".equals(configType)) { + String service = blueprint.getStack().getServiceForConfigType(configType); + if (!blueprint.getServices().contains(service)) { configuration.removeConfigType(configType); - LOG.info("Not found any service for config type '{}'. It will be removed from configuration.", configType); - Map<String, HostGroupInfo> hostGroupInfoMap = clusterTopology.getHostGroupInfo(); - if (MapUtils.isNotEmpty(hostGroupInfoMap)) { - for (Map.Entry<String, HostGroupInfo> hostGroupInfo : hostGroupInfoMap.entrySet()) { - if (hostGroupInfo.getValue().getConfiguration() != null) { - hostGroupInfo.getValue().getConfiguration().removeConfigType(configType); - LOG.info("Not found any service for config type '{}'. It will be removed from host group scoped configuration.", configType); - } - } - } + LOG.info("Removing config type '{}' as service '{}' is not present in either Blueprint or cluster creation template.", configType, service); } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/664ccd18/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java index ece1287..58919b9 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java @@ -217,7 +217,7 @@ public class ClusterConfigurationRequestTest { expect(topology.getConfigRecommendationStrategy()).andReturn(ConfigRecommendationStrategy.NEVER_APPLY).anyTimes(); expect(topology.getBlueprint()).andReturn(blueprint).anyTimes(); expect(topology.getConfiguration()).andReturn(blueprintConfig).anyTimes(); - expect(topology.getHostGroupInfo()).andReturn(Collections.<String, HostGroupInfo>emptyMap()); + expect(topology.getHostGroupInfo()).andReturn(Collections.<String, HostGroupInfo>emptyMap()).anyTimes(); expect(topology.getClusterId()).andReturn(Long.valueOf(1)).anyTimes(); expect(ambariContext.getClusterName(Long.valueOf(1))).andReturn("testCluster").anyTimes(); expect(ambariContext.createConfigurationRequests(anyObject(Map.class))).andReturn(Collections @@ -295,7 +295,7 @@ public class ClusterConfigurationRequestTest { expect(topology.getConfigRecommendationStrategy()).andReturn(ConfigRecommendationStrategy.NEVER_APPLY).anyTimes(); expect(topology.getBlueprint()).andReturn(blueprint).anyTimes(); expect(topology.getConfiguration()).andReturn(stackConfig).anyTimes(); - expect(topology.getHostGroupInfo()).andReturn(Collections.<String, HostGroupInfo>emptyMap()); + expect(topology.getHostGroupInfo()).andReturn(Collections.<String, HostGroupInfo>emptyMap()).anyTimes(); expect(topology.getClusterId()).andReturn(Long.valueOf(1)).anyTimes(); expect(ambariContext.getClusterName(Long.valueOf(1))).andReturn("testCluster").anyTimes(); expect(ambariContext.createConfigurationRequests(anyObject(Map.class))).andReturn(Collections @@ -315,7 +315,7 @@ public class ClusterConfigurationRequestTest { } @Test - public void testProcessClusterConfigRequestRemoveUnusedConfigTypes() { + public void testProcessClusterConfigRequestRemoveUnusedConfigTypes() throws Exception { // GIVEN Configuration configuration = createConfigurations(); Set<String> services = new HashSet<String>(); @@ -323,7 +323,7 @@ public class ClusterConfigurationRequestTest { services.add("RANGER"); Map<String, HostGroupInfo> hostGroupInfoMap = Maps.newHashMap(); HostGroupInfo hg1 = new HostGroupInfo("hg1"); - hg1.setConfiguration(createConfigurations()); + hg1.setConfiguration(createConfigurationsForHostGroup()); hostGroupInfoMap.put("hg1", hg1); expect(topology.getConfiguration()).andReturn(configuration).anyTimes(); @@ -339,15 +339,59 @@ public class ClusterConfigurationRequestTest { // WHEN new ClusterConfigurationRequest(ambariContext, topology, false, stackAdvisorBlueprintProcessor); // THEN - assertFalse(configuration.getFullProperties().containsKey("yarn-site")); - assertFalse(configuration.getFullAttributes().containsKey("yarn-site")); - assertTrue(configuration.getFullAttributes().containsKey("hdfs-site")); - assertTrue(configuration.getFullProperties().containsKey("cluster-env")); - assertTrue(configuration.getFullProperties().containsKey("global")); - assertFalse(hg1.getConfiguration().getFullAttributes().containsKey("yarn-site")); + assertFalse("YARN service not present in topology config thus 'yarn-site' config type should be removed from config.", configuration.getFullProperties().containsKey("yarn-site")); + assertTrue("HDFS service is present in topology host group config thus 'hdfs-site' config type should be left in the config.", configuration.getFullAttributes().containsKey("hdfs-site")); + assertTrue("'cluster-env' config type should not be removed from configuration.", configuration.getFullProperties().containsKey("cluster-env")); + assertTrue("'global' config type should not be removed from configuration.", configuration.getFullProperties().containsKey("global")); + + assertFalse("SPARK service not present in topology host group config thus 'spark-env' config type should be removed from config.", hg1.getConfiguration().getFullAttributes().containsKey("spark-env")); + assertTrue("HDFS service is present in topology host group config thus 'hdfs-site' config type should be left in the config.", hg1.getConfiguration().getFullAttributes().containsKey("hdfs-site")); verify(stack, blueprint, topology); } + @Test + public void testProcessClusterConfigRequestWithOnlyHostGroupConfigRemoveUnusedConfigTypes() throws Exception { + // Given + Map<String, Map<String, String>> config = Maps.newHashMap(); + config.put("cluster-env", new HashMap<String, String>()); + config.put("global", new HashMap<String, String>()); + Map<String, Map<String, Map<String, String>>> attributes = Maps.newHashMap(); + + Configuration configuration = new Configuration(config, attributes); + + Set<String> services = new HashSet<>(); + services.add("HDFS"); + services.add("RANGER"); + Map<String, HostGroupInfo> hostGroupInfoMap = Maps.newHashMap(); + HostGroupInfo hg1 = new HostGroupInfo("hg1"); + hg1.setConfiguration(createConfigurationsForHostGroup()); + hostGroupInfoMap.put("hg1", hg1); + + expect(topology.getConfiguration()).andReturn(configuration).anyTimes(); + expect(topology.getBlueprint()).andReturn(blueprint).anyTimes(); + expect(topology.getHostGroupInfo()).andReturn(hostGroupInfoMap); + expect(blueprint.getStack()).andReturn(stack).anyTimes(); + expect(blueprint.getServices()).andReturn(services).anyTimes(); + expect(stack.getServiceForConfigType("hdfs-site")).andReturn("HDFS").anyTimes(); + expect(stack.getServiceForConfigType("admin-properties")).andReturn("RANGER").anyTimes(); + expect(stack.getServiceForConfigType("yarn-site")).andReturn("YARN").anyTimes(); + + EasyMock.replay(stack, blueprint, topology); + + // When + + new ClusterConfigurationRequest(ambariContext, topology, false, stackAdvisorBlueprintProcessor); + + // Then + assertTrue("'cluster-env' config type should not be removed from configuration.", configuration.getFullProperties().containsKey("cluster-env")); + assertTrue("'global' config type should not be removed from configuration.", configuration.getFullProperties().containsKey("global")); + + assertFalse("SPARK service not present in topology host group config thus 'spark-env' config type should be removed from config.", hg1.getConfiguration().getFullAttributes().containsKey("spark-env")); + assertTrue("HDFS service is present in topology host group config thus 'hdfs-site' config type should be left in the config.", hg1.getConfiguration().getFullAttributes().containsKey("hdfs-site")); + verify(stack, blueprint, topology); + + } + private Configuration createConfigurations() { Map<String, Map<String, String>> firstLevelConfig = Maps.newHashMap(); firstLevelConfig.put("hdfs-site", new HashMap<String, String>()); @@ -362,9 +406,31 @@ public class ClusterConfigurationRequestTest { secondLevelConfig.put("admin-properties", new HashMap<String, String>()); Map<String, Map<String, Map<String, String>>> secondLevelAttributes = Maps.newHashMap(); secondLevelAttributes.put("admin-properties", new HashMap<String, Map<String, String>>()); - secondLevelAttributes.put("yarn-site", new HashMap<String, Map<String, String>>()); + Configuration secondLevelConf = new Configuration(secondLevelConfig, secondLevelAttributes); return new Configuration(firstLevelConfig, firstLevelAttributes, secondLevelConf); } + + private Configuration createConfigurationsForHostGroup() { + Map<String, Map<String, String>> firstLevelConfig = Maps.newHashMap(); + firstLevelConfig.put("hdfs-site", new HashMap<String, String>()); + firstLevelConfig.put("spark-env", new HashMap<String, String>()); + firstLevelConfig.put("cluster-env", new HashMap<String, String>()); + firstLevelConfig.put("global", new HashMap<String, String>()); + + Map<String, Map<String, Map<String, String>>> firstLevelAttributes = Maps.newHashMap(); + firstLevelAttributes.put("hdfs-site", new HashMap<String, Map<String, String>>()); + + Map<String, Map<String, String>> secondLevelConfig = Maps.newHashMap(); + secondLevelConfig.put("admin-properties", new HashMap<String, String>()); + Map<String, Map<String, Map<String, String>>> secondLevelAttributes = Maps.newHashMap(); + secondLevelAttributes.put("admin-properties", new HashMap<String, Map<String, String>>()); + + + Configuration secondLevelConf = new Configuration(secondLevelConfig, secondLevelAttributes); + return new Configuration(firstLevelConfig, firstLevelAttributes, secondLevelConf); + } + + } http://git-wip-us.apache.org/repos/asf/ambari/blob/664ccd18/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java index 156580a..6e7c975 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java @@ -251,8 +251,8 @@ public class ClusterInstallWithoutStartTest { expect(stack.getConfiguration()).andReturn(stackConfig).anyTimes(); expect(stack.getName()).andReturn(STACK_NAME).anyTimes(); expect(stack.getVersion()).andReturn(STACK_VERSION).anyTimes(); - expect(stack.getServiceForConfigType("service1-site")).andReturn("service1"); - expect(stack.getServiceForConfigType("service2-site")).andReturn("service2"); + expect(stack.getServiceForConfigType("service1-site")).andReturn("service1").anyTimes(); + expect(stack.getServiceForConfigType("service2-site")).andReturn("service2").anyTimes(); expect(stack.getExcludedConfigurationTypes("service1")).andReturn(Collections.<String>emptySet()).anyTimes(); expect(stack.getExcludedConfigurationTypes("service2")).andReturn(Collections.<String>emptySet()).anyTimes(); http://git-wip-us.apache.org/repos/asf/ambari/blob/664ccd18/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java index 69c1935..91f4993 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java @@ -228,8 +228,8 @@ public class TopologyManagerTest { expect(stack.getComponents()).andReturn(serviceComponents).anyTimes(); expect(stack.getComponents("service1")).andReturn(serviceComponents.get("service1")).anyTimes(); expect(stack.getComponents("service2")).andReturn(serviceComponents.get("service2")).anyTimes(); - expect(stack.getServiceForConfigType("service1-site")).andReturn("service1"); - expect(stack.getServiceForConfigType("service2-site")).andReturn("service2"); + expect(stack.getServiceForConfigType("service1-site")).andReturn("service1").anyTimes(); + expect(stack.getServiceForConfigType("service2-site")).andReturn("service2").anyTimes(); expect(stack.getConfiguration()).andReturn(stackConfig).anyTimes(); expect(stack.getName()).andReturn(STACK_NAME).anyTimes(); expect(stack.getVersion()).andReturn(STACK_VERSION).anyTimes();