Skip to content

Commit 32ef8bd

Browse files
shwstpprshwstppr
committed
api,server,ui: allow cleaning up external details for host and service
offering Adds parameter - 'cleanupexternaldetails' for updateHost and updateServiceOffering API to allow cleaning up external details. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent 3c4f458 commit 32ef8bd

11 files changed

Lines changed: 124 additions & 26 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,7 @@ public class ApiConstants {
11541154
public static final String OVM3_CLUSTER = "ovm3cluster";
11551155
public static final String OVM3_VIP = "ovm3vip";
11561156
public static final String CLEAN_UP_DETAILS = "cleanupdetails";
1157+
public static final String CLEAN_UP_EXTERNAL_DETAILS = "cleanupexternaldetails";
11571158
public static final String CLEAN_UP_PARAMETERS = "cleanupparameters";
11581159
public static final String VIRTUAL_SIZE = "virtualsize";
11591160
public static final String NETSCALER_CONTROLCENTER_ID = "netscalercontrolcenterid";

‎api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ public class UpdateHostCmd extends BaseCmd {
7272
@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")
7373
protected Map externalDetails;
7474

75+
@Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS,
76+
type = CommandType.BOOLEAN,
77+
description = "Optional boolean field, which indicates if external details should be cleaned up or not " +
78+
"(If set to true, external details removed for this host, externaldetails field ignored; " +
79+
"if false or not set, no action)",
80+
since = "4.22.0")
81+
protected Boolean cleanupExternalDetails;
82+
7583
/////////////////////////////////////////////////////
7684
/////////////////// Accessors ///////////////////////
7785
/////////////////////////////////////////////////////
@@ -112,6 +120,10 @@ public Map<String, String> getExternalDetails() {
112120
return convertExternalDetailsToMap(externalDetails);
113121
}
114122

123+
public Boolean isCleanupExternalDetails() {
124+
return cleanupExternalDetails;
125+
}
126+
115127
/////////////////////////////////////////////////////
116128
/////////////// API Implementation///////////////////
117129
/////////////////////////////////////////////////////

‎api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ public class UpdateServiceOfferingCmd extends BaseCmd {
101101
since = "4.21.0")
102102
private Map externalDetails;
103103

104+
@Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS,
105+
type = CommandType.BOOLEAN,
106+
description = "Optional boolean field, which indicates if external details should be cleaned up or not " +
107+
"(If set to true, external details removed for this offering, externaldetails field ignored; " +
108+
"if false or not set, no action)",
109+
since = "4.22.0")
110+
protected Boolean cleanupExternalDetails;
111+
104112
/////////////////////////////////////////////////////
105113
/////////////////// Accessors ///////////////////////
106114
/////////////////////////////////////////////////////
@@ -205,6 +213,10 @@ public Map<String, String> getExternalDetails() {
205213
return convertExternalDetailsToMap(externalDetails);
206214
}
207215

216+
public Boolean isCleanupExternalDetails() {
217+
return cleanupExternalDetails;
218+
}
219+
208220
/////////////////////////////////////////////////////
209221
/////////////// API Implementation///////////////////
210222
/////////////////////////////////////////////////////

‎engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDao.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ public interface HostDetailsDao extends GenericDao<DetailVO, Long> {
3333

3434
List<DetailVO> findByName(String name);
3535

36+
void removeExternalDetails(long hostId);
37+
3638
void replaceExternalDetails(long hostId, Map<String, String> details);
3739

3840
}

‎engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDaoImpl.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public class HostDetailsDaoImpl extends GenericDaoBase<DetailVO, Long> implement
3939
protected final SearchBuilder<DetailVO> HostSearch;
4040
protected final SearchBuilder<DetailVO> DetailSearch;
4141
protected final SearchBuilder<DetailVO> DetailNameSearch;
42+
protected final SearchBuilder<DetailVO> ExternalDetailSearch;
4243

4344
public HostDetailsDaoImpl() {
4445
HostSearch = createSearchBuilder();
@@ -53,6 +54,11 @@ public HostDetailsDaoImpl() {
5354
DetailNameSearch = createSearchBuilder();
5455
DetailNameSearch.and("name", DetailNameSearch.entity().getName(), SearchCriteria.Op.EQ);
5556
DetailNameSearch.done();
57+
58+
ExternalDetailSearch = createSearchBuilder();
59+
ExternalDetailSearch.and("hostId", ExternalDetailSearch.entity().getHostId(), SearchCriteria.Op.EQ);
60+
ExternalDetailSearch.and("name", ExternalDetailSearch.entity().getName(), SearchCriteria.Op.LIKE);
61+
ExternalDetailSearch.done();
5662
}
5763

5864
@Override
@@ -133,6 +139,17 @@ public List<DetailVO> findByName(String name) {
133139
return listBy(sc);
134140
}
135141

142+
@Override
143+
public void removeExternalDetails(long hostId) {
144+
TransactionLegacy txn = TransactionLegacy.currentTxn();
145+
txn.start();
146+
SearchCriteria<DetailVO> sc = ExternalDetailSearch.create();
147+
sc.setParameters("hostId", hostId);
148+
sc.setParameters("name", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
149+
remove(sc);
150+
txn.commit();
151+
}
152+
136153
@Override
137154
public void replaceExternalDetails(long hostId, Map<String, String> details) {
138155
if (details.isEmpty()) {
@@ -149,11 +166,7 @@ public void replaceExternalDetails(long hostId, Map<String, String> details) {
149166
}
150167
detailVOs.add(new DetailVO(hostId, name, value));
151168
}
152-
SearchBuilder<DetailVO> sb = createSearchBuilder();
153-
sb.and("hostId", sb.entity().getHostId(), SearchCriteria.Op.EQ);
154-
sb.and("name", sb.entity().getName(), SearchCriteria.Op.LIKE);
155-
sb.done();
156-
SearchCriteria<DetailVO> sc = sb.create();
169+
SearchCriteria<DetailVO> sc = ExternalDetailSearch.create();
157170
sc.setParameters("hostId", hostId);
158171
sc.setParameters("name", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
159172
remove(sc);

‎framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public class UpdateExtensionCmd extends BaseCmd {
7474
@Parameter(name = ApiConstants.CLEAN_UP_DETAILS,
7575
type = CommandType.BOOLEAN,
7676
description = "Optional boolean field, which indicates if details should be cleaned up or not " +
77-
"(If set to true, details removed for this action, details field ignored; " +
77+
"(If set to true, details removed for this extension, details field ignored; " +
7878
"if false or not set, no action)")
7979
private Boolean cleanupDetails;
8080

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3798,15 +3798,19 @@ private void setBytesRate(DiskOffering offering, Long bytesReadRate, Long bytesR
37983798
}
37993799

38003800
protected boolean serviceOfferingExternalDetailsNeedUpdate(final Map<String, String> offeringDetails,
3801-
final Map<String, String> externalDetails) {
3802-
if (MapUtils.isEmpty(externalDetails)) {
3801+
final Map<String, String> externalDetails, final boolean cleanupExternalDetails) {
3802+
if (MapUtils.isEmpty(externalDetails) && !cleanupExternalDetails) {
38033803
return false;
38043804
}
38053805

38063806
Map<String, String> existingExternalDetails = offeringDetails.entrySet().stream()
38073807
.filter(detail -> detail.getKey().startsWith(VmDetailConstants.EXTERNAL_DETAIL_PREFIX))
38083808
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
38093809

3810+
if (cleanupExternalDetails) {
3811+
return !MapUtils.isEmpty(existingExternalDetails);
3812+
}
3813+
38103814
if (MapUtils.isEmpty(existingExternalDetails) || existingExternalDetails.size() != externalDetails.size()) {
38113815
return true;
38123816
}
@@ -3836,6 +3840,7 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd)
38363840
ServiceOffering.State state = cmd.getState();
38373841
boolean purgeResources = cmd.isPurgeResources();
38383842
final Map<String, String> externalDetails = cmd.getExternalDetails();
3843+
final boolean cleanupExternalDetails = cmd.isCleanupExternalDetails();
38393844

38403845
if (userId == null) {
38413846
userId = Long.valueOf(User.UID_SYSTEM);
@@ -3929,7 +3934,7 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd)
39293934

39303935
final boolean updateNeeded = name != null || displayText != null || sortKey != null || storageTags != null || hostTags != null || state != null;
39313936
final boolean serviceOfferingExternalDetailsNeedUpdate =
3932-
serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
3937+
serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, cleanupExternalDetails);
39333938
final boolean detailsUpdateNeeded = !filteredDomainIds.equals(existingDomainIds) ||
39343939
!filteredZoneIds.equals(existingZoneIds) || purgeResources != existingPurgeResources ||
39353940
serviceOfferingExternalDetailsNeedUpdate;
@@ -4004,8 +4009,10 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd)
40044009
SearchCriteria<ServiceOfferingDetailsVO> externalDetailsRemoveSC = sb.create();
40054010
externalDetailsRemoveSC.setParameters("detailNameLike", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
40064011
_serviceOfferingDetailsDao.remove(externalDetailsRemoveSC);
4007-
for (Map.Entry<String, String> entry : externalDetails.entrySet()) {
4008-
detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true));
4012+
if (!cleanupExternalDetails) {
4013+
for (Map.Entry<String, String> entry : externalDetails.entrySet()) {
4014+
detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true));
4015+
}
40094016
}
40104017
}
40114018
}

‎server/src/main/java/com/cloud/resource/ResourceManagerImpl.java‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2793,12 +2793,15 @@ private void updateHostTags(HostVO host, Long hostId, List<String> hostTags, Boo
27932793

27942794
@Override
27952795
public Host updateHost(final UpdateHostCmd cmd) throws NoTransitionException {
2796-
return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(),
2797-
cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false, cmd.getExternalDetails());
2796+
return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(), cmd.getAllocationState(), cmd.getUrl(),
2797+
cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false, cmd.getExternalDetails(),
2798+
cmd.isCleanupExternalDetails());
27982799
}
27992800

28002801
private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String allocationState,
2801-
String url, List<String> hostTags, Boolean isTagARule, String annotation, boolean isUpdateFromHostHealthCheck, Map<String, String> externalDetails) throws NoTransitionException {
2802+
String url, List<String> hostTags, Boolean isTagARule, String annotation,
2803+
boolean isUpdateFromHostHealthCheck, Map<String, String> externalDetails,
2804+
boolean cleanupExternalDetails) throws NoTransitionException {
28022805
// Verify that the host exists
28032806
final HostVO host = _hostDao.findById(hostId);
28042807
if (host == null) {
@@ -2822,8 +2825,12 @@ private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String
28222825
updateHostTags(host, hostId, hostTags, isTagARule);
28232826
}
28242827

2825-
if (MapUtils.isNotEmpty(externalDetails)) {
2826-
_hostDetailsDao.replaceExternalDetails(hostId, externalDetails);
2828+
if (cleanupExternalDetails) {
2829+
_hostDetailsDao.removeExternalDetails(hostId);
2830+
} else {
2831+
if (MapUtils.isNotEmpty(externalDetails)) {
2832+
_hostDetailsDao.replaceExternalDetails(hostId, externalDetails);
2833+
}
28272834
}
28282835

28292836
if (url != null) {
@@ -2882,7 +2889,7 @@ private void sendAlertAndAnnotationForAutoEnableDisableKVMHostFeature(HostVO hos
28822889

28832890
@Override
28842891
public Host autoUpdateHostAllocationState(Long hostId, ResourceState.Event resourceEvent) throws NoTransitionException {
2885-
return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null);
2892+
return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null, false);
28862893
}
28872894

28882895
@Override

‎server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,17 +1052,47 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenExternalDeta
10521052
Map<String, String> offeringDetails = Map.of("key1", "value1");
10531053
Map<String, String> externalDetails = Collections.emptyMap();
10541054

1055-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1055+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10561056

10571057
Assert.assertFalse(result);
10581058
}
10591059

1060+
@Test
1061+
public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenExternalDetailsIsEmptyAndCleanupTrue() {
1062+
Map<String, String> offeringDetails = Map.of("key1", "value1");
1063+
Map<String, String> externalDetails = Collections.emptyMap();
1064+
1065+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
1066+
1067+
Assert.assertFalse(result);
1068+
}
1069+
1070+
@Test
1071+
public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingDetailsExistExternalDetailsIsEmptyAndCleanupTrue() {
1072+
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
1073+
Map<String, String> externalDetails = Collections.emptyMap();
1074+
1075+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
1076+
1077+
Assert.assertTrue(result);
1078+
}
1079+
1080+
@Test
1081+
public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsExistValidExternalDetailsAndCleanupTrue() {
1082+
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
1083+
Map<String, String> externalDetails = Collections.emptyMap();
1084+
1085+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
1086+
1087+
Assert.assertTrue(result);
1088+
}
1089+
10601090
@Test
10611091
public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsIsEmpty() {
10621092
Map<String, String> offeringDetails = Map.of("key1", "value1");
10631093
Map<String, String> externalDetails = Map.of("External:key1", "value1");
10641094

1065-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1095+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10661096

10671097
Assert.assertTrue(result);
10681098
}
@@ -1072,7 +1102,7 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenSizesDiffer()
10721102
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
10731103
Map<String, String> externalDetails = Map.of("External:key1", "value1", "External:key2", "value2");
10741104

1075-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1105+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10761106

10771107
Assert.assertTrue(result);
10781108
}
@@ -1082,7 +1112,7 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenValuesDiffer(
10821112
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
10831113
Map<String, String> externalDetails = Map.of("External:key1", "differentValue");
10841114

1085-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1115+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10861116

10871117
Assert.assertTrue(result);
10881118
}
@@ -1092,7 +1122,7 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenDetailsMatch
10921122
Map<String, String> offeringDetails = Map.of("External:key1", "value1", "External:key2", "value2");
10931123
Map<String, String> externalDetails = Map.of("External:key1", "value1", "External:key2", "value2");
10941124

1095-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1125+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10961126

10971127
Assert.assertFalse(result);
10981128
}

‎ui/src/views/AutogenView.vue‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,9 +1772,21 @@ export default {
17721772
params[key] = param.opts[input].name
17731773
}
17741774
} else if (param.type === 'map' && typeof input === 'object') {
1775-
Object.entries(values.externaldetails).forEach(([key, value]) => {
1776-
params[param.name + '[0].' + key] = value
1777-
})
1775+
const details = values[key]
1776+
if (details && Object.keys(details).length > 0) {
1777+
Object.entries(details).forEach(([k, v]) => {
1778+
params[key + '[0].' + k] = v
1779+
})
1780+
} else {
1781+
if (['details', 'externaldetails'].includes(key)) {
1782+
const updateApiParams = this.$getApiParams(action.api)
1783+
if (updateApiParams.hasOwnProperty('cleanup' + key)) {
1784+
params['cleanup' + key] = true
1785+
break
1786+
}
1787+
}
1788+
params[key] = {}
1789+
}
17781790
} else {
17791791
params[key] = input
17801792
}

0 commit comments

Comments
 (0)