Repository: ambari Updated Branches: refs/heads/trunk 9cf27bf86 -> 22b1df064
Revert "AMBARI-7218. rename host group deletes configs (dlysnichenko)" This reverts commit 9ce44c953de1154883887538c5979e5d9036a30c. Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/22b1df06 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/22b1df06 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/22b1df06 Branch: refs/heads/trunk Commit: 22b1df06454e32753864ea3a622d9cd5b8d921fa Parents: 9cf27bf Author: Lisnichenko Dmitro <dlysniche...@hortonworks.com> Authored: Wed Sep 10 13:13:46 2014 +0300 Committer: Lisnichenko Dmitro <dlysniche...@hortonworks.com> Committed: Wed Sep 10 13:13:46 2014 +0300 ---------------------------------------------------------------------- .../internal/ConfigGroupResourceProvider.java | 136 +++++++------------ .../ConfigGroupResourceProviderTest.java | 130 ------------------ 2 files changed, 49 insertions(+), 217 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/22b1df06/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 52df317..6e13d9c 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,36 +383,6 @@ 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() @@ -534,87 +504,79 @@ 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( - String.format( - "The cluster %s does not exist, can not update a config group", - request.getClusterName()), e); + "Attempted to add a config group to a cluster which doesn't exist", e); } + if (request.getId() == null) { + throw new AmbariException("Config group Id is a required parameter."); + } + + validateRequest(request); + // Find config group - Map<Long, ConfigGroup> configGroups = cluster.getConfigGroups(); - ConfigGroup configGroup = configGroups.get(request.getId()); + ConfigGroup configGroup = cluster.getConfigGroups().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; + } - 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; - } - - // 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 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); } + } + + verifyHostList(cluster, hosts, request); - verifyHostList(cluster, hosts, request); + configGroup.setHosts(hosts); - configGroup.setHosts(hosts); + // Update Configs + configGroup.setConfigurations(request.getConfigs()); - // Update Configs - configGroup.setConfigurations(request.getConfigs()); - // Save - configGroup.setName(request.getGroupName()); - configGroup.setDescription(request.getDescription()); - configGroup.setTag(request.getTag()); + // Save + configGroup.setName(request.getGroupName()); + configGroup.setDescription(request.getDescription()); + configGroup.setTag(request.getTag()); - 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()); - } + configLogger.info("Persisting updated Config group" + + ", clusterName = " + configGroup.getClusterName() + + ", id = " + configGroup.getId() + + ", tag = " + configGroup.getTag() + + ", user = " + getManagementController().getAuthName()); + + 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()); } } - 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(); + getManagementController().getConfigHelper().invalidateStaleConfigsCache(); } @SuppressWarnings("unchecked") http://git-wip-us.apache.org/repos/asf/ambari/blob/22b1df06/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 116e435..4194979 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,7 +49,6 @@ 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; @@ -57,7 +56,6 @@ 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 { @@ -307,134 +305,6 @@ 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 {
