server: prevent duplicate HA works and alerts (apache#10624)

Signed-off-by: Abhishek Kumar <[email protected]>

Author: shwstppr

engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java

@@ -247,6 +247,11 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl
     protected final ConfigKey<Boolean> CheckTxnBeforeSending = new ConfigKey<Boolean>("Developer", Boolean.class, "check.txn.before.sending.agent.commands", "false",
             "This parameter allows developers to enable a check to see if a transaction wraps commands that are sent to the resource.  This is not to be enabled on production systems.", true);
 
+    public static final List<Host.Type> HOST_DOWN_ALERT_UNSUPPORTED_HOST_TYPES = Arrays.asList(
+            Host.Type.SecondaryStorage,
+            Host.Type.ConsoleProxy
+    );
+
     @Override
     public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
 
@@ -1106,9 +1111,11 @@ protected boolean handleDisconnectWithInvestigation(final AgentAttache attache,
                 if (determinedState == Status.Down) {
                     final String message = String.format("Host %s is down. Starting HA on the VMs", host);
                     logger.error(message);
-                    if (host.getType() != Host.Type.SecondaryStorage && host.getType() != Host.Type.ConsoleProxy) {
-                        _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(),
-                                host.getPodId(), String.format("Host down, %s", host), message);
+                    if (Status.Down.equals(host.getStatus())) {
+                        logger.debug(String.format("Skipping sending alert for %s as it already in %s state",
+                                host, host.getStatus()));
+                    } else if (!HOST_DOWN_ALERT_UNSUPPORTED_HOST_TYPES.contains(host.getType())) {
+                        _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host down, " + host.getId(), message);
                     }
                     event = Status.Event.HostDown;
                 } else if (determinedState == Status.Up) {

server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java

@@ -24,6 +24,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -45,6 +46,7 @@
 import org.apache.cloudstack.managed.context.ManagedContextRunnable;
 import org.apache.cloudstack.management.ManagementServerHost;
 import org.apache.logging.log4j.ThreadContext;
+import org.apache.commons.collections.CollectionUtils;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.alert.AlertManager;
@@ -73,7 +75,6 @@
 import com.cloud.network.VpcVirtualNetworkApplianceService;
 import com.cloud.resource.ResourceManager;
 import com.cloud.server.ManagementServer;
-import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.storage.StorageManager;
@@ -236,6 +237,18 @@ public void setHaPlanners(List<HAPlanner> haPlanners) {
     long _timeBetweenCleanups;
     String _haTag = null;
 
+    private boolean vmHasPendingHAJob(final List<HaWorkVO> pendingHaWorks, final VMInstanceVO vm) {
+        Optional<HaWorkVO> item = pendingHaWorks.stream()
+                .filter(h -> h.getInstanceId() == vm.getId())
+                .reduce((first, second) -> second);
+        if (item.isPresent() && (item.get().getTimesTried() < _maxRetries ||
+                !item.get().canScheduleNew(_timeBetweenFailures))) {
+            s_logger.debug(String.format("Skipping HA on %s as there is already a running HA job for it", vm));
+            return true;
+        }
+        return false;
+    }
+
     protected HighAvailabilityManagerImpl() {
     }
 
@@ -295,36 +308,44 @@ public void scheduleRestartForVmsOnHost(final HostVO host, boolean investigate,
         logger.warn("Scheduling restart for VMs on host {}", host);
 
         final List<VMInstanceVO> vms = _instanceDao.listByHostId(host.getId());
+        final List<HaWorkVO> pendingHaWorks = _haDao.listPendingHAWorkForHost(host.getId());
         final DataCenterVO dcVO = _dcDao.findById(host.getDataCenterId());
 
         // send an email alert that the host is down
         StringBuilder sb = null;
         List<VMInstanceVO> reorderedVMList = new ArrayList<VMInstanceVO>();
-        if ((vms != null) && !vms.isEmpty()) {
+        int skippedHAVms = 0;
+        if (CollectionUtils.isNotEmpty(vms)) {
             sb = new StringBuilder();
             sb.append("  Starting HA on the following VMs:");
             // collect list of vm names for the alert email
-            for (int i = 0; i < vms.size(); i++) {
-                VMInstanceVO vm = vms.get(i);
+            for (VMInstanceVO vm : vms) {
+                if (vmHasPendingHAJob(pendingHaWorks, vm)) {
+                    skippedHAVms++;
+                    continue;
+                }
                 if (vm.getType() == VirtualMachine.Type.User) {
                     reorderedVMList.add(vm);
                 } else {
                     reorderedVMList.add(0, vm);
                 }
                 if (vm.isHaEnabled()) {
-                    sb.append(" " + vm.getHostName());
+                    sb.append(" ").append(vm.getHostName());
                 }
             }
         }
-
+        if (reorderedVMList.isEmpty() && skippedHAVms > 0 && skippedHAVms == vms.size()) {
+            s_logger.debug(String.format(
+                    "Skipping sending alert for %s as it is suspected to be a duplicate of a recent alert", host));
+            return;
+        }
         // send an email alert that the host is down, include VMs
         HostPodVO podVO = _podDao.findById(host.getPodId());
         String hostDesc = "name: " + host.getName() + " (id:" + host.getId() + "), availability zone: " + dcVO.getName() + ", pod: " + podVO.getName();
         _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "Host is down, " + hostDesc,
                 "Host [" + hostDesc + "] is down." + ((sb != null) ? sb.toString() : ""));
 
         for (VMInstanceVO vm : reorderedVMList) {
-            ServiceOfferingVO vmOffering = _serviceOfferingDao.findById(vm.getServiceOfferingId());
             if (_itMgr.isRootVolumeOnLocalStorage(vm.getId())) {
                 if (logger.isDebugEnabled()){
                     logger.debug("Skipping HA on vm " + vm + ", because it uses local storage. Its fate is tied to the host.");

server/src/main/java/com/cloud/ha/dao/HighAvailabilityDao.java

@@ -85,7 +85,10 @@ public interface HighAvailabilityDao extends GenericDao<HaWorkVO, Long> {
     List<HaWorkVO> listPendingHaWorkForVm(long vmId);
 
     List<HaWorkVO> listPendingMigrationsForVm(long vmId);
+
+    List<HaWorkVO> listPendingHAWorkForHost(long hostId);
+
     int expungeByVmList(List<Long> vmIds, Long batchSize);
     void markPendingWorksAsInvestigating();
     void markServerPendingWorksAsInvestigating(long managementServerId);
-}
+}

server/src/main/java/com/cloud/ha/dao/HighAvailabilityDaoImpl.java

@@ -261,6 +261,18 @@ public int releaseWorkItems(long nodeId) {
     }
 
     @Override
+    public List<HaWorkVO> listPendingHAWorkForHost(long hostId) {
+        SearchBuilder<HaWorkVO> sb = createSearchBuilder();
+        sb.and("hostId", sb.entity().getHostId(), Op.EQ);
+        sb.and("type", sb.entity().getWorkType(), Op.EQ);
+        sb.and("step", sb.entity().getStep(), Op.NIN);
+        SearchCriteria<HaWorkVO> sc = sb.create();
+        sc.setParameters("hostId", hostId);
+        sc.setParameters("type", WorkType.HA);
+        sc.setParameters("step", Step.Done, Step.Cancelled, Step.Error);
+        return listBy(sc);
+    }
+
     public int expungeByVmList(List<Long> vmIds, Long batchSize) {
         if (CollectionUtils.isEmpty(vmIds)) {
             return 0;

server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java

@@ -65,7 +65,6 @@
 import com.cloud.network.VpcVirtualNetworkApplianceService;
 import com.cloud.resource.ResourceManager;
 import com.cloud.server.ManagementServer;
-import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.dao.GuestOSCategoryDao;
@@ -236,8 +235,7 @@ public void scheduleRestartForVmsOnHostNonEmptyVMList() {
         Mockito.when(_podDao.findById(Mockito.anyLong())).thenReturn(Mockito.mock(HostPodVO.class));
         Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(Mockito.mock(DataCenterVO.class));
         Mockito.when(_haDao.findPreviousHA(Mockito.anyLong())).thenReturn(Arrays.asList(Mockito.mock(HaWorkVO.class)));
-        Mockito.when(_haDao.persist((HaWorkVO)Mockito.any())).thenReturn(Mockito.mock(HaWorkVO.class));
-        Mockito.when(_serviceOfferingDao.findById(vm1.getServiceOfferingId())).thenReturn(Mockito.mock(ServiceOfferingVO.class));
+        Mockito.when(_haDao.persist((HaWorkVO)Mockito.anyObject())).thenReturn(Mockito.mock(HaWorkVO.class));
 
         ConfigKey<Boolean> haEnabled = Mockito.mock(ConfigKey.class);
         highAvailabilityManager.VmHaEnabled = haEnabled;