From acd55224cf44adcbd3fa340ce4efe896f6c03a7e Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 22 Jul 2026 17:15:28 +0200 Subject: [PATCH 1/6] conditional error reporting for vm move failure --- api/src/main/java/com/cloud/vm/UserVmService.java | 6 ++++++ .../apache/cloudstack/api/command/admin/vm/AssignVMCmd.java | 4 ++++ server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/com/cloud/vm/UserVmService.java b/api/src/main/java/com/cloud/vm/UserVmService.java index dc9e8c1f0d8c..184fbd83c30b 100644 --- a/api/src/main/java/com/cloud/vm/UserVmService.java +++ b/api/src/main/java/com/cloud/vm/UserVmService.java @@ -40,6 +40,7 @@ import org.apache.cloudstack.api.command.user.vm.UpgradeVMCmd; import org.apache.cloudstack.api.command.user.vmgroup.CreateVMGroupCmd; import org.apache.cloudstack.api.command.user.vmgroup.DeleteVMGroupCmd; +import org.apache.cloudstack.framework.config.ConfigKey; import com.cloud.dc.DataCenter; import com.cloud.deploy.DeploymentPlanner; @@ -64,6 +65,11 @@ public interface UserVmService { + ConfigKey AllowExposingVmAssignFailureDetails = new ConfigKey<>("Advanced", Boolean.class, + "vm.assign.failure.detailed.message.enabled", "true", + "If true, the reason a VM ownership change (assignVirtualMachine) fails, e.g. remaining port forwarding rules or volume snapshots, is returned to the API caller. " + + "If false, only a generic error message referencing the Instance ID is returned, and the detailed reason is only available in the management server logs.", + true, ConfigKey.Scope.Global); /** * Destroys one virtual machine diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java index e11d20d06466..dd726c339a88 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java @@ -36,6 +36,7 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.user.Account; import com.cloud.uservm.UserVm; +import com.cloud.vm.UserVmService; import com.cloud.vm.VirtualMachine; @APICommand(name = "assignVirtualMachine", @@ -128,6 +129,9 @@ public void execute() { ApiErrorCode errorCode = e instanceof InvalidParameterValueException ? ApiErrorCode.PARAM_ERROR : ApiErrorCode.INTERNAL_ERROR; String msg = String.format("Failed to move Instance due to [%s].", getVmId()); logger.error(msg, e); + if (e instanceof InvalidParameterValueException && UserVmService.AllowExposingVmAssignFailureDetails.value()) { + msg = e.getMessage(); + } throw new ServerApiException(errorCode, msg); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 54ba6f0e872b..470f60c5a0aa 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -8889,7 +8889,7 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] {EnableDynamicallyScaleVm, AllowDiskOfferingChangeDuringScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties, KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList, DestroyRootVolumeOnVmDestruction, - EnforceStrictResourceLimitHostTagCheck, StrictHostTags, AllowUserForceStopVm}; + EnforceStrictResourceLimitHostTagCheck, StrictHostTags, AllowUserForceStopVm, AllowExposingVmAssignFailureDetails}; } @Override From 601cd3f32a722812c010695022228306e33a288b Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 4 Aug 2026 11:58:15 +0200 Subject: [PATCH 2/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- api/src/main/java/com/cloud/vm/UserVmService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/com/cloud/vm/UserVmService.java b/api/src/main/java/com/cloud/vm/UserVmService.java index 184fbd83c30b..13d38490bee2 100644 --- a/api/src/main/java/com/cloud/vm/UserVmService.java +++ b/api/src/main/java/com/cloud/vm/UserVmService.java @@ -66,7 +66,7 @@ public interface UserVmService { ConfigKey AllowExposingVmAssignFailureDetails = new ConfigKey<>("Advanced", Boolean.class, - "vm.assign.failure.detailed.message.enabled", "true", + "vm.assign.failure.detailed.message.enabled", "false", "If true, the reason a VM ownership change (assignVirtualMachine) fails, e.g. remaining port forwarding rules or volume snapshots, is returned to the API caller. " + "If false, only a generic error message referencing the Instance ID is returned, and the detailed reason is only available in the management server logs.", true, ConfigKey.Scope.Global); From c05e12c50fd6488c299ac1ea7388d49ff50a275c Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 4 Aug 2026 12:00:41 +0200 Subject: [PATCH 3/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../apache/cloudstack/api/command/admin/vm/AssignVMCmd.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java index dd726c339a88..7ad17b634fef 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java @@ -127,10 +127,10 @@ public void execute() { setResponseObject(response); } catch (Exception e) { ApiErrorCode errorCode = e instanceof InvalidParameterValueException ? ApiErrorCode.PARAM_ERROR : ApiErrorCode.INTERNAL_ERROR; - String msg = String.format("Failed to move Instance due to [%s].", getVmId()); + String msg = String.format("Failed to move Instance [%s].", getVmId()); logger.error(msg, e); if (e instanceof InvalidParameterValueException && UserVmService.AllowExposingVmAssignFailureDetails.value()) { - msg = e.getMessage(); + msg = String.format("Failed to move Instance [%s]: %s", getVmId(), e.getMessage()); } throw new ServerApiException(errorCode, msg); } From 577eb86adca69ea8296490594a6ba4c9a967a9ec Mon Sep 17 00:00:00 2001 From: dahn Date: Tue, 4 Aug 2026 19:31:16 +0200 Subject: [PATCH 4/6] Apply suggestion from @DaanHoogland --- .../org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java index 7ad17b634fef..c9ba04858e8a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java @@ -129,7 +129,7 @@ public void execute() { ApiErrorCode errorCode = e instanceof InvalidParameterValueException ? ApiErrorCode.PARAM_ERROR : ApiErrorCode.INTERNAL_ERROR; String msg = String.format("Failed to move Instance [%s].", getVmId()); logger.error(msg, e); - if (e instanceof InvalidParameterValueException && UserVmService.AllowExposingVmAssignFailureDetails.value()) { + if (e instanceof InvalidParameterValueException && Boolean.TRUE.equals(UserVmService.AllowExposingVmAssignFailureDetails.value())) { msg = String.format("Failed to move Instance [%s]: %s", getVmId(), e.getMessage()); } throw new ServerApiException(errorCode, msg); From 2a94b9e38d56b376e017d996931d2f12b785aae0 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 5 Aug 2026 17:02:35 +0200 Subject: [PATCH 5/6] tests --- .../api/command/admin/vm/AssignVMCmdTest.java | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java new file mode 100644 index 000000000000..548cb9b56cda --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java @@ -0,0 +1,109 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.admin.vm; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.vm.UserVmService; + +public class AssignVMCmdTest { + + private AssignVMCmd assignVMCmd; + private UserVmService userVmService; + private ConfigDepotImpl configDepot; + + @Before + public void setUp() { + assignVMCmd = new AssignVMCmd() { + @Override + public Long getVmId() { + return 1L; + } + }; + + userVmService = mock(UserVmService.class); + assignVMCmd._userVmService = userVmService; + + configDepot = mock(ConfigDepotImpl.class); + ConfigKey.init(configDepot); + } + + @After + public void tearDown() { + ConfigKey.init(null); + } + + private void setDetailedFailureMessageFlag(boolean enabled) { + when(configDepot.getConfigStringValue(UserVmService.AllowExposingVmAssignFailureDetails.key(), + ConfigKey.Scope.Global, null)).thenReturn(Boolean.toString(enabled)); + } + + @Test + public void testExecuteReturnsGenericMessageWhenDetailsDisabled() throws Exception { + setDetailedFailureMessageFlag(false); + when(userVmService.moveVmToUser(assignVMCmd)).thenThrow(new InvalidParameterValueException("account over resource limit")); + + try { + assignVMCmd.execute(); + fail("Expected a ServerApiException to be thrown"); + } catch (ServerApiException e) { + assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); + assertEquals("Failed to move Instance [1].", e.getDescription()); + } + } + + @Test + public void testExecuteReturnsDetailedMessageWhenDetailsEnabledAndParamInvalid() throws Exception { + setDetailedFailureMessageFlag(true); + when(userVmService.moveVmToUser(assignVMCmd)).thenThrow(new InvalidParameterValueException("account over resource limit")); + + try { + assignVMCmd.execute(); + fail("Expected a ServerApiException to be thrown"); + } catch (ServerApiException e) { + assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); + assertEquals("Failed to move Instance [1]: account over resource limit", e.getDescription()); + } + } + + @Test + public void testExecuteReturnsGenericMessageForNonParamExceptionEvenWhenDetailsEnabled() throws Exception { + setDetailedFailureMessageFlag(true); + when(userVmService.moveVmToUser(assignVMCmd)).thenThrow(new ResourceUnavailableException("host unreachable", AssignVMCmd.class, 1L)); + + try { + assignVMCmd.execute(); + fail("Expected a ServerApiException to be thrown"); + } catch (ServerApiException e) { + assertEquals(ApiErrorCode.INTERNAL_ERROR, e.getErrorCode()); + assertEquals("Failed to move Instance [1].", e.getDescription()); + } + } +} From 29f1ccbc2e0b26ec5db2c56e94671e5645548da3 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 12 Nov 2026 10:51:18 +0100 Subject: [PATCH 6/6] Use Instance UUID in assignVirtualMachine failure messages --- .../api/command/admin/vm/AssignVMCmd.java | 9 +++++++-- .../api/command/admin/vm/AssignVMCmdTest.java | 14 +++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java index c9ba04858e8a..6069bcd50105 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java @@ -94,6 +94,10 @@ public Long getVmId() { return virtualMachineId; } + private String getVmUUID() { + return _uuidMgr.getUuid(VirtualMachine.class, getVmId()); + } + public String getAccountName() { return accountName; } @@ -127,10 +131,11 @@ public void execute() { setResponseObject(response); } catch (Exception e) { ApiErrorCode errorCode = e instanceof InvalidParameterValueException ? ApiErrorCode.PARAM_ERROR : ApiErrorCode.INTERNAL_ERROR; - String msg = String.format("Failed to move Instance [%s].", getVmId()); + String vmUuid = getVmUUID(); + String msg = String.format("Failed to move Instance [%s].", vmUuid); logger.error(msg, e); if (e instanceof InvalidParameterValueException && Boolean.TRUE.equals(UserVmService.AllowExposingVmAssignFailureDetails.value())) { - msg = String.format("Failed to move Instance [%s]: %s", getVmId(), e.getMessage()); + msg = String.format("Failed to move Instance [%s]: %s", vmUuid, e.getMessage()); } throw new ServerApiException(errorCode, msg); } diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java index 548cb9b56cda..695a3c313fc6 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmdTest.java @@ -31,10 +31,14 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.utils.db.UUIDManager; import com.cloud.vm.UserVmService; +import com.cloud.vm.VirtualMachine; public class AssignVMCmdTest { + private static final String VM_UUID = "8f6c1c9e-3a2b-4d5e-9f01-23456789abcd"; + private AssignVMCmd assignVMCmd; private UserVmService userVmService; private ConfigDepotImpl configDepot; @@ -51,6 +55,10 @@ public Long getVmId() { userVmService = mock(UserVmService.class); assignVMCmd._userVmService = userVmService; + UUIDManager uuidMgr = mock(UUIDManager.class); + when(uuidMgr.getUuid(VirtualMachine.class, 1L)).thenReturn(VM_UUID); + assignVMCmd._uuidMgr = uuidMgr; + configDepot = mock(ConfigDepotImpl.class); ConfigKey.init(configDepot); } @@ -75,7 +83,7 @@ public void testExecuteReturnsGenericMessageWhenDetailsDisabled() throws Excepti fail("Expected a ServerApiException to be thrown"); } catch (ServerApiException e) { assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); - assertEquals("Failed to move Instance [1].", e.getDescription()); + assertEquals("Failed to move Instance [" + VM_UUID + "].", e.getDescription()); } } @@ -89,7 +97,7 @@ public void testExecuteReturnsDetailedMessageWhenDetailsEnabledAndParamInvalid() fail("Expected a ServerApiException to be thrown"); } catch (ServerApiException e) { assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); - assertEquals("Failed to move Instance [1]: account over resource limit", e.getDescription()); + assertEquals("Failed to move Instance [" + VM_UUID + "]: account over resource limit", e.getDescription()); } } @@ -103,7 +111,7 @@ public void testExecuteReturnsGenericMessageForNonParamExceptionEvenWhenDetailsE fail("Expected a ServerApiException to be thrown"); } catch (ServerApiException e) { assertEquals(ApiErrorCode.INTERNAL_ERROR, e.getErrorCode()); - assertEquals("Failed to move Instance [1].", e.getDescription()); + assertEquals("Failed to move Instance [" + VM_UUID + "].", e.getDescription()); } } }