core: VDS Maintenance will wait for all disk transfers to happen and …

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java

@@ -42,7 +42,6 @@
 import org.ovirt.engine.core.common.businessentities.VdsSpmStatus;
 import org.ovirt.engine.core.common.businessentities.VmDynamic;
 import org.ovirt.engine.core.common.businessentities.network.Network;
-import org.ovirt.engine.core.common.businessentities.storage.ImageTransfer;
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.common.errors.EngineError;
@@ -375,8 +374,6 @@ protected boolean validate() {
                             result = failValidation(EngineMessage.VDS_CANNOT_MAINTENANCE_SPM_WITH_RUNNING_TASKS);
                         } else if (!validateNoRunningJobs(vds)) {
                             result = false;
-                        } else if (!validateNoActiveImageTransfers(vds)) {
-                            result = false;
                         } else if (!clusters.get(vds.getClusterId()).isInUpgradeMode()) {
                             result = handlePositiveEnforcingAffinityGroup(vdsId, vms, clusters.get(vds.getClusterId()).getCompatibilityVersion());
                         }
@@ -475,22 +472,6 @@ private boolean validateNoRunningJobs(VDS vds) {
         return true;
     }
 
-    private boolean validateNoActiveImageTransfers(VDS vds) {
-        List<ImageTransfer> transfers = imageTransferDao.getByVdsId(vds.getId());
-        if (!transfers.stream().allMatch(ImageTransfer::isPausedOrFinished)) {
-            List<String> replacements = new ArrayList<>(3);
-            replacements.add(ReplacementUtils.createSetVariableString("host", vds.getName()));
-            replacements.addAll(ReplacementUtils.replaceWith("disks",
-                    transfers.stream()
-                            .filter(imageTransfer -> !imageTransfer.isPausedOrFinished())
-                            .map(ImageTransfer::getDiskId)
-                            .sorted()
-                            .collect(Collectors.toList())));
-            return failValidation(EngineMessage.VDS_CANNOT_MAINTENANCE_HOST_WITH_RUNNING_IMAGE_TRANSFERS, replacements);
-        }
-        return true;
-    }
-
     /*
      * Validates gluster specific properties before moving the host to maintenance. Following things will be checked as
      * part of this check 1. Ensure gluster quorum can be met for all the volumes 2. Ensure there is no unsynced entry

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java

@@ -446,7 +446,6 @@ public enum EngineMessage {
     VDS_CANNOT_MAINTENANCE_SPM_CONTENDING(ErrorType.CONFLICT),
     VDS_CANNOT_MAINTENANCE_VDS_IS_IN_MAINTENANCE(ErrorType.CONFLICT),
     VDS_CANNOT_MAINTENANCE_VDS_HAS_AFFINITY_VMS(ErrorType.CONFLICT),
-    VDS_CANNOT_MAINTENANCE_HOST_WITH_RUNNING_IMAGE_TRANSFERS(ErrorType.CONFLICT),
     VDS_NO_UUID(ErrorType.CONFLICT),
     VDS_ALREADY_UP(ErrorType.CONFLICT),
     VDS_NON_RESPONSIVE(ErrorType.CONFLICT),

backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategy.java

@@ -21,21 +21,26 @@
 import org.ovirt.engine.core.common.businessentities.VDSStatus;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VmRngDevice;
+import org.ovirt.engine.core.common.businessentities.storage.ImageTransfer;
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.common.utils.ClusterEmulatedMachines;
 import org.ovirt.engine.core.common.utils.EmulatedMachineCommonUtils;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dao.ClusterDao;
+import org.ovirt.engine.core.dao.ImageTransferDao;
 import org.ovirt.engine.core.dao.VdsDao;
 import org.ovirt.engine.core.dao.VmDao;
 import org.ovirt.engine.core.dao.VmDynamicDao;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * This class defines virt strategy entry points, which are needed in host monitoring phase
  */
 @Singleton
 public class VirtMonitoringStrategy implements MonitoringStrategy {
+    private static final Logger log = LoggerFactory.getLogger(VirtMonitoringStrategy.class);
 
     private final ClusterDao clusterDao;
     private final VdsDao vdsDao;
@@ -45,19 +50,29 @@ public class VirtMonitoringStrategy implements MonitoringStrategy {
     @Inject
     private Instance<IVdsEventListener> eventListener;
 
+    private ImageTransferDao imageTransferDao;
+
     @Inject
     public VirtMonitoringStrategy(ClusterDao clusterDao,
             VdsDao vdsDao,
             VmDao vmDao,
-            VmDynamicDao vmDynamicDao) {
+            VmDynamicDao vmDynamicDao,
+            ImageTransferDao imageTransferDao) {
         this.clusterDao = clusterDao;
         this.vdsDao = vdsDao;
         this.vmDao = vmDao;
         this.vmDynamicDao = vmDynamicDao;
+        this.imageTransferDao = imageTransferDao;
     }
 
     @Override
     public boolean canMoveToMaintenance(VDS vds) {
+        List<ImageTransfer> transfers = imageTransferDao.getByVdsId(vds.getId());
+        if (!transfers.stream().allMatch(ImageTransfer::isPausedOrFinished)) {
+            log.info("Can't move host '{}' to maintenance,"
+                    + " waiting for ongoing image transfers to complete", vds.getName());
+            return false;
+        }
         if (!Config.<Boolean>getValue(ConfigValues.MaintenanceVdsIgnoreExternalVms)) {
             // We can only move to maintenance in case no VMs are running on the host
             return vds.getVmCount() == 0 && !isAnyVmRunOnVdsInDb(vds.getId());

backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/monitoring/MultipleServicesMonitoringStrategyTest.java

@@ -22,6 +22,7 @@
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dao.ClusterDao;
+import org.ovirt.engine.core.dao.ImageTransferDao;
 import org.ovirt.engine.core.dao.VdsDao;
 import org.ovirt.engine.core.dao.VmDao;
 import org.ovirt.engine.core.utils.MockConfigDescriptor;
@@ -41,7 +42,7 @@ public static Stream<MockConfigDescriptor<?>> mockConfiguration() {
 
     public MultipleServicesMonitoringStrategyTest() {
         virtStrategy =
-                spy(new VirtMonitoringStrategy(mock(ClusterDao.class), mock(VdsDao.class), mockVmDao(), null));
+                spy(new VirtMonitoringStrategy(mock(ClusterDao.class), mock(VdsDao.class), mockVmDao(), null, mock(ImageTransferDao.class)));
         doReturn(false).when(virtStrategy).isAnyVmRunOnVdsInDb(any());
         glusterStrategy = spy(new GlusterMonitoringStrategy());
         doNothing().when(virtStrategy).vdsNonOperational(any(), any(), any());

backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategyTest.java

@@ -11,6 +11,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.stream.Stream;
 
@@ -22,11 +23,14 @@
 import org.ovirt.engine.core.common.businessentities.VDSStatus;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VmRngDevice;
+import org.ovirt.engine.core.common.businessentities.storage.ImageTransfer;
+import org.ovirt.engine.core.common.businessentities.storage.ImageTransferPhase;
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.common.scheduling.ClusterPolicy;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.compat.Version;
 import org.ovirt.engine.core.dao.ClusterDao;
+import org.ovirt.engine.core.dao.ImageTransferDao;
 import org.ovirt.engine.core.dao.VdsDao;
 import org.ovirt.engine.core.dao.VmDao;
 import org.ovirt.engine.core.utils.MockConfigDescriptor;
@@ -36,8 +40,10 @@
 public class VirtMonitoringStrategyTest {
     private Guid vdsId = Guid.newGuid();
     private Guid vdsId2 = Guid.newGuid();
+    private Guid vdsIdNoImageTransfers = Guid.newGuid();
+    private Guid vdsIdNoActiveImageTransfers = Guid.newGuid();
+    private Guid vdsIdImageTransfersRunning = Guid.newGuid();
     private Guid clusterId = Guid.newGuid();
-
     private VDS vdsFromDb;
     private Cluster cluster;
 
@@ -51,7 +57,7 @@ public void setUp() {
         vdsFromDb.setId(vdsId);
         vdsFromDb.setClusterId(clusterId);
 
-        virtStrategy = spy(new VirtMonitoringStrategy(mockCluster(), mockVdsDao(), mockVmDao(), null));
+        virtStrategy = spy(new VirtMonitoringStrategy(mockCluster(), mockVdsDao(), mockVmDao(), null, mockImageTransferDao()));
         doNothing().when(virtStrategy).vdsNonOperational(any(), any(), any());
     }
 
@@ -69,6 +75,14 @@ public void testVirtCanMoveToMaintenance() {
         assertTrue(virtStrategy.canMoveToMaintenance(vds));
         doReturn(true).when(virtStrategy).isAnyNonExternalVmRunningOnVds(vds);
         assertFalse(virtStrategy.canMoveToMaintenance(vds));
+        // next, testing hosts with/without image transfers so first make sure there's no running vm on the host
+        doReturn(false).when(virtStrategy).isAnyNonExternalVmRunningOnVds(vds);
+        vds.setId(vdsIdNoImageTransfers);
+        assertTrue(virtStrategy.canMoveToMaintenance(vds));
+        vds.setId(vdsIdNoActiveImageTransfers);
+        assertTrue(virtStrategy.canMoveToMaintenance(vds));
+        vds.setId(vdsIdImageTransfersRunning);
+        assertFalse(virtStrategy.canMoveToMaintenance(vds));
     }
 
     @Test
@@ -172,6 +186,17 @@ public void testNeedToProcessHardwareCapsTrue() {
         newVds.setCpuFlags("flag2");
         assertTrue(virtStrategy.processHardwareCapabilitiesNeeded(oldVds, newVds));
     }
+    private ImageTransferDao mockImageTransferDao() {
+        ImageTransferDao mock = mock(ImageTransferDao.class);
+        ImageTransfer imageTransferFinished = new ImageTransfer();
+        ImageTransfer imageTransferWip = new ImageTransfer();
+        imageTransferFinished.setPhase(ImageTransferPhase.FINISHED_SUCCESS);
+        when(mock.getByVdsId(vdsIdNoImageTransfers)).thenReturn(Collections.emptyList());
+        when(mock.getByVdsId(vdsIdNoActiveImageTransfers)).thenReturn(Arrays.asList(imageTransferFinished));
+        imageTransferWip.setPhase(ImageTransferPhase.TRANSFERRING);
+        when(mock.getByVdsId(vdsIdImageTransfersRunning)).thenReturn(Arrays.asList(imageTransferWip));
+        return mock;
+    }
 
     private ClusterDao mockCluster() {
         ClusterDao mock = mock(ClusterDao.class);

frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java

@@ -101,8 +101,6 @@ public interface AppErrors extends ConstantsWithLookup {
 
     String VDS_CANNOT_MAINTENANCE_VDS_HAS_AFFINITY_VMS();
 
-    String VDS_CANNOT_MAINTENANCE_HOST_WITH_RUNNING_IMAGE_TRANSFERS();
-
     String CPU_TYPE_UNSUPPORTED_IN_THIS_CLUSTER_VERSION();
 
     String ACTION_TYPE_FAILED_VM_CLUSTER_DIFFERENT_ARCHITECTURES();

frontend/webadmin/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties

@@ -1435,7 +1435,6 @@ VDS_CANNOT_MAINTENANCE_VDS_IS_NOT_RESPONDING_WITH_VMS=Cannot switch Host to Main
 VDS_CANNOT_MAINTENANCE_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot switch the following Hosts to Maintenance mode\: ${HostsList}.\nThe following VMs cannot be migrated because they have activated Disk Snapshot attached (VM/Disk Snapshots)\: \n \n${disksInfo} \n \nplease deactivate/detach the Disk snapshots or turn off those VMs and try again.
 VDS_CANNOT_MAINTENANCE_GLUSTER_QUORUM_CANNOT_BE_MET=Cannot switch the following Host(s) to Maintenance mode\: ${HostsList}.\nGluster quorum will be lost for the following Volumes: ${VolumesList}.
 VDS_CANNOT_MAINTENANCE_UNSYNCED_ENTRIES_PRESENT_IN_GLUSTER_BRICKS=Cannot switch the following Host(s) to Maintenance mode\: ${HostsList}.\nUnsynced entries present in following gluster bricks: ${BricksList}.
-VDS_CANNOT_MAINTENANCE_HOST_WITH_RUNNING_IMAGE_TRANSFERS=Cannot switch Host ${host} to Maintenance mode. Image transfer is in progress for the following (${disks_COUNTER}) disks: \n\n ${disks} \n\nPlease wait for the operations to complete and try again.
 VDS_CANNOT_REMOVE_CLUSTER_VDS_DETECTED=Cannot ${action} ${type}. Cluster contains one or more Hosts
 VDS_CANNOT_REMOVE_DEFAULT_CLUSTER=Cannot remove default Host Cluster.
 VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME=Cannot ${action} ${type}. Server having Gluster volume.