diff --git a/api/src/main/java/com/cloud/vm/UserVmService.java b/api/src/main/java/com/cloud/vm/UserVmService.java index dc9e8c1f0d8c..13d38490bee2 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", "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); /** * 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..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 @@ -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", @@ -126,8 +127,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 due to [%s].", getVmId()); + String msg = String.format("Failed to move Instance [%s].", getVmId()); 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()); + } 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 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()); + } + } +} diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 7e58cd01050a..9238d9bfec06 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -8891,7 +8891,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