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 27b17d96b8f..5d2bbe858d7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -1161,6 +1161,7 @@ public class ApiConstants { public static final String OVM3_CLUSTER = "ovm3cluster"; 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_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/admin/host/UpdateHostCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java index 9baea58b7b5..86f002b9b7d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java @@ -72,6 +72,14 @@ public class UpdateHostCmd extends BaseCmd { @Parameter(name = ApiConstants.EXTERNAL_DETAILS, type = CommandType.MAP, description = "Details in key/value pairs using format externaldetails[i].keyname=keyvalue. Example: externaldetails[0].endpoint.url=urlvalue", since = "4.21.0") protected Map externalDetails; + @Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS, + type = CommandType.BOOLEAN, + description = "Optional boolean field, which indicates if external details should be cleaned up or not " + + "(If set to true, external details removed for this host, externaldetails field ignored; " + + "if false or not set, no action)", + since = "4.22.0") + protected Boolean cleanupExternalDetails; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -112,6 +120,10 @@ public class UpdateHostCmd extends BaseCmd { return convertExternalDetailsToMap(externalDetails); } + public boolean isCleanupExternalDetails() { + return Boolean.TRUE.equals(cleanupExternalDetails); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java index 7f66ac7058a..9d973dfc524 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java @@ -101,6 +101,14 @@ public class UpdateServiceOfferingCmd extends BaseCmd { since = "4.21.0") private Map externalDetails; + @Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS, + type = CommandType.BOOLEAN, + description = "Optional boolean field, which indicates if external details should be cleaned up or not " + + "(If set to true, external details removed for this offering, externaldetails field ignored; " + + "if false or not set, no action)", + since = "4.22.0") + protected Boolean cleanupExternalDetails; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -205,6 +213,10 @@ public class UpdateServiceOfferingCmd extends BaseCmd { return convertExternalDetailsToMap(externalDetails); } + public boolean isCleanupExternalDetails() { + return Boolean.TRUE.equals(cleanupExternalDetails); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDao.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDao.java index 5d8bd0a0a3a..b72ec5239ff 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDao.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDao.java @@ -33,6 +33,8 @@ public interface HostDetailsDao extends GenericDao { List findByName(String name); + void removeExternalDetails(long hostId); + void replaceExternalDetails(long hostId, Map details); } diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDaoImpl.java index 3eb9faeb1c1..0f8e8623506 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDaoImpl.java @@ -39,6 +39,7 @@ public class HostDetailsDaoImpl extends GenericDaoBase implement protected final SearchBuilder HostSearch; protected final SearchBuilder DetailSearch; protected final SearchBuilder DetailNameSearch; + protected final SearchBuilder ExternalDetailSearch; public HostDetailsDaoImpl() { HostSearch = createSearchBuilder(); @@ -53,6 +54,11 @@ public class HostDetailsDaoImpl extends GenericDaoBase implement DetailNameSearch = createSearchBuilder(); DetailNameSearch.and("name", DetailNameSearch.entity().getName(), SearchCriteria.Op.EQ); DetailNameSearch.done(); + + ExternalDetailSearch = createSearchBuilder(); + ExternalDetailSearch.and("hostId", ExternalDetailSearch.entity().getHostId(), SearchCriteria.Op.EQ); + ExternalDetailSearch.and("name", ExternalDetailSearch.entity().getName(), SearchCriteria.Op.LIKE); + ExternalDetailSearch.done(); } @Override @@ -133,6 +139,17 @@ public class HostDetailsDaoImpl extends GenericDaoBase implement return listBy(sc); } + @Override + public void removeExternalDetails(long hostId) { + TransactionLegacy txn = TransactionLegacy.currentTxn(); + txn.start(); + SearchCriteria sc = ExternalDetailSearch.create(); + sc.setParameters("hostId", hostId); + sc.setParameters("name", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%"); + remove(sc); + txn.commit(); + } + @Override public void replaceExternalDetails(long hostId, Map details) { if (details.isEmpty()) { @@ -149,11 +166,7 @@ public class HostDetailsDaoImpl extends GenericDaoBase implement } detailVOs.add(new DetailVO(hostId, name, value)); } - SearchBuilder sb = createSearchBuilder(); - sb.and("hostId", sb.entity().getHostId(), SearchCriteria.Op.EQ); - sb.and("name", sb.entity().getName(), SearchCriteria.Op.LIKE); - sb.done(); - SearchCriteria sc = sb.create(); + SearchCriteria sc = ExternalDetailSearch.create(); sc.setParameters("hostId", hostId); sc.setParameters("name", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%"); remove(sc); diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java index 713e7550a1e..ded07d2dd32 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java @@ -74,7 +74,7 @@ public class UpdateExtensionCmd extends BaseCmd { @Parameter(name = ApiConstants.CLEAN_UP_DETAILS, type = CommandType.BOOLEAN, description = "Optional boolean field, which indicates if details should be cleaned up or not " + - "(If set to true, details removed for this action, details field ignored; " + + "(If set to true, details removed for this extension, details field ignored; " + "if false or not set, no action)") private Boolean cleanupDetails; diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 6f9db1dcdbd..81970b0f8a7 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -3807,8 +3807,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } protected boolean serviceOfferingExternalDetailsNeedUpdate(final Map offeringDetails, - final Map externalDetails) { - if (MapUtils.isEmpty(externalDetails)) { + final Map externalDetails, final boolean cleanupExternalDetails) { + if (MapUtils.isEmpty(externalDetails) && !cleanupExternalDetails) { return false; } @@ -3816,6 +3816,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati .filter(detail -> detail.getKey().startsWith(VmDetailConstants.EXTERNAL_DETAIL_PREFIX)) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + if (cleanupExternalDetails) { + return !MapUtils.isEmpty(existingExternalDetails); + } + if (MapUtils.isEmpty(existingExternalDetails) || existingExternalDetails.size() != externalDetails.size()) { return true; } @@ -3845,6 +3849,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati ServiceOffering.State state = cmd.getState(); boolean purgeResources = cmd.isPurgeResources(); final Map externalDetails = cmd.getExternalDetails(); + final boolean cleanupExternalDetails = cmd.isCleanupExternalDetails(); if (userId == null) { userId = Long.valueOf(User.UID_SYSTEM); @@ -3938,7 +3943,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati final boolean updateNeeded = name != null || displayText != null || sortKey != null || storageTags != null || hostTags != null || state != null; final boolean serviceOfferingExternalDetailsNeedUpdate = - serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails); + serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, cleanupExternalDetails); final boolean detailsUpdateNeeded = !filteredDomainIds.equals(existingDomainIds) || !filteredZoneIds.equals(existingZoneIds) || purgeResources != existingPurgeResources || serviceOfferingExternalDetailsNeedUpdate; @@ -4013,8 +4018,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati SearchCriteria externalDetailsRemoveSC = sb.create(); externalDetailsRemoveSC.setParameters("detailNameLike", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%"); _serviceOfferingDetailsDao.remove(externalDetailsRemoveSC); - for (Map.Entry entry : externalDetails.entrySet()) { - detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true)); + if (!cleanupExternalDetails) { + for (Map.Entry entry : externalDetails.entrySet()) { + detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true)); + } } } } diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index c4983e871da..7a428fc6545 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -2799,12 +2799,15 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, @Override public Host updateHost(final UpdateHostCmd cmd) throws NoTransitionException { - return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(), - cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false, cmd.getExternalDetails()); + return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(), cmd.getAllocationState(), cmd.getUrl(), + cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false, + cmd.getExternalDetails(), cmd.isCleanupExternalDetails()); } private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String allocationState, - String url, List hostTags, Boolean isTagARule, String annotation, boolean isUpdateFromHostHealthCheck, Map externalDetails) throws NoTransitionException { + String url, List hostTags, Boolean isTagARule, String annotation, + boolean isUpdateFromHostHealthCheck, Map externalDetails, + boolean cleanupExternalDetails) throws NoTransitionException { // Verify that the host exists final HostVO host = _hostDao.findById(hostId); if (host == null) { @@ -2828,8 +2831,12 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, updateHostTags(host, hostId, hostTags, isTagARule); } - if (MapUtils.isNotEmpty(externalDetails)) { - _hostDetailsDao.replaceExternalDetails(hostId, externalDetails); + if (cleanupExternalDetails) { + _hostDetailsDao.removeExternalDetails(hostId); + } else { + if (MapUtils.isNotEmpty(externalDetails)) { + _hostDetailsDao.replaceExternalDetails(hostId, externalDetails); + } } if (url != null) { @@ -2888,7 +2895,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, @Override public Host autoUpdateHostAllocationState(Long hostId, ResourceState.Event resourceEvent) throws NoTransitionException { - return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null); + return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null, false); } @Override diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java index 1ffd7967eec..a62f4d113af 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java @@ -1028,17 +1028,47 @@ public class ConfigurationManagerImplTest { Map offeringDetails = Map.of("key1", "value1"); Map externalDetails = Collections.emptyMap(); - boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails); + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false); Assert.assertFalse(result); } + @Test + public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenExternalDetailsIsEmptyAndCleanupTrue() { + Map offeringDetails = Map.of("key1", "value1"); + Map externalDetails = Collections.emptyMap(); + + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true); + + Assert.assertFalse(result); + } + + @Test + public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingDetailsExistExternalDetailsIsEmptyAndCleanupTrue() { + Map offeringDetails = Map.of("External:key1", "value1"); + Map externalDetails = Collections.emptyMap(); + + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true); + + Assert.assertTrue(result); + } + + @Test + public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsExistValidExternalDetailsAndCleanupTrue() { + Map offeringDetails = Map.of("External:key1", "value1"); + Map externalDetails = Collections.emptyMap(); + + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true); + + Assert.assertTrue(result); + } + @Test public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsIsEmpty() { Map offeringDetails = Map.of("key1", "value1"); Map externalDetails = Map.of("External:key1", "value1"); - boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails); + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false); Assert.assertTrue(result); } @@ -1048,7 +1078,7 @@ public class ConfigurationManagerImplTest { Map offeringDetails = Map.of("External:key1", "value1"); Map externalDetails = Map.of("External:key1", "value1", "External:key2", "value2"); - boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails); + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false); Assert.assertTrue(result); } @@ -1058,7 +1088,7 @@ public class ConfigurationManagerImplTest { Map offeringDetails = Map.of("External:key1", "value1"); Map externalDetails = Map.of("External:key1", "differentValue"); - boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails); + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false); Assert.assertTrue(result); } @@ -1068,7 +1098,7 @@ public class ConfigurationManagerImplTest { Map offeringDetails = Map.of("External:key1", "value1", "External:key2", "value2"); Map externalDetails = Map.of("External:key1", "value1", "External:key2", "value2"); - boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails); + boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false); Assert.assertFalse(result); } diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue index 1f18621728f..8c46ca64225 100644 --- a/ui/src/views/AutogenView.vue +++ b/ui/src/views/AutogenView.vue @@ -1770,9 +1770,22 @@ export default { params[key] = param.opts[input].name } } else if (param.type === 'map' && typeof input === 'object') { - Object.entries(values.externaldetails).forEach(([key, value]) => { - params[param.name + '[0].' + key] = value - }) + const details = values[key] + if (details && Object.keys(details).length > 0) { + Object.entries(details).forEach(([k, v]) => { + params[key + '[0].' + k] = v + }) + } else { + if (['details', 'externaldetails'].includes(key)) { + const updateApiParams = this.$getApiParams(action.api) + const cleanupKey = 'cleanup' + key + if (cleanupKey in updateApiParams) { + params[cleanupKey] = true + break + } + } + params[key] = {} + } } else { params[key] = input } diff --git a/ui/src/views/infra/HostUpdate.vue b/ui/src/views/infra/HostUpdate.vue index fc41082e009..1fee7a6846c 100644 --- a/ui/src/views/infra/HostUpdate.vue +++ b/ui/src/views/infra/HostUpdate.vue @@ -198,10 +198,12 @@ export default { if (values.istagarule !== undefined) { params.istagarule = values.istagarule } - if (values.externaldetails) { + if (values.externaldetails && Object.keys(values.externaldetails).length > 0) { Object.entries(values.externaldetails).forEach(([key, value]) => { params['externaldetails[0].' + key] = value }) + } else { + params.cleanupexternaldetails = true } this.loading = true