You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by nc...@apache.org on 2014/02/28 16:44:28 UTC

git commit: AMBARI-4876. Improve error while trying to delete a service. (ncole)

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 <nc...@hortonworks.com>
Authored: Thu Feb 27 16:24:18 2014 -0500
Committer: Nate Cole <nc...@hortonworks.com>
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");