Return-Path: X-Original-To: apmail-ambari-commits-archive@www.apache.org Delivered-To: apmail-ambari-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4A4A01836E for ; Fri, 11 Sep 2015 14:30:27 +0000 (UTC) Received: (qmail 3431 invoked by uid 500); 11 Sep 2015 14:30:27 -0000 Delivered-To: apmail-ambari-commits-archive@ambari.apache.org Received: (qmail 3398 invoked by uid 500); 11 Sep 2015 14:30:27 -0000 Mailing-List: contact commits-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: ambari-dev@ambari.apache.org Delivered-To: mailing list commits@ambari.apache.org Received: (qmail 3385 invoked by uid 99); 11 Sep 2015 14:30:27 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 11 Sep 2015 14:30:27 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id E2E11E00C5; Fri, 11 Sep 2015 14:30:26 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: aonishuk@apache.org To: commits@ambari.apache.org Date: Fri, 11 Sep 2015 14:30:27 -0000 Message-Id: <934aab3eafb245bb83498d59f7741f2b@git.apache.org> In-Reply-To: <37ab1213e7624860a20d8aa42d142611@git.apache.org> References: <37ab1213e7624860a20d8aa42d142611@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [2/2] ambari git commit: 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) 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/0293d4cf Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/0293d4cf Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/0293d4cf Branch: refs/heads/branch-2.1 Commit: 0293d4cf03fef6155168f98714b33bea4bc276c8 Parents: d9f600e Author: Andrew Onishuk Authored: Fri Sep 11 17:30:11 2015 +0300 Committer: Andrew Onishuk Committed: Fri Sep 11 17:30:11 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/0293d4cf/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 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 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/0293d4cf/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 916bc49..2f3641d 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 true if config type exists, else - false + */ + 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/0293d4cf/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 4fe24e9..1fece76 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); @@ -2454,6 +2459,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> getHostsDesiredConfigs(Collection hostIds) { if (hostIds == null || hostIds.isEmpty()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/0293d4cf/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 e20cd25..c56d7ce 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 @@ -1346,7 +1346,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/0293d4cf/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>() { + @Override + public Map answer() throws Throwable { + Map configGroupMap = new HashMap(); + 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 properties = new LinkedHashMap(); + + Set> hostSet = new HashSet>(); + Map host1 = new HashMap(); + host1.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, "h1"); + hostSet.add(host1); + Map host2 = new HashMap(); + host2.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, "h2"); + hostSet.add(host2); + + Set> configSet = new HashSet>(); + Map configMap = new HashMap(); + Map configs = new HashMap(); + 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 mapRequestProps = new HashMap(); + 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();