Skip to content

Commit 0014315

Browse files
shwstpprowsferraro
shwstppr
authored and
owsferraro
committed
api,server: allow cleaning up vm extraconfig (apache#11974)
1 parent 3dce6ef commit 0014315

File tree

8 files changed

+196
-55
lines changed

8 files changed

+196
-55
lines changed

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,7 @@ public class ApiConstants {
11681168
public static final String OVM3_VIP = "ovm3vip";
11691169
public static final String CLEAN_UP_DETAILS = "cleanupdetails";
11701170
public static final String CLEAN_UP_EXTERNAL_DETAILS = "cleanupexternaldetails";
1171+
public static final String CLEAN_UP_EXTRA_CONFIG = "cleanupextraconfig";
11711172
public static final String CLEAN_UP_PARAMETERS = "cleanupparameters";
11721173
public static final String VIRTUAL_SIZE = "virtualsize";
11731174
public static final String NETSCALER_CONTROLCENTER_ID = "netscalercontrolcenterid";

‎api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction,
163163
description = "Lease expiry action, valid values are STOP and DESTROY")
164164
private String leaseExpiryAction;
165165

166+
@Parameter(name = ApiConstants.CLEAN_UP_EXTRA_CONFIG, type = CommandType.BOOLEAN, since = "4.23.0",
167+
description = "Optional boolean field, which indicates if extraconfig for the instance should be " +
168+
"cleaned up or not (If set to true, extraconfig removed for this instance, extraconfig field " +
169+
"ignored; if false or not set, no action)")
170+
private Boolean cleanupExtraConfig;
171+
166172
/////////////////////////////////////////////////////
167173
/////////////////// Accessors ///////////////////////
168174
/////////////////////////////////////////////////////
@@ -274,14 +280,18 @@ public String getExtraConfig() {
274280
return extraConfig;
275281
}
276282

277-
/////////////////////////////////////////////////////
278-
/////////////// API Implementation///////////////////
279-
/////////////////////////////////////////////////////
280-
281283
public Long getOsTypeId() {
282284
return osTypeId;
283285
}
284286

287+
public boolean isCleanupExtraConfig() {
288+
return Boolean.TRUE.equals(cleanupExtraConfig);
289+
}
290+
291+
/////////////////////////////////////////////////////
292+
/////////////// API Implementation///////////////////
293+
/////////////////////////////////////////////////////
294+
285295
@Override
286296
public String getCommandName() {
287297
return s_name;

‎engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDao.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@
2222
import com.cloud.vm.VMInstanceDetailVO;
2323

2424
public interface VMInstanceDetailsDao extends GenericDao<VMInstanceDetailVO, Long>, ResourceDetailsDao<VMInstanceDetailVO> {
25+
int removeDetailsWithPrefix(long vmId, String prefix);
2526
}

‎engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@
1717
package com.cloud.vm.dao;
1818

1919

20+
import org.apache.commons.lang3.StringUtils;
2021
import org.springframework.stereotype.Component;
2122

2223
import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
2324

25+
import com.cloud.utils.db.SearchBuilder;
26+
import com.cloud.utils.db.SearchCriteria;
2427
import com.cloud.vm.VMInstanceDetailVO;
2528

2629
@Component
@@ -31,4 +34,18 @@ public void addDetail(long resourceId, String key, String value, boolean display
3134
super.addDetail(new VMInstanceDetailVO(resourceId, key, value, display));
3235
}
3336

37+
@Override
38+
public int removeDetailsWithPrefix(long vmId, String prefix) {
39+
if (StringUtils.isBlank(prefix)) {
40+
return 0;
41+
}
42+
SearchBuilder<VMInstanceDetailVO> sb = createSearchBuilder();
43+
sb.and("vmId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
44+
sb.and("prefix", sb.entity().getName(), SearchCriteria.Op.LIKE);
45+
sb.done();
46+
SearchCriteria<VMInstanceDetailVO> sc = sb.create();
47+
sc.setParameters("vmId", vmId);
48+
sc.setParameters("prefix", prefix + "%");
49+
return super.remove(sc);
50+
}
3451
}

‎engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDetailsDaoImplTest.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package com.cloud.vm.dao;
19+
20+
import static org.mockito.ArgumentMatchers.any;
21+
import static org.mockito.ArgumentMatchers.anyString;
22+
import static org.mockito.Mockito.doReturn;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.when;
25+
26+
import org.junit.Assert;
27+
import org.junit.Test;
28+
import org.junit.runner.RunWith;
29+
import org.mockito.InjectMocks;
30+
import org.mockito.Mockito;
31+
import org.mockito.Spy;
32+
import org.mockito.junit.MockitoJUnitRunner;
33+
34+
import com.cloud.utils.db.SearchBuilder;
35+
import com.cloud.utils.db.SearchCriteria;
36+
import com.cloud.vm.VMInstanceDetailVO;
37+
38+
@RunWith(MockitoJUnitRunner.class)
39+
public class VMInstanceDetailsDaoImplTest {
40+
@Spy
41+
@InjectMocks
42+
private VMInstanceDetailsDaoImpl vmInstanceDetailsDaoImpl;
43+
44+
@Test
45+
public void removeDetailsWithPrefixReturnsZeroWhenPrefixIsBlank() {
46+
Assert.assertEquals(0, vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, ""));
47+
Assert.assertEquals(0, vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, " "));
48+
Assert.assertEquals(0, vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, null));
49+
}
50+
51+
@Test
52+
public void removeDetailsWithPrefixRemovesMatchingDetails() {
53+
SearchBuilder<VMInstanceDetailVO> sb = mock(SearchBuilder.class);
54+
VMInstanceDetailVO entity = mock(VMInstanceDetailVO.class);
55+
when(sb.entity()).thenReturn(entity);
56+
when(sb.and(anyString(), any(), any(SearchCriteria.Op.class))).thenReturn(sb);
57+
SearchCriteria<VMInstanceDetailVO> sc = mock(SearchCriteria.class);
58+
when(sb.create()).thenReturn(sc);
59+
when(vmInstanceDetailsDaoImpl.createSearchBuilder()).thenReturn(sb);
60+
doReturn(3).when(vmInstanceDetailsDaoImpl).remove(sc);
61+
int removedCount = vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, "testPrefix");
62+
Assert.assertEquals(3, removedCount);
63+
Mockito.verify(sc).setParameters("vmId", 1L);
64+
Mockito.verify(sc).setParameters("prefix", "testPrefix%");
65+
Mockito.verify(vmInstanceDetailsDaoImpl, Mockito.times(1)).remove(sc);
66+
}
67+
68+
@Test
69+
public void removeDetailsWithPrefixDoesNotRemoveWhenNoMatch() {
70+
SearchBuilder<VMInstanceDetailVO> sb = mock(SearchBuilder.class);
71+
VMInstanceDetailVO entity = mock(VMInstanceDetailVO.class);
72+
when(sb.entity()).thenReturn(entity);
73+
when(sb.and(anyString(), any(), any(SearchCriteria.Op.class))).thenReturn(sb);
74+
SearchCriteria<VMInstanceDetailVO> sc = mock(SearchCriteria.class);
75+
when(sb.create()).thenReturn(sc);
76+
when(vmInstanceDetailsDaoImpl.createSearchBuilder()).thenReturn(sb);
77+
doReturn(0).when(vmInstanceDetailsDaoImpl).remove(sc);
78+
79+
int removedCount = vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, "nonExistentPrefix");
80+
81+
Assert.assertEquals(0, removedCount);
82+
Mockito.verify(sc).setParameters("vmId", 1L);
83+
Mockito.verify(sc).setParameters("prefix", "nonExistentPrefix%");
84+
Mockito.verify(vmInstanceDetailsDaoImpl, Mockito.times(1)).remove(sc);
85+
}
86+
}

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2882,6 +2882,22 @@ private void adjustVmLimits(Account owner, UserVmVO vmInstance, ServiceOfferingV
28822882
}
28832883
}
28842884

2885+
protected void updateVmExtraConfig(UserVmVO userVm, String extraConfig, boolean cleanupExtraConfig) {
2886+
if (cleanupExtraConfig) {
2887+
logger.info("Cleaning up extraconfig from user vm: {}", userVm.getUuid());
2888+
vmInstanceDetailsDao.removeDetailsWithPrefix(userVm.getId(), ApiConstants.EXTRA_CONFIG);
2889+
return;
2890+
}
2891+
if (StringUtils.isNotBlank(extraConfig)) {
2892+
if (EnableAdditionalVmConfig.valueIn(userVm.getAccountId())) {
2893+
logger.info("Adding extra configuration to user vm: {}", userVm.getUuid());
2894+
addExtraConfig(userVm, extraConfig);
2895+
} else {
2896+
throw new InvalidParameterValueException("attempted setting extraconfig but enable.additional.vm.configuration is disabled");
2897+
}
2898+
}
2899+
}
2900+
28852901
@Override
28862902
@ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription = "updating Vm")
28872903
public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException {
@@ -2899,6 +2915,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
28992915
List<Long> securityGroupIdList = getSecurityGroupIdList(cmd);
29002916
boolean cleanupDetails = cmd.isCleanupDetails();
29012917
String extraConfig = cmd.getExtraConfig();
2918+
boolean cleanupExtraConfig = cmd.isCleanupExtraConfig();
29022919

29032920
UserVmVO vmInstance = _vmDao.findById(cmd.getId());
29042921
VMTemplateVO template = _templateDao.findById(vmInstance.getTemplateId());
@@ -2931,10 +2948,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
29312948
.map(item -> (item).trim())
29322949
.collect(Collectors.toList());
29332950
List<VMInstanceDetailVO> existingDetails = vmInstanceDetailsDao.listDetails(id);
2934-
if (cleanupDetails){
2935-
if (template != null && template.isDeployAsIs()) {
2936-
throw new InvalidParameterValueException("Detail settings are read from OVA, it cannot be cleaned up by API call.");
2937-
}
2951+
if (cleanupDetails) {
29382952
if (caller != null && caller.getType() == Account.Type.ADMIN) {
29392953
for (final VMInstanceDetailVO detail : existingDetails) {
29402954
if (detail != null && detail.isDisplay() && !isExtraConfig(detail.getName())) {
@@ -3014,15 +3028,8 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
30143028
vmInstance.setDetails(details);
30153029
_vmDao.saveDetails(vmInstance);
30163030
}
3017-
if (StringUtils.isNotBlank(extraConfig)) {
3018-
if (EnableAdditionalVmConfig.valueIn(accountId)) {
3019-
logger.info("Adding extra configuration to user vm: " + vmInstance.getUuid());
3020-
addExtraConfig(vmInstance, extraConfig);
3021-
} else {
3022-
throw new InvalidParameterValueException("attempted setting extraconfig but enable.additional.vm.configuration is disabled");
3023-
}
3024-
}
30253031
}
3032+
updateVmExtraConfig(userVm, extraConfig, cleanupExtraConfig);
30263033

30273034
if (VMLeaseManager.InstanceLeaseEnabled.value() && cmd.getLeaseDuration() != null) {
30283035
applyLeaseOnUpdateInstance(vmInstance, cmd.getLeaseDuration(), cmd.getLeaseExpiryAction());

0 commit comments

Comments
 (0)