This is an automated email from the ASF dual-hosted git repository.
vishesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 18972caf5f1 api,server: allow cleaning up vm extraconfig (#11974)
18972caf5f1 is described below
commit 18972caf5f1cfc90bdad59669dc7f8cb031ba64d
Author: Abhishek Kumar <[email protected]>
AuthorDate: Fri Jan 30 13:54:01 2026 +0530
api,server: allow cleaning up vm extraconfig (#11974)
---
.../org/apache/cloudstack/api/ApiConstants.java | 1 +
.../api/command/user/vm/UpdateVMCmd.java | 18 ++++-
.../com/cloud/vm/dao/VMInstanceDetailsDao.java | 1 +
.../com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java | 17 +++++
.../cloud/vm/dao/VMInstanceDetailsDaoImplTest.java | 86 ++++++++++++++++++++++
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 28 ++++---
.../java/com/cloud/vm/UserVmManagerImplTest.java | 75 ++++++++++++++++++-
ui/src/views/compute/EditVM.vue | 2 +
8 files changed, 212 insertions(+), 16 deletions(-)
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index 790070e5244..1ab6fba6081 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -1168,6 +1168,7 @@ public class ApiConstants {
public static final String OVM3_VIP = "ovm3vip";
public static final String CLEAN_UP_DETAILS = "cleanupdetails";
public static final String CLEAN_UP_EXTERNAL_DETAILS =
"cleanupexternaldetails";
+ public static final String CLEAN_UP_EXTRA_CONFIG = "cleanupextraconfig";
public static final String CLEAN_UP_PARAMETERS = "cleanupparameters";
public static final String VIRTUAL_SIZE = "virtualsize";
public static final String NETSCALER_CONTROLCENTER_ID =
"netscalercontrolcenterid";
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java
index ddc2c06eb09..e0da954d381 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java
@@ -163,6 +163,12 @@ public class UpdateVMCmd extends BaseCustomIdCmd
implements SecurityGroupAction,
description = "Lease expiry action, valid values are STOP and
DESTROY")
private String leaseExpiryAction;
+ @Parameter(name = ApiConstants.CLEAN_UP_EXTRA_CONFIG, type =
CommandType.BOOLEAN, since = "4.23.0",
+ description = "Optional boolean field, which indicates if
extraconfig for the instance should be " +
+ "cleaned up or not (If set to true, extraconfig removed
for this instance, extraconfig field " +
+ "ignored; if false or not set, no action)")
+ private Boolean cleanupExtraConfig;
+
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@@ -271,14 +277,18 @@ public class UpdateVMCmd extends BaseCustomIdCmd
implements SecurityGroupAction,
return extraConfig;
}
- /////////////////////////////////////////////////////
- /////////////// API Implementation///////////////////
- /////////////////////////////////////////////////////
-
public Long getOsTypeId() {
return osTypeId;
}
+ public boolean isCleanupExtraConfig() {
+ return Boolean.TRUE.equals(cleanupExtraConfig);
+ }
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
@Override
public String getCommandName() {
return s_name;
diff --git
a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDao.java
b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDao.java
index ea9ac5afba6..4cbdc516ba0 100644
--- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDao.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDao.java
@@ -22,4 +22,5 @@ import com.cloud.utils.db.GenericDao;
import com.cloud.vm.VMInstanceDetailVO;
public interface VMInstanceDetailsDao extends GenericDao<VMInstanceDetailVO,
Long>, ResourceDetailsDao<VMInstanceDetailVO> {
+ int removeDetailsWithPrefix(long vmId, String prefix);
}
diff --git
a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java
b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java
index ca11b005fb2..4c2fdd6f8d4 100644
--- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDetailsDaoImpl.java
@@ -17,10 +17,13 @@
package com.cloud.vm.dao;
+import org.apache.commons.lang3.StringUtils;
import org.springframework.stereotype.Component;
import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
import com.cloud.vm.VMInstanceDetailVO;
@Component
@@ -31,4 +34,18 @@ public class VMInstanceDetailsDaoImpl extends
ResourceDetailsDaoBase<VMInstanceD
super.addDetail(new VMInstanceDetailVO(resourceId, key, value,
display));
}
+ @Override
+ public int removeDetailsWithPrefix(long vmId, String prefix) {
+ if (StringUtils.isBlank(prefix)) {
+ return 0;
+ }
+ SearchBuilder<VMInstanceDetailVO> sb = createSearchBuilder();
+ sb.and("vmId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
+ sb.and("prefix", sb.entity().getName(), SearchCriteria.Op.LIKE);
+ sb.done();
+ SearchCriteria<VMInstanceDetailVO> sc = sb.create();
+ sc.setParameters("vmId", vmId);
+ sc.setParameters("prefix", prefix + "%");
+ return super.remove(sc);
+ }
}
diff --git
a/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDetailsDaoImplTest.java
b/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDetailsDaoImplTest.java
new file mode 100644
index 00000000000..06238e386d5
--- /dev/null
+++
b/engine/schema/src/test/java/com/cloud/vm/dao/VMInstanceDetailsDaoImplTest.java
@@ -0,0 +1,86 @@
+// 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 com.cloud.vm.dao;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.vm.VMInstanceDetailVO;
+
+@RunWith(MockitoJUnitRunner.class)
+public class VMInstanceDetailsDaoImplTest {
+ @Spy
+ @InjectMocks
+ private VMInstanceDetailsDaoImpl vmInstanceDetailsDaoImpl;
+
+ @Test
+ public void removeDetailsWithPrefixReturnsZeroWhenPrefixIsBlank() {
+ Assert.assertEquals(0,
vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, ""));
+ Assert.assertEquals(0,
vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, " "));
+ Assert.assertEquals(0,
vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, null));
+ }
+
+ @Test
+ public void removeDetailsWithPrefixRemovesMatchingDetails() {
+ SearchBuilder<VMInstanceDetailVO> sb = mock(SearchBuilder.class);
+ VMInstanceDetailVO entity = mock(VMInstanceDetailVO.class);
+ when(sb.entity()).thenReturn(entity);
+ when(sb.and(anyString(), any(),
any(SearchCriteria.Op.class))).thenReturn(sb);
+ SearchCriteria<VMInstanceDetailVO> sc = mock(SearchCriteria.class);
+ when(sb.create()).thenReturn(sc);
+ when(vmInstanceDetailsDaoImpl.createSearchBuilder()).thenReturn(sb);
+ doReturn(3).when(vmInstanceDetailsDaoImpl).remove(sc);
+ int removedCount =
vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, "testPrefix");
+ Assert.assertEquals(3, removedCount);
+ Mockito.verify(sc).setParameters("vmId", 1L);
+ Mockito.verify(sc).setParameters("prefix", "testPrefix%");
+ Mockito.verify(vmInstanceDetailsDaoImpl, Mockito.times(1)).remove(sc);
+ }
+
+ @Test
+ public void removeDetailsWithPrefixDoesNotRemoveWhenNoMatch() {
+ SearchBuilder<VMInstanceDetailVO> sb = mock(SearchBuilder.class);
+ VMInstanceDetailVO entity = mock(VMInstanceDetailVO.class);
+ when(sb.entity()).thenReturn(entity);
+ when(sb.and(anyString(), any(),
any(SearchCriteria.Op.class))).thenReturn(sb);
+ SearchCriteria<VMInstanceDetailVO> sc = mock(SearchCriteria.class);
+ when(sb.create()).thenReturn(sc);
+ when(vmInstanceDetailsDaoImpl.createSearchBuilder()).thenReturn(sb);
+ doReturn(0).when(vmInstanceDetailsDaoImpl).remove(sc);
+
+ int removedCount =
vmInstanceDetailsDaoImpl.removeDetailsWithPrefix(1L, "nonExistentPrefix");
+
+ Assert.assertEquals(0, removedCount);
+ Mockito.verify(sc).setParameters("vmId", 1L);
+ Mockito.verify(sc).setParameters("prefix", "nonExistentPrefix%");
+ Mockito.verify(vmInstanceDetailsDaoImpl, Mockito.times(1)).remove(sc);
+ }
+}
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index eccea944fe6..b9b6e5a6839 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -2866,6 +2866,22 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
}
}
+ protected void updateVmExtraConfig(UserVmVO userVm, String extraConfig,
boolean cleanupExtraConfig) {
+ if (cleanupExtraConfig) {
+ logger.info("Cleaning up extraconfig from user vm: {}",
userVm.getUuid());
+ vmInstanceDetailsDao.removeDetailsWithPrefix(userVm.getId(),
ApiConstants.EXTRA_CONFIG);
+ return;
+ }
+ if (StringUtils.isNotBlank(extraConfig)) {
+ if (EnableAdditionalVmConfig.valueIn(userVm.getAccountId())) {
+ logger.info("Adding extra configuration to user vm: {}",
userVm.getUuid());
+ addExtraConfig(userVm, extraConfig);
+ } else {
+ throw new InvalidParameterValueException("attempted setting
extraconfig but enable.additional.vm.configuration is disabled");
+ }
+ }
+ }
+
@Override
@ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription =
"updating Vm")
public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws
ResourceUnavailableException, InsufficientCapacityException {
@@ -2883,6 +2899,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
List<Long> securityGroupIdList = getSecurityGroupIdList(cmd);
boolean cleanupDetails = cmd.isCleanupDetails();
String extraConfig = cmd.getExtraConfig();
+ boolean cleanupExtraConfig = cmd.isCleanupExtraConfig();
UserVmVO vmInstance = _vmDao.findById(cmd.getId());
VMTemplateVO template =
_templateDao.findById(vmInstance.getTemplateId());
@@ -2919,7 +2936,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
.map(item -> (item).trim())
.collect(Collectors.toList());
List<VMInstanceDetailVO> existingDetails =
vmInstanceDetailsDao.listDetails(id);
- if (cleanupDetails){
+ if (cleanupDetails) {
if (caller != null && caller.getType() == Account.Type.ADMIN) {
for (final VMInstanceDetailVO detail : existingDetails) {
if (detail != null && detail.isDisplay() &&
!isExtraConfig(detail.getName())) {
@@ -2982,15 +2999,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
vmInstance.setDetails(details);
_vmDao.saveDetails(vmInstance);
}
- if (StringUtils.isNotBlank(extraConfig)) {
- if (EnableAdditionalVmConfig.valueIn(accountId)) {
- logger.info("Adding extra configuration to user vm: " +
vmInstance.getUuid());
- addExtraConfig(vmInstance, extraConfig);
- } else {
- throw new InvalidParameterValueException("attempted
setting extraconfig but enable.additional.vm.configuration is disabled");
- }
- }
}
+ updateVmExtraConfig(userVm, extraConfig, cleanupExtraConfig);
if (VMLeaseManager.InstanceLeaseEnabled.value() &&
cmd.getLeaseDuration() != null) {
applyLeaseOnUpdateInstance(vmInstance, cmd.getLeaseDuration(),
cmd.getLeaseExpiryAction());
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index 4ec9caab5aa..4edafb3a05a 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -40,8 +40,10 @@ import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
+import java.lang.reflect.Field;
import java.text.SimpleDateFormat;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
@@ -59,8 +61,6 @@ import java.util.Map;
import java.util.TimeZone;
import java.util.UUID;
-import com.cloud.domain.Domain;
-import com.cloud.storage.dao.SnapshotPolicyDao;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.api.ApiCommandResourceType;
@@ -88,6 +88,7 @@ import
org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
import org.apache.cloudstack.storage.template.VnfTemplateManager;
@@ -117,6 +118,7 @@ import com.cloud.deploy.DataCenterDeployment;
import com.cloud.deploy.DeployDestination;
import com.cloud.deploy.DeploymentPlanner;
import com.cloud.deploy.DeploymentPlanningManager;
+import com.cloud.domain.Domain;
import com.cloud.domain.DomainVO;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.ActionEventUtils;
@@ -142,9 +144,9 @@ import com.cloud.network.dao.LoadBalancerVMMapDao;
import com.cloud.network.dao.LoadBalancerVMMapVO;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.dao.NetworkVO;
-import com.cloud.network.element.UserDataServiceProvider;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkVO;
+import com.cloud.network.element.UserDataServiceProvider;
import com.cloud.network.guru.NetworkGuru;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.network.rules.PortForwardingRule;
@@ -172,6 +174,7 @@ import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.GuestOSDao;
import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.SnapshotPolicyDao;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.template.VirtualMachineTemplate;
@@ -467,6 +470,24 @@ public class UserVmManagerImplTest {
Class<InvalidParameterValueException>
expectedInvalidParameterValueException = InvalidParameterValueException.class;
Class<CloudRuntimeException> expectedCloudRuntimeException =
CloudRuntimeException.class;
+ private Map<ConfigKey, Object> originalConfigValues = new HashMap<>();
+
+
+ private void updateDefaultConfigValue(final ConfigKey configKey, final
Object o, boolean revert) {
+ try {
+ final String name = "_defaultValue";
+ Field f = ConfigKey.class.getDeclaredField(name);
+ f.setAccessible(true);
+ String stringVal = String.valueOf(o);
+ if (!revert) {
+ originalConfigValues.put(configKey, f.get(configKey));
+ }
+ f.set(configKey, stringVal);
+ } catch (IllegalAccessException | NoSuchFieldException e) {
+ Assert.fail("Failed to mock config " + configKey.key() + " value
due to " + e.getMessage());
+ }
+ }
+
@Before
public void beforeTest() {
userVmManagerImpl.resourceLimitService = resourceLimitMgr;
@@ -496,6 +517,9 @@ public class UserVmManagerImplTest {
public void afterTest() {
CallContext.unregister();
unmanagedVMsManagerMockedStatic.close();
+ for (Map.Entry<ConfigKey, Object> entry :
originalConfigValues.entrySet()) {
+ updateDefaultConfigValue(entry.getKey(), entry.getValue(), true);
+ }
}
@Test
@@ -4178,4 +4202,49 @@ public class UserVmManagerImplTest {
verify(userVmDao, times(1)).releaseFromLockTable(vmId);
}
+ @Test
+ public void updateVmExtraConfigCleansUpWhenCleanupFlagIsTrue() {
+ UserVmVO userVm = mock(UserVmVO.class);
+ when(userVm.getUuid()).thenReturn("test-uuid");
+ when(userVm.getId()).thenReturn(1L);
+
+ userVmManagerImpl.updateVmExtraConfig(userVm, "someConfig", true);
+
+ verify(vmInstanceDetailsDao, times(1)).removeDetailsWithPrefix(1L,
ApiConstants.EXTRA_CONFIG);
+ verifyNoMoreInteractions(vmInstanceDetailsDao);
+ }
+
+ @Test
+ public void updateVmExtraConfigAddsConfigWhenValidAndEnabled() {
+ UserVmVO userVm = mock(UserVmVO.class);
+ when(userVm.getUuid()).thenReturn("test-uuid");
+ when(userVm.getAccountId()).thenReturn(1L);
+
when(userVm.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+ doNothing().when(userVmManagerImpl).persistExtraConfigKvm(anyString(),
eq(userVm));
+ updateDefaultConfigValue(UserVmManagerImpl.EnableAdditionalVmConfig,
true, false);
+
+ userVmManagerImpl.updateVmExtraConfig(userVm, "validConfig", false);
+
+ verify(vmInstanceDetailsDao,
never()).removeDetailsWithPrefix(anyLong(), anyString());
+ verify(userVmManagerImpl, times(1)).addExtraConfig(userVm,
"validConfig");
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void updateVmExtraConfigThrowsExceptionWhenConfigDisabled() {
+ UserVmVO userVm = mock(UserVmVO.class);
+ when(userVm.getAccountId()).thenReturn(1L);
+ updateDefaultConfigValue(UserVmManagerImpl.EnableAdditionalVmConfig,
false, false);
+
+ userVmManagerImpl.updateVmExtraConfig(userVm, "validConfig", false);
+ }
+
+ @Test
+ public void updateVmExtraConfigDoesNothingWhenExtraConfigIsBlank() {
+ UserVmVO userVm = mock(UserVmVO.class);
+
+ userVmManagerImpl.updateVmExtraConfig(userVm, "", false);
+
+ verify(vmInstanceDetailsDao,
never()).removeDetailsWithPrefix(anyLong(), anyString());
+ verify(userVmManagerImpl, never()).addExtraConfig(any(UserVmVO.class),
anyString());
+ }
}
diff --git a/ui/src/views/compute/EditVM.vue b/ui/src/views/compute/EditVM.vue
index 19055afde97..9f08a3368a2 100644
--- a/ui/src/views/compute/EditVM.vue
+++ b/ui/src/views/compute/EditVM.vue
@@ -425,6 +425,8 @@ export default {
}
if (values.extraconfig && values.extraconfig.length > 0) {
params.extraconfig = encodeURIComponent(values.extraconfig)
+ } else if (this.combinedExtraConfig) {
+ params.cleanupextraconfig = true
}
this.loading = true
