Skip to content

Commit 531dd9d

Browse files
shwstpprdhslove
shwstppr
authored and
dhslove
committed
api,server,ui: allow cleaning up external details for host and serviceoffering (apache#11548)
1 parent 174f577 commit 531dd9d

File tree

11 files changed

+123
-67
lines changed

11 files changed

+123
-67
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
@@ -1166,6 +1166,7 @@ public class ApiConstants {
11661166
public static final String OVM3_CLUSTER = "ovm3cluster";
11671167
public static final String OVM3_VIP = "ovm3vip";
11681168
public static final String CLEAN_UP_DETAILS = "cleanupdetails";
1169+
public static final String CLEAN_UP_EXTERNAL_DETAILS = "cleanupexternaldetails";
11691170
public static final String CLEAN_UP_PARAMETERS = "cleanupparameters";
11701171
public static final String VIRTUAL_SIZE = "virtualsize";
11711172
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
@@ -75,6 +75,14 @@ public class UpdateHostCmd extends BaseCmd {
7575
@Parameter(name = ApiConstants.MIGRATION_IP, type = CommandType.STRING, description = "Add an migration ip to this host", since = "4.20")
7676
private String migrationIp;
7777

78+
@Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS,
79+
type = CommandType.BOOLEAN,
80+
description = "Optional boolean field, which indicates if external details should be cleaned up or not " +
81+
"(If set to true, external details removed for this host, externaldetails field ignored; " +
82+
"if false or not set, no action)",
83+
since = "4.22.0")
84+
protected Boolean cleanupExternalDetails;
85+
7886
/////////////////////////////////////////////////////
7987
/////////////////// Accessors ///////////////////////
8088
/////////////////////////////////////////////////////
@@ -119,6 +127,10 @@ public String getMigrationIp() {
119127
return migrationIp;
120128
}
121129

130+
public boolean isCleanupExternalDetails() {
131+
return Boolean.TRUE.equals(cleanupExternalDetails);
132+
}
133+
122134
/////////////////////////////////////////////////////
123135
/////////////// API Implementation///////////////////
124136
/////////////////////////////////////////////////////

‎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 Boolean.TRUE.equals(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
@@ -3837,15 +3837,19 @@ private void setBytesRate(DiskOffering offering, Long bytesReadRate, Long bytesR
38373837
}
38383838

38393839
protected boolean serviceOfferingExternalDetailsNeedUpdate(final Map<String, String> offeringDetails,
3840-
final Map<String, String> externalDetails) {
3841-
if (MapUtils.isEmpty(externalDetails)) {
3840+
final Map<String, String> externalDetails, final boolean cleanupExternalDetails) {
3841+
if (MapUtils.isEmpty(externalDetails) && !cleanupExternalDetails) {
38423842
return false;
38433843
}
38443844

38453845
Map<String, String> existingExternalDetails = offeringDetails.entrySet().stream()
38463846
.filter(detail -> detail.getKey().startsWith(VmDetailConstants.EXTERNAL_DETAIL_PREFIX))
38473847
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
38483848

3849+
if (cleanupExternalDetails) {
3850+
return !MapUtils.isEmpty(existingExternalDetails);
3851+
}
3852+
38493853
if (MapUtils.isEmpty(existingExternalDetails) || existingExternalDetails.size() != externalDetails.size()) {
38503854
return true;
38513855
}
@@ -3875,6 +3879,7 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd)
38753879
ServiceOffering.State state = cmd.getState();
38763880
boolean purgeResources = cmd.isPurgeResources();
38773881
final Map<String, String> externalDetails = cmd.getExternalDetails();
3882+
final boolean cleanupExternalDetails = cmd.isCleanupExternalDetails();
38783883

38793884
if (userId == null) {
38803885
userId = Long.valueOf(User.UID_SYSTEM);
@@ -3968,7 +3973,7 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd)
39683973

39693974
final boolean updateNeeded = name != null || displayText != null || sortKey != null || storageTags != null || hostTags != null || state != null;
39703975
final boolean serviceOfferingExternalDetailsNeedUpdate =
3971-
serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
3976+
serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, cleanupExternalDetails);
39723977
final boolean detailsUpdateNeeded = !filteredDomainIds.equals(existingDomainIds) ||
39733978
!filteredZoneIds.equals(existingZoneIds) || purgeResources != existingPurgeResources ||
39743979
serviceOfferingExternalDetailsNeedUpdate;
@@ -4043,8 +4048,10 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd)
40434048
SearchCriteria<ServiceOfferingDetailsVO> externalDetailsRemoveSC = sb.create();
40444049
externalDetailsRemoveSC.setParameters("detailNameLike", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
40454050
_serviceOfferingDetailsDao.remove(externalDetailsRemoveSC);
4046-
for (Map.Entry<String, String> entry : externalDetails.entrySet()) {
4047-
detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true));
4051+
if (!cleanupExternalDetails) {
4052+
for (Map.Entry<String, String> entry : externalDetails.entrySet()) {
4053+
detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true));
4054+
}
40484055
}
40494056
}
40504057
}

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

Lines changed: 11 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2819,11 +2819,13 @@ private void updateHostTags(HostVO host, Long hostId, List<String> hostTags, Boo
28192819
@Override
28202820
public Host updateHost(final UpdateHostCmd cmd) throws NoTransitionException {
28212821
return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(),
2822-
cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false, cmd.getExternalDetails(), cmd.getMigrationIp());
2822+
cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false,
2823+
cmd.getExternalDetails(), cmd.isCleanupExternalDetails(), cmd.getMigrationIp());
28232824
}
28242825

28252826
private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String allocationState,
2826-
String url, List<String> hostTags, Boolean isTagARule, String annotation, boolean isUpdateFromHostHealthCheck, String migrationIp) throws NoTransitionException {
2827+
String url, List<String> hostTags, Boolean isTagARule, String annotation, boolean isUpdateFromHostHealthCheck,
2828+
boolean cleanupExternalDetails, String migrationIp) throws NoTransitionException {
28272829
// Verify that the host exists
28282830
final HostVO host = _hostDao.findById(hostId);
28292831
if (host == null) {
@@ -2849,50 +2851,12 @@ private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String
28492851

28502852
updateMigratinIp(host, migrationIp);
28512853

2852-
if (url != null) {
2853-
_storageMgr.updateSecondaryStorage(hostId, url);
2854-
}
2855-
try {
2856-
_storageMgr.enableHost(hostId);
2857-
} catch (StorageUnavailableException | StorageConflictException e) {
2858-
logger.error(String.format("Failed to setup host %s when enabled", host));
2859-
}
2860-
2861-
final HostVO updatedHost = _hostDao.findById(hostId);
2862-
2863-
sendAlertAndAnnotationForAutoEnableDisableKVMHostFeature(host, allocationState,
2864-
isUpdateFromHostHealthCheck, isUpdateHostAllocation, annotation);
2865-
2866-
return updatedHost;
2867-
}
2868-
2869-
private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String allocationState,
2870-
String url, List<String> hostTags, Boolean isTagARule, String annotation, boolean isUpdateFromHostHealthCheck, Map<String, String> externalDetails) throws NoTransitionException {
2871-
// Verify that the host exists
2872-
final HostVO host = _hostDao.findById(hostId);
2873-
if (host == null) {
2874-
throw new InvalidParameterValueException("Host with id " + hostId + " doesn't exist");
2875-
}
2876-
2877-
boolean isUpdateHostAllocation = false;
2878-
if (StringUtils.isNotBlank(allocationState)) {
2879-
isUpdateHostAllocation = updateHostAllocationState(host, allocationState, isUpdateFromHostHealthCheck);
2880-
}
2881-
2882-
if (StringUtils.isNotBlank(name)) {
2883-
updateHostName(host, name);
2884-
}
2885-
2886-
if (guestOSCategoryId != null) {
2887-
updateHostGuestOSCategory(hostId, guestOSCategoryId);
2888-
}
2889-
2890-
if (hostTags != null) {
2891-
updateHostTags(host, hostId, hostTags, isTagARule);
2892-
}
2893-
2894-
if (MapUtils.isNotEmpty(externalDetails)) {
2895-
_hostDetailsDao.replaceExternalDetails(hostId, externalDetails);
2854+
if (cleanupExternalDetails) {
2855+
_hostDetailsDao.removeExternalDetails(hostId);
2856+
} else {
2857+
if (MapUtils.isNotEmpty(externalDetails)) {
2858+
_hostDetailsDao.replaceExternalDetails(hostId, externalDetails);
2859+
}
28962860
}
28972861

28982862
if (url != null) {
@@ -2951,7 +2915,7 @@ private void sendAlertAndAnnotationForAutoEnableDisableKVMHostFeature(HostVO hos
29512915

29522916
@Override
29532917
public Host autoUpdateHostAllocationState(Long hostId, ResourceState.Event resourceEvent) throws NoTransitionException {
2954-
return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null);
2918+
return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null, false);
29552919
}
29562920

29572921
@Override

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

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

1031-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1031+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10321032

10331033
Assert.assertFalse(result);
10341034
}
10351035

1036+
@Test
1037+
public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenExternalDetailsIsEmptyAndCleanupTrue() {
1038+
Map<String, String> offeringDetails = Map.of("key1", "value1");
1039+
Map<String, String> externalDetails = Collections.emptyMap();
1040+
1041+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
1042+
1043+
Assert.assertFalse(result);
1044+
}
1045+
1046+
@Test
1047+
public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingDetailsExistExternalDetailsIsEmptyAndCleanupTrue() {
1048+
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
1049+
Map<String, String> externalDetails = Collections.emptyMap();
1050+
1051+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
1052+
1053+
Assert.assertTrue(result);
1054+
}
1055+
1056+
@Test
1057+
public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsExistValidExternalDetailsAndCleanupTrue() {
1058+
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
1059+
Map<String, String> externalDetails = Collections.emptyMap();
1060+
1061+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
1062+
1063+
Assert.assertTrue(result);
1064+
}
1065+
10361066
@Test
10371067
public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsIsEmpty() {
10381068
Map<String, String> offeringDetails = Map.of("key1", "value1");
10391069
Map<String, String> externalDetails = Map.of("External:key1", "value1");
10401070

1041-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1071+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10421072

10431073
Assert.assertTrue(result);
10441074
}
@@ -1048,7 +1078,7 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenSizesDiffer()
10481078
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
10491079
Map<String, String> externalDetails = Map.of("External:key1", "value1", "External:key2", "value2");
10501080

1051-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1081+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10521082

10531083
Assert.assertTrue(result);
10541084
}
@@ -1058,7 +1088,7 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenValuesDiffer(
10581088
Map<String, String> offeringDetails = Map.of("External:key1", "value1");
10591089
Map<String, String> externalDetails = Map.of("External:key1", "differentValue");
10601090

1061-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1091+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10621092

10631093
Assert.assertTrue(result);
10641094
}
@@ -1068,7 +1098,7 @@ public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenDetailsMatch
10681098
Map<String, String> offeringDetails = Map.of("External:key1", "value1", "External:key2", "value2");
10691099
Map<String, String> externalDetails = Map.of("External:key1", "value1", "External:key2", "value2");
10701100

1071-
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
1101+
boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
10721102

10731103
Assert.assertFalse(result);
10741104
}

‎ui/src/views/AutogenView.vue‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,9 +2085,22 @@ export default {
20852085
params[key] = param.opts[input].name
20862086
}
20872087
} else if (param.type === 'map' && typeof input === 'object') {
2088-
Object.entries(values.externaldetails).forEach(([key, value]) => {
2089-
params[param.name + '[0].' + key] = value
2090-
})
2088+
const details = values[key]
2089+
if (details && Object.keys(details).length > 0) {
2090+
Object.entries(details).forEach(([k, v]) => {
2091+
params[key + '[0].' + k] = v
2092+
})
2093+
} else {
2094+
if (['details', 'externaldetails'].includes(key)) {
2095+
const updateApiParams = this.$getApiParams(action.api)
2096+
const cleanupKey = 'cleanup' + key
2097+
if (cleanupKey in updateApiParams) {
2098+
params[cleanupKey] = true
2099+
break
2100+
}
2101+
}
2102+
params[key] = {}
2103+
}
20912104
} else {
20922105
params[key] = input
20932106
}

0 commit comments

Comments
 (0)