core: Return validation messages from SchedulingManager

If validation of MigrateMultipleVmsCommand fails because SchedulingManager.canSchedule() returns an empty list of hosts, need to pass error messages from SchedulingManager up by command invocation hierarchy, so they could be displayed to the user. Change-Id: I4db998b4840129e6be03fbe99417286de16f223b Bug-Url: https://bugzilla.redhat.com/1789389 Signed-off-by: Shmuel Melamud <smelamud@redhat.com>
oVirt · Jul 7, 2022 · 44d5b80 · 44d5b80
1 parent c9b4ead
commit 44d5b80
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 14 deletions.
diff --git a/...nd/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceVdsCommand.java b/...nd/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceVdsCommand.java
@@ -17,6 +17,7 @@
 import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.VdcObjectType;
+import org.ovirt.engine.core.common.action.ActionReturnValue;
 import org.ovirt.engine.core.common.action.ActionType;
 import org.ovirt.engine.core.common.action.MaintenanceVdsParameters;
 import org.ovirt.engine.core.common.action.MigrateMultipleVmsParameters;
@@ -89,7 +90,12 @@ protected void executeCommand() {
                 }
             }
 
-            setSucceeded(migrateAllVms(getExecutionContext()));
+            ActionReturnValue returnValue = migrateAllVms(getExecutionContext());
+            setSucceeded(returnValue.getSucceeded());
+            if (!returnValue.isValid()) {
+                getReturnValue().setValid(false);
+                getReturnValue().getValidationMessages().addAll(returnValue.getValidationMessages());
+            }
 
             // if non responsive move directly to maintenance
             if (getVds().getStatus() == VDSStatus.NonResponsive
@@ -120,17 +126,14 @@ protected void orderListOfRunningVmsOnVds(Guid vdsId) {
     /**
      * Note: you must call {@link #orderListOfRunningVmsOnVds(Guid)} before calling this method
      */
-    protected boolean migrateAllVms(ExecutionContext parentContext) {
+    protected ActionReturnValue migrateAllVms(ExecutionContext parentContext) {
         return migrateAllVms(parentContext, false);
     }
 
     /**
      * Note: you must call {@link #orderListOfRunningVmsOnVds(Guid)} before calling this method
      */
-    protected boolean migrateAllVms(ExecutionContext parentContext, boolean HAOnly) {
-
-        boolean succeeded = true;
-
+    protected ActionReturnValue migrateAllVms(ExecutionContext parentContext, boolean HAOnly) {
         if (shouldSetMigrationClientCerts()) {
             runAnsibleMigrationCerts();
         }
@@ -143,17 +146,19 @@ protected boolean migrateAllVms(ExecutionContext parentContext, boolean HAOnly)
             }
         }
 
-        if (!migrateVms(vmsToMigrate, parentContext)) {
-            succeeded = false;
+        ActionReturnValue returnValue = migrateVms(vmsToMigrate, parentContext);
+        if (!returnValue.getSucceeded()) {
             // There is no way to find out which VMs failed to migrate, so the error message is general.
             log.error("Failed to migrate one or more VMs.");
         }
-        return succeeded;
+        return returnValue;
     }
 
-    private boolean migrateVms(List<VM> vms, ExecutionContext parentContext) {
+    private ActionReturnValue migrateVms(List<VM> vms, ExecutionContext parentContext) {
         if (vms.isEmpty()) {
-            return true;
+            ActionReturnValue returnValue = new ActionReturnValue();
+            returnValue.setSucceeded(true);
+            return returnValue;
         }
 
         boolean forceMigration = !getParameters().isInternal();
@@ -177,8 +182,7 @@ private boolean migrateVms(List<VM> vms, ExecutionContext parentContext) {
         parameters.setReason(MessageBundler.getMessage(AuditLogType.MIGRATION_REASON_HOST_IN_MAINTENANCE));
         return runInternalAction(ActionType.MigrateMultipleVms,
                 parameters,
-                createMigrateVmsContext(parentContext))
-                .getSucceeded();
+                createMigrateVmsContext(parentContext));
     }
 
     protected CommandContext createMigrateVmsContext(ExecutionContext parentContext) {

diff --git a/...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateMultipleVmsCommand.java b/...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateMultipleVmsCommand.java
@@ -102,11 +102,12 @@ protected boolean validate() {
             return false;
         }
 
+        List<String> outputMessages = new ArrayList<>();
         Map<Guid, List<VDS>> possibleVmHosts = schedulingManager.prepareCall(getCluster())
                 .hostBlackList(getHostBlackList())
                 .hostWhiteList(getHostWhiteList())
                 .ignoreHardVmToVmAffinity(getParameters().isCanIgnoreHardVmAffinity())
-                // TODO - Use error messages from scheduling
+                .outputMessages(outputMessages)
                 .canSchedule(getVms());
 
         possibleVmsToMigrate = new ArrayList<>(getVms().size());
@@ -119,6 +120,7 @@ protected boolean validate() {
         }
 
         if (possibleVmsToMigrate.isEmpty()) {
+            getReturnValue().getValidationMessages().addAll(outputMessages);
             return false;
         }