api,server: allow cleaning up vm extraconfig

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

@@ -1165,6 +1165,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";

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 String getExtraConfig() {
         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;

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

@@ -22,4 +22,5 @@
 import com.cloud.vm.VMInstanceDetailVO;
 
 public interface VMInstanceDetailsDao extends GenericDao<VMInstanceDetailVO, Long>, ResourceDetailsDao<VMInstanceDetailVO> {
+    int removeDetailsWithPrefix(long vmId, String prefix);
 }

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 void addDetail(long resourceId, String key, String value, boolean display
         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);
+    }
 }

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);
+    }
+}

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

@@ -2866,6 +2866,22 @@ private void adjustVmLimits(Account owner, UserVmVO vmInstance, ServiceOfferingV
         }
     }
 
+    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 UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
         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 UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
                 .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,14 +2999,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
                 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) {

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -40,8 +40,10 @@
 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;
@@ -87,6 +89,7 @@
 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;
@@ -466,6 +469,18 @@ public class UserVmManagerImplTest {
     Class<InvalidParameterValueException> expectedInvalidParameterValueException = InvalidParameterValueException.class;
     Class<CloudRuntimeException> expectedCloudRuntimeException = CloudRuntimeException.class;
 
+
+    private void overrideDefaultConfigValue(final ConfigKey configKey, final Object o) {
+        try {
+            final String name = "_defaultValue";
+            Field f = ConfigKey.class.getDeclaredField(name);
+            f.setAccessible(true);
+            f.set(configKey, String.valueOf(o));
+        } catch (IllegalAccessException | NoSuchFieldException  e) {
+            Assert.fail("Failed to mock config " + configKey.key() + " value due to " + e.getMessage());
+        }
+    }
+
     @Before
     public void beforeTest() {
         userVmManagerImpl.resourceLimitService = resourceLimitMgr;
@@ -4177,4 +4192,49 @@ public void testUnmanageUserVMSuccess() {
         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));
+        overrideDefaultConfigValue(UserVmManagerImpl.EnableAdditionalVmConfig, true);
+
+        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);
+        overrideDefaultConfigValue(UserVmManagerImpl.EnableAdditionalVmConfig, 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());
+    }
 }

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