AMBARI-7218. rename host group deletes configs (dlysnichenko)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/9ce44c95 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/9ce44c95 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/9ce44c95 Branch: refs/heads/branch-alerts-dev Commit: 9ce44c953de1154883887538c5979e5d9036a30c Parents: daae69e Author: Lisnichenko Dmitro <dlysniche...@hortonworks.com> Authored: Mon Sep 8 21:48:15 2014 +0300 Committer: Lisnichenko Dmitro <dlysniche...@hortonworks.com> Committed: Tue Sep 9 16:16:42 2014 +0300 ---------------------------------------------------------------------- .../internal/ConfigGroupResourceProvider.java | 136 ++++++++++++------- .../ConfigGroupResourceProviderTest.java | 130 ++++++++++++++++++ 2 files changed, 217 insertions(+), 49 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/9ce44c95/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 6e13d9c..52df317 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 @@ -383,6 +383,36 @@ public class ConfigGroupResourceProvider extends cluster.deleteConfigGroup(request.getId()); } + private void basicRequestValidation(ConfigGroupRequest request) { + if (request.getId() == null + || request.getClusterName() == null + || request.getClusterName().isEmpty() + || request.getGroupName() == null + || request.getGroupName().isEmpty()) { + LOG.debug("Received a config group request with request id = " + + request.getId() + ", cluster name = " + + request.getClusterName() + ", group name = " + request.getGroupName()); + throw new IllegalArgumentException("Request id, " + + "cluster name and " + + "group name have to be provided."); + } + } + + private void validateRenameRequest(ConfigGroupRequest request) { + if (request.getTag() != null + || (request.getHosts() != null && ! request.getHosts().isEmpty()) + || request.getDescription() != null + || request.getServiceConfigVersionNote() != null + || (request.getConfigs()!=null && ! request.getConfigs().isEmpty())) { + throw new IllegalArgumentException("Request with id " + + request.getId() + + " seems to be a config group rename request. " + + "Renaming config group can not be combined with other " + + "operations, so hosts, configs, description, service config version note " + + "request fields should not be populated."); + } + } + private void validateRequest(ConfigGroupRequest request) { if (request.getClusterName() == null || request.getClusterName().isEmpty() @@ -504,81 +534,89 @@ public class ConfigGroupResourceProvider extends Clusters clusters = getManagementController().getClusters(); for (ConfigGroupRequest request : requests) { + basicRequestValidation(request); Cluster cluster; try { cluster = clusters.getCluster(request.getClusterName()); } catch (ClusterNotFoundException e) { throw new ParentObjectNotFoundException( - "Attempted to add a config group to a cluster which doesn't exist", e); + String.format( + "The cluster %s does not exist, can not update a config group", + request.getClusterName()), e); } - if (request.getId() == null) { - throw new AmbariException("Config group Id is a required parameter."); - } - - validateRequest(request); - // Find config group - ConfigGroup configGroup = cluster.getConfigGroups().get(request.getId()); + Map<Long, ConfigGroup> configGroups = cluster.getConfigGroups(); + ConfigGroup configGroup = configGroups.get(request.getId()); if (configGroup == null) { throw new AmbariException("Config group not found" + ", clusterName = " + request.getClusterName() + ", groupId = " + request.getId()); } - String serviceName = configGroup.getServiceName(); - String requestServiceName = cluster.getServiceForConfigTypes(request.getConfigs().keySet()); - if (serviceName != null && requestServiceName !=null && !StringUtils.equals(serviceName, requestServiceName)) { - throw new IllegalArgumentException("Config group " + configGroup.getId() + - " is mapped to service " + serviceName + ", " + - "but request contain configs from service " + requestServiceName); - } else if (serviceName == null && requestServiceName != null) { - configGroup.setServiceName(requestServiceName); - serviceName = requestServiceName; - } - // Update hosts - Map<String, Host> hosts = new HashMap<String, Host>(); - if (request.getHosts() != null && !request.getHosts().isEmpty()) { - for (String hostname : request.getHosts()) { - Host host = clusters.getHost(hostname); - if (host == null) { - throw new HostNotFoundException(hostname); - } - hosts.put(hostname, host); + if (! configGroup.getName().equals(request.getGroupName())) { + // Looks like user tries to rename a config group + validateRenameRequest(request); + configGroup.setName(request.getGroupName()); + persist(configGroup); + } else { // Any other action + validateRequest(request); + String serviceName = configGroup.getServiceName(); + String requestServiceName = cluster.getServiceForConfigTypes(request.getConfigs().keySet()); + if (serviceName != null && requestServiceName !=null && !StringUtils.equals(serviceName, requestServiceName)) { + throw new IllegalArgumentException("Config group " + configGroup.getId() + + " is mapped to service " + serviceName + ", " + + "but request contain configs from service " + requestServiceName); + } else if (serviceName == null && requestServiceName != null) { + configGroup.setServiceName(requestServiceName); + serviceName = requestServiceName; } - } - - verifyHostList(cluster, hosts, request); - configGroup.setHosts(hosts); + // Update hosts + Map<String, Host> hosts = new HashMap<String, Host>(); + if (request.getHosts() != null && !request.getHosts().isEmpty()) { + for (String hostname : request.getHosts()) { + Host host = clusters.getHost(hostname); + if (host == null) { + throw new HostNotFoundException(hostname); + } + hosts.put(hostname, host); + } + } - // Update Configs - configGroup.setConfigurations(request.getConfigs()); + verifyHostList(cluster, hosts, request); - // Save - configGroup.setName(request.getGroupName()); - configGroup.setDescription(request.getDescription()); - configGroup.setTag(request.getTag()); + configGroup.setHosts(hosts); - configLogger.info("Persisting updated Config group" - + ", clusterName = " + configGroup.getClusterName() - + ", id = " + configGroup.getId() - + ", tag = " + configGroup.getTag() - + ", user = " + getManagementController().getAuthName()); + // Update Configs + configGroup.setConfigurations(request.getConfigs()); + // Save + configGroup.setName(request.getGroupName()); + configGroup.setDescription(request.getDescription()); + configGroup.setTag(request.getTag()); - configGroup.persist(); - if (serviceName != null) { - cluster.createServiceConfigVersion(serviceName, getManagementController().getAuthName(), null, configGroup); - } else { - LOG.warn("Could not determine service name for config group {}, service config version not created", - configGroup.getId()); + persist(configGroup); + if (serviceName != null) { + cluster.createServiceConfigVersion(serviceName, getManagementController().getAuthName(), null, configGroup); + } else { + LOG.warn("Could not determine service name for config group {}, service config version not created", + configGroup.getId()); + } } } - getManagementController().getConfigHelper().invalidateStaleConfigsCache(); } + private void persist(ConfigGroup configGroup) { + configLogger.info("Persisting updated Config group" + + ", clusterName = " + configGroup.getClusterName() + + ", id = " + configGroup.getId() + + ", tag = " + configGroup.getTag() + + ", user = " + getManagementController().getAuthName()); + configGroup.persist(); + } + @SuppressWarnings("unchecked") ConfigGroupRequest getConfigGroupRequest(Map<String, Object> properties) { Object groupIdObj = properties.get(CONFIGGROUP_ID_PROPERTY_ID); http://git-wip-us.apache.org/repos/asf/ambari/blob/9ce44c95/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 4194979..116e435 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 @@ -49,6 +49,7 @@ import java.util.Set; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertTrue; +import static junit.framework.Assert.fail; import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.capture; import static org.easymock.EasyMock.createMock; @@ -56,6 +57,7 @@ import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.reset; import static org.easymock.EasyMock.verify; public class ConfigGroupResourceProviderTest { @@ -305,6 +307,134 @@ public class ConfigGroupResourceProviderTest { configGroup, response, configGroupResponse, configHelper); } + @Test + public void testUpdateConfigGroup_renameGroup() 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); + final ConfigGroup configGroup = createNiceMock(ConfigGroup.class); + ConfigGroupResponse configGroupResponse = createNiceMock + (ConfigGroupResponse.class); + + expect(managementController.getClusters()).andReturn(clusters).anyTimes(); + expect(managementController.getAuthName()).andReturn("admin").anyTimes(); + expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes(); + + expect(configGroup.getName()).andReturn("originalGroupName").anyTimes(); + expect(configGroup.getId()).andReturn(25L).anyTimes(); + expect(configGroup.convertToResponse()).andReturn(configGroupResponse).anyTimes(); + expect(configGroupResponse.getClusterName()).andReturn("Cluster100").anyTimes(); + expect(configGroupResponse.getId()).andReturn(25L).anyTimes(); + + replay(configGroup); + + 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; + } + }); + + + expect(managementController.getConfigHelper()).andReturn(configHelper).once(); + configHelper.invalidateStaleConfigsCache(); + expectLastCall().once(); + + replay(managementController, clusters, cluster, + response, configGroupResponse, configHelper); + + ResourceProvider provider = getConfigGroupResourceProvider + (managementController); + + String newGroupName = "newGroupName"; + /* + * Check rename request validation + */ + 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, + newGroupName); + 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(); + + try { + provider.updateResources(request, predicate); + fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + + verify(configGroup); + + /* + * Check execution of rename request + */ + reset(configGroup); + expect(configGroup.getName()).andReturn("originalGroupName").anyTimes(); + expect(configGroup.getId()).andReturn(25L).anyTimes(); + + expect(configGroup.convertToResponse()).andReturn(configGroupResponse).anyTimes(); + + configGroup.setName(newGroupName); + expectLastCall().once(); + replay(configGroup); + + reset(cluster); + Map<Long, ConfigGroup> configGroupMap = new HashMap<Long, ConfigGroup>(); + configGroupMap.put(configGroup.getId(), configGroup); + expect(cluster.getConfigGroups()).andReturn(configGroupMap).anyTimes(); + replay(cluster); + + properties = new LinkedHashMap<String, Object>(); + properties.put(ConfigGroupResourceProvider + .CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID, "Cluster100"); + properties.put(ConfigGroupResourceProvider.CONFIGGROUP_NAME_PROPERTY_ID, + newGroupName); + request = PropertyHelper.getUpdateRequest(properties, mapRequestProps); + provider.updateResources(request, predicate); + verify(configGroup); + } + @SuppressWarnings("unchecked") @Test public void testGetConfigGroup() throws Exception {
