Repository: ambari Updated Branches: refs/heads/branch-2.1 d9f600e23 -> 0293d4cf0 refs/heads/trunk 1c9df2d3f -> 7c2dd3944
AMBARI-13075. Make ambari-server robust/debuggable if user accidentally adds a config type to a config groups when it does not exist as base type (aonishuk) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/7c2dd394 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/7c2dd394 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/7c2dd394 Branch: refs/heads/trunk Commit: 7c2dd39448643c428e9deb9decc47bec73937484 Parents: 1c9df2d Author: Andrew Onishuk <[email protected]> Authored: Fri Sep 11 17:30:02 2015 +0300 Committer: Andrew Onishuk <[email protected]> Committed: Fri Sep 11 17:30:02 2015 +0300 ---------------------------------------------------------------------- .../internal/ConfigGroupResourceProvider.java | 12 +++ .../org/apache/ambari/server/state/Cluster.java | 6 ++ .../server/state/cluster/ClusterImpl.java | 21 ++++ .../ambari/server/state/host/HostImpl.java | 7 +- .../ConfigGroupResourceProviderTest.java | 106 +++++++++++++++++++ 5 files changed, 151 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/7c2dd394/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java index 30bcc86..2642792 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java @@ -373,6 +373,17 @@ public class ConfigGroupResourceProvider extends return responses; } + private void verifyConfigs(Map<String, Config> configs, String clusterName) throws AmbariException { + Clusters clusters = getManagementController().getClusters(); + for (String key : configs.keySet()) { + if(!clusters.getCluster(clusterName).isConfigTypeExists(key)){ + throw new AmbariException("Trying to add not existent config type to config group:"+ + " configType="+key+ + " cluster="+clusterName); + } + } + } + private void verifyHostList(Cluster cluster, Map<Long, Host> hosts, ConfigGroupRequest request) throws AmbariException { @@ -595,6 +606,7 @@ public class ConfigGroupResourceProvider extends configGroup.setHosts(hosts); // Update Configs + verifyConfigs(request.getConfigs(), request.getClusterName()); configGroup.setConfigurations(request.getConfigs()); // Save http://git-wip-us.apache.org/repos/asf/ambari/blob/7c2dd394/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java index 5209dfb..0f259d5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java @@ -381,6 +381,12 @@ public interface Cluster { Config getDesiredConfigByType(String configType); /** + * Check if config type exists in cluster. + * @param configType the type of configuration + * @return <code>true</code> if config type exists, else - <code>false</code> + */ + boolean isConfigTypeExists(String configType); + /** * Gets the desired configurations for the cluster. * @return a map of type-to-configuration information. */ http://git-wip-us.apache.org/repos/asf/ambari/blob/7c2dd394/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java index a48fb54..4b20cba 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java @@ -2010,6 +2010,11 @@ public class ClusterImpl implements Cluster { c.setServiceName(null); c.setTag(e.getTag()); c.setUser(e.getUser()); + if(!allConfigs.containsKey(e.getType())) { + LOG.error("Config inconsistency exists:" + + " unknown configType=" + e.getType()); + continue; + } c.setVersion(allConfigs.get(e.getType()).get(e.getTag()).getVersion()); map.put(e.getType(), c); @@ -2455,6 +2460,22 @@ public class ClusterImpl implements Cluster { } @Override + public boolean isConfigTypeExists(String configType) { + clusterGlobalLock.readLock().lock(); + try { + for (ClusterConfigMappingEntity e : clusterEntity.getConfigMappingEntities()) { + if (e.getType().equals(configType)) { + return true; + } + } + + return false; + } finally { + clusterGlobalLock.readLock().unlock(); + } + } + + @Override public Map<Long, Map<String, DesiredConfig>> getHostsDesiredConfigs(Collection<Long> hostIds) { if (hostIds == null || hostIds.isEmpty()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/7c2dd394/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java index f7b74b9..d121bb5 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java @@ -1355,7 +1355,12 @@ public class HostImpl implements Host { hostConfig = new HostConfig(); hostConfigMap.put(configType, hostConfig); if (cluster != null) { - hostConfig.setDefaultVersionTag(cluster.getDesiredConfigByType(configType).getTag()); + Config conf = cluster.getDesiredConfigByType(configType); + if(conf == null) + LOG.error("Config inconsistency exists:"+ + " unknown configType="+configType); + else + hostConfig.setDefaultVersionTag(conf.getTag()); } } Config config = configEntry.getValue(); http://git-wip-us.apache.org/repos/asf/ambari/blob/7c2dd394/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java index 753daec..4bf3f15 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java @@ -32,6 +32,7 @@ import org.apache.ambari.server.controller.spi.Request; import org.apache.ambari.server.controller.spi.Resource; import org.apache.ambari.server.controller.spi.ResourceAlreadyExistsException; import org.apache.ambari.server.controller.spi.ResourceProvider; +import org.apache.ambari.server.controller.spi.SystemException; import org.apache.ambari.server.controller.utilities.PredicateBuilder; import org.apache.ambari.server.controller.utilities.PropertyHelper; import org.apache.ambari.server.orm.InMemoryDefaultTestModule; @@ -253,7 +254,111 @@ public class ConfigGroupResourceProviderTest { assertNotNull(exception); assertTrue(exception instanceof ResourceAlreadyExistsException); } + @Test + public void testUpdateConfigGroupWithWrongConfigType() throws Exception { + AmbariManagementController managementController = createMock(AmbariManagementController.class); + RequestStatusResponse response = createNiceMock(RequestStatusResponse.class); + ConfigHelper configHelper = createNiceMock(ConfigHelper.class); + Clusters clusters = createNiceMock(Clusters.class); + Cluster cluster = createNiceMock(Cluster.class); + Host h1 = createNiceMock(Host.class); + Host h2 = createNiceMock(Host.class); + HostEntity hostEntity1 = createMock(HostEntity.class); + HostEntity hostEntity2 = createMock(HostEntity.class); + + final ConfigGroup configGroup = createNiceMock(ConfigGroup.class); + ConfigGroupResponse configGroupResponse = createNiceMock + (ConfigGroupResponse.class); + expect(cluster.isConfigTypeExists("core-site")).andReturn(false).anyTimes(); + expect(managementController.getClusters()).andReturn(clusters).anyTimes(); + expect(managementController.getAuthName()).andReturn("admin").anyTimes(); + expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes(); + expect(clusters.getHost("h1")).andReturn(h1); + expect(clusters.getHost("h2")).andReturn(h2); + expect(hostDAO.findByName("h1")).andReturn(hostEntity1).anyTimes(); + expect(hostDAO.findById(1L)).andReturn(hostEntity1).anyTimes(); + expect(hostDAO.findByName("h2")).andReturn(hostEntity2).anyTimes(); + expect(hostDAO.findById(2L)).andReturn(hostEntity2).anyTimes(); + expect(hostEntity1.getHostId()).andReturn(1L).atLeastOnce(); + expect(hostEntity2.getHostId()).andReturn(2L).atLeastOnce(); + expect(h1.getHostId()).andReturn(1L).anyTimes(); + expect(h2.getHostId()).andReturn(2L).anyTimes(); + + expect(configGroup.getName()).andReturn("test-1").anyTimes(); + expect(configGroup.getId()).andReturn(25L).anyTimes(); + expect(configGroup.getTag()).andReturn("tag-1").anyTimes(); + + expect(configGroup.convertToResponse()).andReturn(configGroupResponse).anyTimes(); + expect(configGroupResponse.getClusterName()).andReturn("Cluster100").anyTimes(); + expect(configGroupResponse.getId()).andReturn(25L).anyTimes(); + + expect(cluster.getConfigGroups()).andStubAnswer(new IAnswer<Map<Long, ConfigGroup>>() { + @Override + public Map<Long, ConfigGroup> answer() throws Throwable { + Map<Long, ConfigGroup> configGroupMap = new HashMap<Long, ConfigGroup>(); + configGroupMap.put(configGroup.getId(), configGroup); + return configGroupMap; + } + }); + + replay(managementController, clusters, cluster, + configGroup, response, configGroupResponse, configHelper, hostDAO, hostEntity1, hostEntity2, h1, h2); + + ResourceProvider provider = getConfigGroupResourceProvider + (managementController); + + Map<String, Object> properties = new LinkedHashMap<String, Object>(); + + Set<Map<String, Object>> hostSet = new HashSet<Map<String, Object>>(); + Map<String, Object> host1 = new HashMap<String, Object>(); + host1.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, "h1"); + hostSet.add(host1); + Map<String, Object> host2 = new HashMap<String, Object>(); + host2.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, "h2"); + hostSet.add(host2); + + Set<Map<String, Object>> configSet = new HashSet<Map<String, Object>>(); + Map<String, String> configMap = new HashMap<String, String>(); + Map<String, Object> configs = new HashMap<String, Object>(); + configs.put("type", "core-site"); + configs.put("tag", "version100"); + configMap.put("key1", "value1"); + configs.put("properties", configMap); + configSet.add(configs); + + properties.put(ConfigGroupResourceProvider + .CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID, "Cluster100"); + properties.put(ConfigGroupResourceProvider.CONFIGGROUP_NAME_PROPERTY_ID, + "test-1"); + properties.put(ConfigGroupResourceProvider.CONFIGGROUP_TAG_PROPERTY_ID, + "tag-1"); + properties.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTS_PROPERTY_ID, + hostSet ); + properties.put(ConfigGroupResourceProvider.CONFIGGROUP_CONFIGS_PROPERTY_ID, + configSet); + + Map<String, String> mapRequestProps = new HashMap<String, String>(); + mapRequestProps.put("context", "Called from a test"); + + Request request = PropertyHelper.getUpdateRequest(properties, mapRequestProps); + + Predicate predicate = new PredicateBuilder().property + (ConfigGroupResourceProvider.CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID).equals + ("Cluster100").and(). + property(ConfigGroupResourceProvider.CONFIGGROUP_ID_PROPERTY_ID).equals + (25L).toPredicate(); + SystemException systemException = null; + try { + provider.updateResources(request, predicate); + } + catch (SystemException e){ + systemException = e; + } + assertNotNull(systemException); + verify(managementController, clusters, cluster, + configGroup, response, configGroupResponse, configHelper, hostDAO, hostEntity1, hostEntity2, h1, h2); + } @Test public void testUpdateConfigGroup() throws Exception { AmbariManagementController managementController = createMock(AmbariManagementController.class); @@ -270,6 +375,7 @@ public class ConfigGroupResourceProviderTest { ConfigGroupResponse configGroupResponse = createNiceMock (ConfigGroupResponse.class); + expect(cluster.isConfigTypeExists("core-site")).andReturn(true).anyTimes(); expect(managementController.getClusters()).andReturn(clusters).anyTimes(); expect(managementController.getAuthName()).andReturn("admin").anyTimes(); expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
