Skip to content

Commit 0466bf9

Browse files
authored
server,api,ui: host auto-select for migrateVirtualMachineWithVolume (#7554)
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent cb4e35f commit 0466bf9

File tree

5 files changed

+307
-85
lines changed

5 files changed

+307
-85
lines changed

api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ public class MigrateVirtualMachineWithVolumeCmd extends BaseAsyncCmd {
8383
"<1b331390-59f2-4796-9993-bf11c6e76225>&migrateto[2].pool=<41fdb564-9d3b-447d-88ed-7628f7640cbc>")
8484
private Map migrateVolumeTo;
8585

86+
@Parameter(name = ApiConstants.AUTO_SELECT,
87+
since = "4.19.0",
88+
type = CommandType.BOOLEAN,
89+
description = "Automatically select a destination host for a running instance, if hostId is not specified. false by default")
90+
private Boolean autoSelect;
91+
8692
/////////////////////////////////////////////////////
8793
/////////////////// Accessors ///////////////////////
8894
/////////////////////////////////////////////////////
@@ -144,31 +150,39 @@ public ApiCommandResourceType getApiResourceType() {
144150
return ApiCommandResourceType.VirtualMachine;
145151
}
146152

153+
private Host getDestinationHost() {
154+
if (getHostId() == null) {
155+
return null;
156+
}
157+
Host destinationHost = _resourceService.getHost(getHostId());
158+
// OfflineVmwareMigration: destination host would have to not be a required parameter for stopped VMs
159+
if (destinationHost == null) {
160+
s_logger.error(String.format("Unable to find the host with ID [%s].", getHostId()));
161+
throw new InvalidParameterValueException("Unable to find the specified host to migrate the VM.");
162+
}
163+
return destinationHost;
164+
}
165+
147166
@Override
148167
public void execute() {
149-
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo)) {
150-
throw new InvalidParameterValueException(String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO));
168+
if (hostId == null && MapUtils.isEmpty(migrateVolumeTo) && !Boolean.TRUE.equals(autoSelect)) {
169+
throw new InvalidParameterValueException(String.format("Either %s or %s must be passed or %s must be true for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO, ApiConstants.AUTO_SELECT));
151170
}
152171

153172
VirtualMachine virtualMachine = _userVmService.getVm(getVirtualMachineId());
154-
if (!VirtualMachine.State.Running.equals(virtualMachine.getState()) && hostId != null) {
173+
if (!VirtualMachine.State.Running.equals(virtualMachine.getState()) && (hostId != null || Boolean.TRUE.equals(autoSelect))) {
155174
throw new InvalidParameterValueException(String.format("%s is not in the Running state to migrate it to the new host.", virtualMachine));
156175
}
157176

158-
if (!VirtualMachine.State.Stopped.equals(virtualMachine.getState()) && hostId == null) {
159-
throw new InvalidParameterValueException(String.format("%s is not in the Stopped state to migrate, use the %s parameter to migrate it to a new host.",
160-
virtualMachine, ApiConstants.HOST_ID));
177+
if (!VirtualMachine.State.Stopped.equals(virtualMachine.getState()) && hostId == null && Boolean.FALSE.equals(autoSelect)) {
178+
throw new InvalidParameterValueException(String.format("%s is not in the Stopped state to migrate, use the %s or %s parameter to migrate it to a new host.",
179+
virtualMachine, ApiConstants.HOST_ID,ApiConstants.AUTO_SELECT));
161180
}
162181

163182
try {
164183
VirtualMachine migratedVm = null;
165-
if (hostId != null) {
166-
Host destinationHost = _resourceService.getHost(getHostId());
167-
// OfflineVmwareMigration: destination host would have to not be a required parameter for stopped VMs
168-
if (destinationHost == null) {
169-
s_logger.error(String.format("Unable to find the host with ID [%s].", getHostId()));
170-
throw new InvalidParameterValueException("Unable to find the specified host to migrate the VM.");
171-
}
184+
if (getHostId() != null || Boolean.TRUE.equals(autoSelect)) {
185+
Host destinationHost = getDestinationHost();
172186
migratedVm = _userVmService.migrateVirtualMachineWithVolume(getVirtualMachineId(), destinationHost, getVolumeToPool());
173187
} else if (MapUtils.isNotEmpty(migrateVolumeTo)) {
174188
migratedVm = _userVmService.vmStorageMigration(getVirtualMachineId(), getVolumeToPool());

api/src/test/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmdTest.java

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,10 @@
1616
// under the License.
1717
package org.apache.cloudstack.api.command.admin.vm;
1818

19-
import com.cloud.exception.InvalidParameterValueException;
20-
import com.cloud.exception.ManagementServerException;
21-
import com.cloud.exception.ResourceUnavailableException;
22-
import com.cloud.exception.VirtualMachineMigrationException;
23-
import com.cloud.host.Host;
24-
import com.cloud.resource.ResourceService;
25-
import com.cloud.uservm.UserVm;
26-
import com.cloud.utils.db.UUIDManager;
27-
import com.cloud.vm.UserVmService;
28-
import com.cloud.vm.VirtualMachine;
19+
import java.util.HashMap;
20+
import java.util.List;
21+
import java.util.Map;
22+
2923
import org.apache.cloudstack.api.ApiConstants;
3024
import org.apache.cloudstack.api.ResponseGenerator;
3125
import org.apache.cloudstack.api.ResponseObject;
@@ -40,13 +34,21 @@
4034
import org.mockito.Mock;
4135
import org.mockito.Mockito;
4236
import org.mockito.Spy;
43-
import org.powermock.modules.junit4.PowerMockRunner;
37+
import org.mockito.junit.MockitoJUnitRunner;
4438
import org.springframework.test.util.ReflectionTestUtils;
4539

46-
import java.util.List;
47-
import java.util.Map;
40+
import com.cloud.exception.InvalidParameterValueException;
41+
import com.cloud.exception.ManagementServerException;
42+
import com.cloud.exception.ResourceUnavailableException;
43+
import com.cloud.exception.VirtualMachineMigrationException;
44+
import com.cloud.host.Host;
45+
import com.cloud.resource.ResourceService;
46+
import com.cloud.uservm.UserVm;
47+
import com.cloud.utils.db.UUIDManager;
48+
import com.cloud.vm.UserVmService;
49+
import com.cloud.vm.VirtualMachine;
4850

49-
@RunWith(PowerMockRunner.class)
51+
@RunWith(MockitoJUnitRunner.class)
5052
public class MigrateVirtualMachineWithVolumeCmdTest {
5153
@Mock
5254
UserVmService userVmServiceMock;
@@ -68,44 +70,46 @@ public class MigrateVirtualMachineWithVolumeCmdTest {
6870

6971
@Spy
7072
@InjectMocks
71-
MigrateVirtualMachineWithVolumeCmd cmdSpy = new MigrateVirtualMachineWithVolumeCmd();
73+
MigrateVirtualMachineWithVolumeCmd cmdSpy;
7274

7375
private Long hostId = 1L;
74-
private Long virtualMachineUuid = 1L;
76+
private Long virtualMachineId = 1L;
7577
private String virtualMachineName = "VM-name";
76-
private Map<String, String> migrateVolumeTo = Map.of("key","value");
78+
private Map<String, String> migrateVolumeTo = null;
7779
private SystemVmResponse systemVmResponse = new SystemVmResponse();
7880
private UserVmResponse userVmResponse = new UserVmResponse();
7981

8082
@Before
81-
public void setup() {
82-
Mockito.when(cmdSpy.getVirtualMachineId()).thenReturn(virtualMachineUuid);
83-
Mockito.when(cmdSpy.getHostId()).thenReturn(hostId);
84-
Mockito.when(cmdSpy.getVolumeToPool()).thenReturn(migrateVolumeTo);
83+
public void setUp() throws Exception {
84+
ReflectionTestUtils.setField(cmdSpy, "virtualMachineId", virtualMachineId);
85+
migrateVolumeTo = new HashMap<>();
86+
migrateVolumeTo.put("volume", "abc");
87+
migrateVolumeTo.put("pool", "xyz");
8588
}
8689

8790
@Test
88-
public void executeTestHostIdIsNullAndMigrateVolumeToIsNullThrowsInvalidParameterValueException(){
91+
public void executeTestRequiredArgsNullThrowsInvalidParameterValueException() {
8992
ReflectionTestUtils.setField(cmdSpy, "hostId", null);
9093
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", null);
94+
ReflectionTestUtils.setField(cmdSpy, "autoSelect", null);
9195

9296
try {
9397
cmdSpy.execute();
9498
} catch (Exception e) {
9599
Assert.assertEquals(InvalidParameterValueException.class, e.getClass());
96-
String expected = String.format("Either %s or %s must be passed for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO);
100+
String expected = String.format("Either %s or %s must be passed or %s must be true for migrating the VM.", ApiConstants.HOST_ID, ApiConstants.MIGRATE_TO, ApiConstants.AUTO_SELECT);
97101
Assert.assertEquals(expected , e.getMessage());
98102
}
99103
}
100104

101105
@Test
102-
public void executeTestVMIsStoppedAndHostIdIsNotNullThrowsInvalidParameterValueException(){
106+
public void executeTestVMIsStoppedAndHostIdIsNotNullThrowsInvalidParameterValueException() {
103107
ReflectionTestUtils.setField(cmdSpy, "hostId", hostId);
104108
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
105109

106110
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock);
107111
Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Stopped);
108-
Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineUuid, virtualMachineName));
112+
Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineId, virtualMachineName));
109113

110114
try {
111115
cmdSpy.execute();
@@ -117,33 +121,35 @@ public void executeTestVMIsStoppedAndHostIdIsNotNullThrowsInvalidParameterValueE
117121
}
118122

119123
@Test
120-
public void executeTestVMIsRunningAndHostIdIsNullThrowsInvalidParameterValueException(){
124+
public void executeTestVMIsRunningHostIdIsNullAndAutoSelectIsFalseThrowsInvalidParameterValueException() {
121125
ReflectionTestUtils.setField(cmdSpy, "hostId", null);
126+
ReflectionTestUtils.setField(cmdSpy, "autoSelect", false);
122127
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
123128

124129
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock);
125130
Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Running);
126-
Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineUuid, virtualMachineName));
131+
Mockito.when(virtualMachineMock.toString()).thenReturn(String.format("VM [uuid: %s, name: %s]", virtualMachineId, virtualMachineName));
127132

128133
try {
129134
cmdSpy.execute();
130135
} catch (Exception e) {
131136
Assert.assertEquals(InvalidParameterValueException.class, e.getClass());
132-
String expected = String.format("%s is not in the Stopped state to migrate, use the %s parameter to migrate it to a new host.", virtualMachineMock,
133-
ApiConstants.HOST_ID);
137+
String expected = String.format("%s is not in the Stopped state to migrate, use the %s or %s parameter to migrate it to a new host.", virtualMachineMock,
138+
ApiConstants.HOST_ID, ApiConstants.AUTO_SELECT);
134139
Assert.assertEquals(expected , e.getMessage());
135140
}
136141
}
137142

138143
@Test
139-
public void executeTestHostIdIsNullThrowsInvalidParameterValueException(){
144+
public void executeTestHostIdIsNullThrowsInvalidParameterValueException() {
145+
ReflectionTestUtils.setField(cmdSpy, "virtualMachineId", virtualMachineId);
140146
ReflectionTestUtils.setField(cmdSpy, "hostId", hostId);
141147
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
148+
ReflectionTestUtils.setField(cmdSpy, "autoSelect", false);
142149

143150
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock);
144151
Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Running);
145152
Mockito.when(resourceServiceMock.getHost(Mockito.anyLong())).thenReturn(null);
146-
Mockito.when(uuidManagerMock.getUuid(Host.class, virtualMachineUuid)).thenReturn(virtualMachineUuid.toString());
147153

148154
try {
149155
cmdSpy.execute();
@@ -154,15 +160,22 @@ public void executeTestHostIdIsNullThrowsInvalidParameterValueException(){
154160
}
155161
}
156162

163+
private Map getMockedMigrateVolumeToApiCmdParam() {
164+
Map<String, String> migrateVolumeTo = new HashMap<>();
165+
migrateVolumeTo.put("volume", "abc");
166+
migrateVolumeTo.put("pool", "xyz");
167+
return Map.of("", migrateVolumeTo);
168+
}
169+
157170
@Test
158171
public void executeTestHostIsNotNullMigratedVMIsNullThrowsServerApiException() throws ManagementServerException, ResourceUnavailableException, VirtualMachineMigrationException {
159172
ReflectionTestUtils.setField(cmdSpy, "hostId", hostId);
160-
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
173+
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam());
161174

162175
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock);
163176
Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Running);
164-
Mockito.when(resourceServiceMock.getHost(Mockito.anyLong())).thenReturn(hostMock);
165-
Mockito.when(userVmServiceMock.migrateVirtualMachineWithVolume(virtualMachineUuid, hostMock, migrateVolumeTo)).thenReturn(null);
177+
Mockito.when(resourceServiceMock.getHost(hostId)).thenReturn(hostMock);
178+
Mockito.when(userVmServiceMock.migrateVirtualMachineWithVolume(Mockito.anyLong(), Mockito.any(), Mockito.anyMap())).thenReturn(null);
166179

167180
try {
168181
cmdSpy.execute();
@@ -176,11 +189,11 @@ public void executeTestHostIsNotNullMigratedVMIsNullThrowsServerApiException() t
176189
@Test
177190
public void executeTestHostIsNullMigratedVMIsNullThrowsServerApiException() {
178191
ReflectionTestUtils.setField(cmdSpy, "hostId", null);
179-
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
192+
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam());
180193

181194
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock);
182195
Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Stopped);
183-
Mockito.when(userVmServiceMock.vmStorageMigration(virtualMachineUuid, migrateVolumeTo)).thenReturn(null);
196+
Mockito.when(userVmServiceMock.vmStorageMigration(Mockito.anyLong(), Mockito.anyMap())).thenReturn(null);
184197

185198
try {
186199
cmdSpy.execute();
@@ -194,11 +207,11 @@ public void executeTestHostIsNullMigratedVMIsNullThrowsServerApiException() {
194207
@Test
195208
public void executeTestSystemVMMigratedWithSuccess() {
196209
ReflectionTestUtils.setField(cmdSpy, "hostId", null);
197-
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
210+
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam());
198211

199212
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(virtualMachineMock);
200213
Mockito.when(virtualMachineMock.getState()).thenReturn(VirtualMachine.State.Stopped);
201-
Mockito.when(userVmServiceMock.vmStorageMigration(virtualMachineUuid, migrateVolumeTo)).thenReturn(virtualMachineMock);
214+
Mockito.when(userVmServiceMock.vmStorageMigration(Mockito.anyLong(), Mockito.anyMap())).thenReturn(virtualMachineMock);
202215
Mockito.when(virtualMachineMock.getType()).thenReturn(VirtualMachine.Type.ConsoleProxy);
203216
Mockito.when(responseGeneratorMock.createSystemVmResponse(virtualMachineMock)).thenReturn(systemVmResponse);
204217

@@ -211,11 +224,11 @@ public void executeTestSystemVMMigratedWithSuccess() {
211224
public void executeTestUserVMMigratedWithSuccess() {
212225
UserVm userVmMock = Mockito.mock(UserVm.class);
213226
ReflectionTestUtils.setField(cmdSpy, "hostId", null);
214-
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", migrateVolumeTo);
227+
ReflectionTestUtils.setField(cmdSpy, "migrateVolumeTo", getMockedMigrateVolumeToApiCmdParam());
215228

216229
Mockito.when(userVmServiceMock.getVm(Mockito.anyLong())).thenReturn(userVmMock);
217230
Mockito.when(userVmMock.getState()).thenReturn(VirtualMachine.State.Stopped);
218-
Mockito.when(userVmServiceMock.vmStorageMigration(virtualMachineUuid, migrateVolumeTo)).thenReturn(userVmMock);
231+
Mockito.when(userVmServiceMock.vmStorageMigration(Mockito.anyLong(), Mockito.anyMap())).thenReturn(userVmMock);
219232
Mockito.when(userVmMock.getType()).thenReturn(VirtualMachine.Type.User);
220233
Mockito.when(responseGeneratorMock.createUserVmResponse(ResponseObject.ResponseView.Full, "virtualmachine", userVmMock)).thenReturn(List.of(userVmResponse));
221234

0 commit comments

Comments
 (0)