You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ao...@apache.org on 2015/09/11 16:30:27 UTC
[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 <ao...@hortonworks.com>
Authored: Fri Sep 11 17:30:11 2015 +0300
Committer: Andrew Onishuk <ao...@hortonworks.com>
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<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/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 <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/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<Long, Map<String, DesiredConfig>> getHostsDesiredConfigs(Collection<Long> 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<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();