Repository: ambari Updated Branches: refs/heads/trunk 3c2ec028d -> 0a7b75f0d
AMBARI-4876. Improve error while trying to delete a service. (ncole) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/0a7b75f0 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/0a7b75f0 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/0a7b75f0 Branch: refs/heads/trunk Commit: 0a7b75f0d24458d80f7a2a321342cb47379ec23a Parents: 3c2ec02 Author: Nate Cole <[email protected]> Authored: Thu Feb 27 16:24:18 2014 -0500 Committer: Nate Cole <[email protected]> Committed: Fri Feb 28 10:43:52 2014 -0500 ---------------------------------------------------------------------- .../internal/ServiceResourceProvider.java | 28 +++++- .../nagios/NagiosPropertyProvider.java | 2 +- .../internal/ServiceResourceProviderTest.java | 98 +++++++++++++++++++- 3 files changed, 121 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/0a7b75f0/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java index 8a8b435..1e402eb 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java @@ -759,15 +759,39 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider throws AmbariException { Clusters clusters = getManagementController().getClusters(); - + + Set<Service> removable = new HashSet<Service>(); + for (ServiceRequest serviceRequest : request) { if (StringUtils.isEmpty(serviceRequest.getClusterName()) || StringUtils.isEmpty(serviceRequest.getServiceName())) { // FIXME throw correct error throw new AmbariException("invalid arguments"); } else { - clusters.getCluster(serviceRequest.getClusterName()).deleteService(serviceRequest.getServiceName()); + + Service service = clusters.getCluster( + serviceRequest.getClusterName()).getService( + serviceRequest.getServiceName()); + + if (!service.getDesiredState().isRemovableState()) { + throw new AmbariException("Cannot remove " + service.getName() + ". Desired state " + + service.getDesiredState() + " is not removable. Service must be stopped or disabled."); + } else { + for (ServiceComponent sc : service.getServiceComponents().values()) { + if (!sc.canBeRemoved()) { + throw new AmbariException ("Cannot remove " + + serviceRequest.getClusterName() + "/" + serviceRequest.getServiceName() + + ". " + sc.getName() + " is in a non-removable state."); + } + } + } + + removable.add(service); } } + + for (Service service : removable) + service.getCluster().deleteService(service.getName()); + return null; } http://git-wip-us.apache.org/repos/asf/ambari/blob/0a7b75f0/ambari-server/src/main/java/org/apache/ambari/server/controller/nagios/NagiosPropertyProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/nagios/NagiosPropertyProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/nagios/NagiosPropertyProvider.java index 90c0e4e..f80d4a6 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/nagios/NagiosPropertyProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/nagios/NagiosPropertyProvider.java @@ -310,7 +310,7 @@ public class NagiosPropertyProvider extends BaseProvider implements PropertyProv nagiosHost = hosts.keySet().iterator().next(); } catch (AmbariException e) { - LOG.error("Cannot find a nagios service. Skipping alerts."); + LOG.debug("Cannot find a nagios service. Skipping alerts."); } if (null != nagiosHost) { http://git-wip-us.apache.org/repos/asf/ambari/blob/0a7b75f0/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java index 2bc51bb..634c876 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java @@ -49,6 +49,7 @@ import org.apache.ambari.server.controller.spi.Predicate; import org.apache.ambari.server.controller.spi.Request; import org.apache.ambari.server.controller.spi.Resource; 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.state.Cluster; @@ -405,14 +406,22 @@ public class ServiceResourceProviderTest { AmbariManagementController managementController = createMock(AmbariManagementController.class); Clusters clusters = createNiceMock(Clusters.class); Cluster cluster = createNiceMock(Cluster.class); + Service service = createNiceMock(Service.class); + + String serviceName = "Service100"; // set expectations expect(managementController.getClusters()).andReturn(clusters).anyTimes(); expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes(); - cluster.deleteService("Service100"); + expect(cluster.getService(serviceName)).andReturn(service).anyTimes(); + expect(service.getDesiredState()).andReturn(State.INSTALLED).anyTimes(); + expect(service.getName()).andReturn(serviceName).anyTimes(); + expect(service.getServiceComponents()).andReturn(new HashMap<String, ServiceComponent>()); + expect(service.getCluster()).andReturn(cluster); + cluster.deleteService(serviceName); // replay - replay(managementController, clusters, cluster); + replay(managementController, clusters, cluster, service); ResourceProvider provider = getServiceProvider(managementController); @@ -422,7 +431,7 @@ public class ServiceResourceProviderTest { // delete the service named Service100 Predicate predicate = new PredicateBuilder().property(ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID).equals("Cluster100").and() - .property(ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID).equals("Service100").toPredicate(); + .property(ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID).equals(serviceName).toPredicate(); provider.deleteResources(predicate); @@ -434,10 +443,91 @@ public class ServiceResourceProviderTest { Assert.assertNull(lastEvent.getRequest()); // verify - verify(managementController, clusters, cluster); + verify(managementController, clusters, cluster, service); } @Test + public void testDeleteResourcesBadServiceState() throws Exception { + AmbariManagementController managementController = createMock(AmbariManagementController.class); + Clusters clusters = createNiceMock(Clusters.class); + Cluster cluster = createNiceMock(Cluster.class); + Service service = createNiceMock(Service.class); + + String serviceName = "Service100"; + + // set expectations + expect(managementController.getClusters()).andReturn(clusters).anyTimes(); + expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes(); + expect(cluster.getService(serviceName)).andReturn(service).anyTimes(); + expect(service.getDesiredState()).andReturn(State.STARTED).anyTimes(); + expect(service.getName()).andReturn(serviceName).anyTimes(); + + // replay + replay(managementController, clusters, cluster, service); + + ResourceProvider provider = getServiceProvider(managementController); + + AbstractResourceProviderTest.TestObserver observer = new AbstractResourceProviderTest.TestObserver(); + + ((ObservableResourceProvider)provider).addObserver(observer); + + // delete the service named Service100 + Predicate predicate = new PredicateBuilder().property(ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID).equals("Cluster100").and() + .property(ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID).equals(serviceName).toPredicate(); + + try { + provider.deleteResources(predicate); + Assert.fail("Expected exception deleting a service in a non-removable state."); + } catch (SystemException e) { + // expected + } + } + + @Test + public void testDeleteResourcesBadComponentState() throws Exception{ + AmbariManagementController managementController = createMock(AmbariManagementController.class); + Clusters clusters = createNiceMock(Clusters.class); + Cluster cluster = createNiceMock(Cluster.class); + Service service = createNiceMock(Service.class); + ServiceComponent sc = createNiceMock(ServiceComponent.class); + Map<String, ServiceComponent> scMap = new HashMap<String, ServiceComponent>(); + scMap.put("Component100", sc); + + + String serviceName = "Service100"; + + // set expectations + expect(managementController.getClusters()).andReturn(clusters).anyTimes(); + expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes(); + expect(cluster.getService(serviceName)).andReturn(service).anyTimes(); + expect(service.getDesiredState()).andReturn(State.INSTALLED).anyTimes(); + expect(service.getName()).andReturn(serviceName).anyTimes(); + expect(service.getServiceComponents()).andReturn(scMap); + expect(sc.getDesiredState()).andReturn(State.STARTED); + + // replay + replay(managementController, clusters, cluster, service, sc); + + ResourceProvider provider = getServiceProvider(managementController); + + AbstractResourceProviderTest.TestObserver observer = new AbstractResourceProviderTest.TestObserver(); + + ((ObservableResourceProvider)provider).addObserver(observer); + + // delete the service named Service100 + Predicate predicate = new PredicateBuilder().property(ServiceResourceProvider.SERVICE_CLUSTER_NAME_PROPERTY_ID).equals("Cluster100").and() + .property(ServiceResourceProvider.SERVICE_SERVICE_NAME_PROPERTY_ID).equals(serviceName).toPredicate(); + + try { + provider.deleteResources(predicate); + Assert.fail("Expected exception deleting a service in a non-removable state."); + } catch (SystemException e) { + // expected + } + } + + + @Test public void testCheckPropertyIds() throws Exception { Set<String> propertyIds = new HashSet<String>(); propertyIds.add("foo");
