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

…block new ones if a user want to set a host to maintenance and at least one of the disk is in transfer the command will fail and will send an error. now it will not send an error it will wait for the disk transfer to finnish (the host will be in preparingForMaintenance mode) and new transfers will be blocked from starting. Bug-Url: https://bugzilla.redhat.com/2120228 Signed-off-by: Artiom Divak <adivak@redhat.com>
oVirt · Sep 20, 2022 · 26e1687 · 26e1687
1 parent 4697324
commit 26e1687
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 26 deletions.
diff --git a/...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java b/...r/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

diff --git a/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java b/...nager/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),

diff --git a/...oker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategy.java b/...oker/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());

diff --git a/...va/org/ovirt/engine/core/vdsbroker/monitoring/MultipleServicesMonitoringStrategyTest.java b/...va/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());

diff --git a/.../src/test/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategyTest.java b/.../src/test/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategyTest.java
@@ -11,7 +11,9 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -22,11 +24,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,6 +41,13 @@
 public class VirtMonitoringStrategyTest {
     private Guid vdsId = Guid.newGuid();
     private Guid vdsId2 = Guid.newGuid();
+
+    private Guid vdsId3 = Guid.newGuid();
+
+    private Guid vdsId4 = Guid.newGuid();
+
+    private Guid vdsId5 = Guid.newGuid();
+
     private Guid clusterId = Guid.newGuid();
 
     private VDS vdsFromDb;
@@ -51,7 +63,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, mock(ImageTransferDao.class)));
         doNothing().when(virtStrategy).vdsNonOperational(any(), any(), any());
     }
 
@@ -69,6 +81,15 @@ public void testVirtCanMoveToMaintenance() {
         assertTrue(virtStrategy.canMoveToMaintenance(vds));
         doReturn(true).when(virtStrategy).isAnyNonExternalVmRunningOnVds(vds);
         assertFalse(virtStrategy.canMoveToMaintenance(vds));
+        vds.setId(vdsId3);
+        doReturn(false).when(virtStrategy).isAnyNonExternalVmRunningOnVds(vds);
+        assertTrue(virtStrategy.canMoveToMaintenance(vds));
+        vds.setId(vdsId4);
+        doReturn(false).when(virtStrategy).isAnyNonExternalVmRunningOnVds(vds);
+        assertTrue(virtStrategy.canMoveToMaintenance(vds));
+        vds.setId(vdsId5);
+        doReturn(true).when(virtStrategy).isAnyNonExternalVmRunningOnVds(vds);
+        assertFalse(virtStrategy.canMoveToMaintenance(vds));
     }
 
     @Test
@@ -172,6 +193,20 @@ public void testNeedToProcessHardwareCapsTrue() {
         newVds.setCpuFlags("flag2");
         assertTrue(virtStrategy.processHardwareCapabilitiesNeeded(oldVds, newVds));
     }
+    private  ImageTransferDao mockImageTransferDao() {
+        ImageTransferDao mock = mock(ImageTransferDao.class);
+        List<ImageTransfer> imageTransfers = new ArrayList<>();
+        List<ImageTransfer> imageTransfers2 = new ArrayList<>();
+        ImageTransfer imageTransfer = new ImageTransfer();
+        imageTransfer.setPhase(ImageTransferPhase.FINISHED_SUCCESS);
+        imageTransfers.add(imageTransfer);
+        when(mock.getByVdsId(vdsId3)).thenReturn(null);
+        when(mock.getByVdsId(vdsId4)).thenReturn(imageTransfers);
+        imageTransfer.setPhase(ImageTransferPhase.TRANSFERRING);
+        imageTransfers2.add(imageTransfer);
+        when(mock.getByVdsId(vdsId5)).thenReturn(imageTransfers2);
+        return mock;
+    }
 
     private ClusterDao mockCluster() {
         ClusterDao mock = mock(ClusterDao.class);

diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/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();

diff --git a/...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -1434,7 +1434,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.

diff --git a/ui-extensions-resources b/ui-extensions-resources
@@ -0,0 +1 @@
+/home/adivak/Desktop/oVirt/dist/ui-extensions-resources
diff --git a/ui-extensions.json b/ui-extensions.json
@@ -0,0 +1 @@
+/home/adivak/Desktop/oVirt/dist/ui-extensions.json