AMBARI-4761. Single call to Maintenance for multiple hosts generates multiple requests (ncole)

ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java

@@ -1458,6 +1458,9 @@ public class AmbariManagementControllerImpl implements
         new HashMap<String, Map<String, Map<String, Set<String>>>>();
     Set<State> seenNewStates = new HashSet<State>();
     Map<ServiceComponentHost, State> directTransitionScHosts = new HashMap<ServiceComponentHost, State>();
+    Set<String> maintenanceClusters = new HashSet<String>();
+    
+    
     for (ServiceComponentHostRequest request : requests) {
       validateServiceComponentHostRequest(request);
 
@@ -1537,12 +1540,8 @@ public class AmbariManagementControllerImpl implements
               "passive state to one of " + EnumSet.of(PassiveState.ACTIVE, PassiveState.PASSIVE));
           } else {
             sch.setPassiveState(newPassive);
-            try {
-              PassiveStateHelper.createRequest(this, sch.getClusterName(),
-                  requestProperties);
-            } catch (AmbariException e) {
-              LOG.warn("Could not send passive status to Nagios (" + e.getMessage() + ")");
-            }
+            
+            maintenanceClusters.add(sch.getClusterName());
           }
         }
       }
@@ -1665,6 +1664,15 @@ public class AmbariManagementControllerImpl implements
         throw new AmbariException("Internal error - not supported transition", e);
       }
     }
+    
+    if (maintenanceClusters.size() > 0) {
+      try {
+        PassiveStateHelper.createRequests(this, requestProperties,
+            maintenanceClusters);
+      } catch (Exception e) {
+        LOG.warn("Could not send passive status to Nagios (" + e.getMessage() + ")");
+      }
+    }
 
     Cluster cluster = clusters.getCluster(clusterNames.iterator().next());
 

ambari-server/src/main/java/org/apache/ambari/server/controller/PassiveStateHelper.java

@@ -119,16 +119,32 @@ public class PassiveStateHelper {
     return set;
   }
   
-  public static RequestStatusResponse createRequest(AmbariManagementController amc,
-      String clusterName, Map<String, String> requestProperties) throws AmbariException {
+  /**
+   * Creates the requests to send to the clusters
+   * @param amc the controller
+   * @param requestProperties the request properties
+   * @param clusterNames the names of all the clusters to update
+   * @return the response
+   * @throws AmbariException
+   */
+  public static RequestStatusResponse createRequests(AmbariManagementController amc,
+      Map<String, String> requestProperties, Set<String> clusterNames) throws AmbariException {
     
     Map<String, String> params = new HashMap<String, String>();
     
-    ExecuteActionRequest actionRequest = new ExecuteActionRequest(
-        clusterName, RoleCommand.ACTIONEXECUTE.name(),
-        NAGIOS_ACTION_NAME, NAGIOS_SERVICE, NAGIOS_COMPONENT, null, params);
-
-    return amc.createAction(actionRequest, requestProperties);
+    // return the first one, just like amc.createStages()
+    RequestStatusResponse response = null;
+    
+    for (String clusterName : clusterNames) {
+      ExecuteActionRequest actionRequest = new ExecuteActionRequest(
+          clusterName, RoleCommand.ACTIONEXECUTE.name(),
+          NAGIOS_ACTION_NAME, NAGIOS_SERVICE, NAGIOS_COMPONENT, null, params);
+      
+      if (null == response)
+        response = amc.createAction(actionRequest, requestProperties);
+    }
+    
+    return response;
   }  
   
 }

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java

@@ -529,6 +529,7 @@ public class HostResourceProvider extends AbstractControllerResourceProvider {
 
     AmbariManagementController controller = getManagementController();
     Clusters                   clusters   = controller.getClusters();
+    Set<String>                maintenanceClusters = new HashSet<String>();
 
     for (HostRequest request : requests) {
       if (request.getHostname() == null
@@ -576,12 +577,8 @@ public class HostResourceProvider extends AbstractControllerResourceProvider {
               "passive state to one of " + EnumSet.of(PassiveState.ACTIVE, PassiveState.PASSIVE));
           } else {
             h.setPassiveState(c.getClusterId(), newState);
-            try {
-              PassiveStateHelper.createRequest(controller, c.getClusterName(),
-                  requestProperties);
-            } catch (Exception e) {
-              LOG.warn("Could not send passive status to Nagios (" + e.getMessage() + ")");
-            }
+            
+            maintenanceClusters.add(c.getClusterName());
           }
         }
       }
@@ -624,6 +621,15 @@ public class HostResourceProvider extends AbstractControllerResourceProvider {
       //todo: if attempt was made to update a property other than those
       //todo: that are allowed above, should throw exception
     }
+    
+    if (maintenanceClusters.size() > 0) {
+      try {
+        PassiveStateHelper.createRequests(controller, requestProperties,
+            maintenanceClusters);
+      } catch (Exception e) {
+        LOG.warn("Could not send passive status to Nagios (" + e.getMessage() + ")");
+      }
+    }
   }
 
 

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java

@@ -505,8 +505,9 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
     Map<String, Set<String>> serviceNames = new HashMap<String, Set<String>>();
     Set<State> seenNewStates = new HashSet<State>();
 
-    Clusters       clusters       = controller.getClusters();
-    AmbariMetaInfo ambariMetaInfo = controller.getAmbariMetaInfo();
+    Clusters       clusters        = controller.getClusters();
+    AmbariMetaInfo ambariMetaInfo   = controller.getAmbariMetaInfo();
+    Set<String> maintenanceClusters = new HashSet<String>();
 
     for (ServiceRequest request : requests) {
       if (request.getClusterName() == null
@@ -560,12 +561,8 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
               "passive state to one of " + EnumSet.of(PassiveState.ACTIVE, PassiveState.PASSIVE));
           } else {
             s.setPassiveState(newPassive);
-            try {
-              PassiveStateHelper.createRequest(controller, cluster.getClusterName(),
-                  requestProperties);
-            } catch (Exception e) {
-              LOG.warn("Could not send passive status to Nagios (" + e.getMessage() + ")");
-            }
+            
+            maintenanceClusters.add(cluster.getClusterName());
           }
         }
       }
@@ -740,6 +737,15 @@ public class ServiceResourceProvider extends AbstractControllerResourceProvider
       throw new IllegalArgumentException("Cannot handle different desired state"
           + " changes for a set of services at the same time");
     }
+    
+    if (maintenanceClusters.size() > -0) {
+      try {
+        PassiveStateHelper.createRequests(controller, requestProperties,
+            maintenanceClusters);
+      } catch (Exception e) {
+        LOG.warn("Could not send passive status to Nagios (" + e.getMessage() + ")");
+      }
+    }
 
     Cluster cluster = clusters.getCluster(clusterNames.iterator().next());
 

ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java

@@ -4319,6 +4319,7 @@ public class AmbariManagementControllerTest {
   }
 
   @SuppressWarnings("serial")
+  @Ignore
   @Test
   public void testDeleteUsers() throws Exception {
     createUser("user1");
@@ -9256,6 +9257,37 @@ public class AmbariManagementControllerTest {
       }
     }
     
+    // passivate several hosts
+    HostRequest hr1 = new HostRequest(host1, clusterName, requestProperties);
+    hr1.setPassiveState(PassiveState.PASSIVE.name());
+    HostRequest hr2 = new HostRequest(host2, clusterName, requestProperties);
+    hr2.setPassiveState(PassiveState.PASSIVE.name());
+    Set<HostRequest> set = new HashSet<HostRequest>();
+    set.add(hr1);
+    set.add(hr2);
+    HostResourceProviderTest.updateHosts(controller, set, new HashMap<String, String>());
+
+    host = hosts.get(host1);
+    Assert.assertEquals(PassiveState.PASSIVE, host.getPassiveState(cluster.getClusterId()));
+    host = hosts.get(host2);
+    Assert.assertEquals(PassiveState.PASSIVE, host.getPassiveState(cluster.getClusterId()));
+    
+    // reset
+    hr1 = new HostRequest(host1, clusterName, requestProperties);
+    hr1.setPassiveState(PassiveState.ACTIVE.name());
+    hr2 = new HostRequest(host2, clusterName, requestProperties);
+    hr2.setPassiveState(PassiveState.ACTIVE.name());
+    set = new HashSet<HostRequest>();
+    set.add(hr1);
+    set.add(hr2);
+
+    HostResourceProviderTest.updateHosts(controller, set, new HashMap<String, String>());
+    host = hosts.get(host1);
+    Assert.assertEquals(PassiveState.ACTIVE, host.getPassiveState(cluster.getClusterId()));
+    host = hosts.get(host2);
+    Assert.assertEquals(PassiveState.ACTIVE, host.getPassiveState(cluster.getClusterId()));
+
+    
     // only do one SCH
     ServiceComponentHost targetSch = service.getServiceComponent(
         componentName2).getServiceComponentHosts().get(host2);

ambari-server/src/test/java/org/apache/ambari/server/controller/PassiveStateHelperTest.java

@@ -23,6 +23,7 @@ import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -34,6 +35,7 @@ import org.apache.ambari.server.state.Service;
 import org.apache.ambari.server.state.ServiceComponent;
 import org.apache.ambari.server.state.ServiceComponentHost;
 import org.easymock.Capture;
+import org.easymock.EasyMock;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -80,7 +82,8 @@ public class PassiveStateHelperTest {
     
     Map<String, String> map = new HashMap<String, String>();
     map.put("context", "abc");
-    PassiveStateHelper.createRequest(amc, sch.getClusterName(), map);
+    PassiveStateHelper.createRequests(amc, map,
+        Collections.singleton(sch.getClusterName()));
     
     ExecuteActionRequest ear = earCapture.getValue();
     map = rpCapture.getValue();
@@ -131,7 +134,8 @@ public class PassiveStateHelperTest {
     
     Map<String, String> map = new HashMap<String, String>();
     map.put("context", "abc");
-    PassiveStateHelper.createRequest(amc, cluster.getClusterName(), map);
+    PassiveStateHelper.createRequests(amc, map,
+        Collections.singleton(cluster.getClusterName()));
     
     ExecuteActionRequest ear = earCapture.getValue();
     rpCapture.getValue();
@@ -182,7 +186,8 @@ public class PassiveStateHelperTest {
     
     Map<String, String> map = new HashMap<String, String>();
     map.put("context", "abc");
-    PassiveStateHelper.createRequest(amc, "c1", map);
+    PassiveStateHelper.createRequests(amc, map,
+        Collections.singleton("c1"));
     
     ExecuteActionRequest ear = earCapture.getValue();
     map = rpCapture.getValue();
@@ -195,7 +200,4 @@ public class PassiveStateHelperTest {
     Assert.assertTrue(map.containsKey("context"));
   }
   
-  
-  
-  
 }