Skip to content

Commit 3981d42

Browse files
Pearl1594Pearl1594
committed
combine add/clone disk/compute offering forms
1 parent 3aa1040 commit 3981d42

File tree

12 files changed

+1807
-2537
lines changed

12 files changed

+1807
-2537
lines changed

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

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3929,11 +3929,11 @@ private DiskOfferingVO getSourceDiskOffering(ServiceOfferingVO sourceOffering) {
39293929
return sourceDiskOfferingId != null ? _diskOfferingDao.findById(sourceDiskOfferingId) : null;
39303930
}
39313931

3932-
private <T> T getOrDefault(T cmdValue, T defaultValue) {
3932+
public <T> T getOrDefault(T cmdValue, T defaultValue) {
39333933
return cmdValue != null ? cmdValue : defaultValue;
39343934
}
39353935

3936-
private Boolean resolveBooleanParam(Map<String, String> requestParams, String paramKey,
3936+
public Boolean resolveBooleanParam(Map<String, String> requestParams, String paramKey,
39373937
java.util.function.Supplier<Boolean> cmdValueSupplier, Boolean defaultValue) {
39383938
return requestParams != null && requestParams.containsKey(paramKey) ? cmdValueSupplier.get() : defaultValue;
39393939
}
@@ -8287,7 +8287,39 @@ public NetworkOffering cloneNetworkOffering(final CloneNetworkOfferingCmd cmd) {
82878287
logger.info("Cloning network offering {} (id: {}) to new offering with name: {}",
82888288
sourceOffering.getName(), sourceOfferingId, name);
82898289

8290-
// Resolve parameters from source offering and apply add/drop logic
8290+
String detectedProvider = cmd.getProvider();
8291+
8292+
if (detectedProvider == null || detectedProvider.isEmpty()) {
8293+
Map<Network.Service, Set<Network.Provider>> sourceServiceProviderMap =
8294+
_networkModel.getNetworkOfferingServiceProvidersMap(sourceOfferingId);
8295+
8296+
if (sourceServiceProviderMap.containsKey(Network.Service.NetworkACL)) {
8297+
Set<Network.Provider> networkAclProviders = sourceServiceProviderMap.get(Network.Service.NetworkACL);
8298+
if (networkAclProviders != null && !networkAclProviders.isEmpty()) {
8299+
Network.Provider provider = networkAclProviders.iterator().next();
8300+
if (provider == Network.Provider.Nsx) {
8301+
detectedProvider = "NSX";
8302+
} else if (provider == Network.Provider.Netris) {
8303+
detectedProvider = "Netris";
8304+
}
8305+
}
8306+
}
8307+
}
8308+
8309+
// If this is an NSX/Netris offering, prevent network mode changes
8310+
if (detectedProvider != null && (detectedProvider.equals("NSX") || detectedProvider.equals("Netris"))) {
8311+
String cmdNetworkMode = cmd.getNetworkMode();
8312+
if (cmdNetworkMode != null && sourceOffering.getNetworkMode() != null) {
8313+
if (!cmdNetworkMode.equalsIgnoreCase(sourceOffering.getNetworkMode().toString())) {
8314+
throw new InvalidParameterValueException(
8315+
String.format("Cannot change network mode when cloning %s provider network offerings. " +
8316+
"Source offering has network mode '%s', but '%s' was specified. " +
8317+
"The network mode is determined by the provider configuration and cannot be modified.",
8318+
detectedProvider, sourceOffering.getNetworkMode(), cmdNetworkMode));
8319+
}
8320+
}
8321+
}
8322+
82918323
applySourceOfferingValuesToCloneCmd(cmd, sourceOffering);
82928324

82938325
return createNetworkOffering(cmd);
@@ -8443,7 +8475,20 @@ private void applyResolvedValuesToCommand(CloneNetworkOfferingCmd cmd, NetworkOf
84438475
.anyMatch(key -> key.startsWith(ApiConstants.SERVICE_CAPABILITY_LIST));
84448476

84458477
if (!hasCapabilityParams && sourceServiceCapabilityList != null && !sourceServiceCapabilityList.isEmpty()) {
8446-
setField(cmd, "serviceCapabilitiesList", sourceServiceCapabilityList);
8478+
// Filter capabilities to only include those for services in the final service list
8479+
// This ensures that if services are dropped, their capabilities are also removed
8480+
Map<String, Map<String, String>> filteredCapabilities = new HashMap<>();
8481+
for (Map.Entry<String, Map<String, String>> entry : sourceServiceCapabilityList.entrySet()) {
8482+
Map<String, String> capabilityMap = entry.getValue();
8483+
String serviceName = capabilityMap.get("service");
8484+
if (serviceName != null && finalServices.contains(serviceName)) {
8485+
filteredCapabilities.put(entry.getKey(), capabilityMap);
8486+
}
8487+
}
8488+
8489+
if (!filteredCapabilities.isEmpty()) {
8490+
setField(cmd, "serviceCapabilitiesList", filteredCapabilities);
8491+
}
84478492
}
84488493

84498494
applyIfNotProvided(cmd, requestParams, "displayText", ApiConstants.DISPLAY_TEXT, cmd.getDisplayText(), sourceOffering.getDisplayText());

‎server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,10 @@ private void applyResolvedValuesToCommand(CloneVPCOfferingCmd cmd, VpcOfferingVO
10241024

10251025
if ((cmd.getServiceCapabilityList() == null || cmd.getServiceCapabilityList().isEmpty())
10261026
&& sourceServiceCapabilityList != null && !sourceServiceCapabilityList.isEmpty()) {
1027-
ConfigurationManagerImpl.setField(cmd, "serviceCapabilityList", sourceServiceCapabilityList);
1027+
Map<String, String> filteredCapabilities = filterServiceCapabilities(sourceServiceCapabilityList, finalServices);
1028+
if (!filteredCapabilities.isEmpty()) {
1029+
ConfigurationManagerImpl.setField(cmd, "serviceCapabilityList", filteredCapabilities);
1030+
}
10281031
}
10291032

10301033
if (cmd.getDisplayText() == null && sourceOffering.getDisplayText() != null) {
@@ -1095,6 +1098,47 @@ private <T> T getRawFieldValue(Object obj, String fieldName, Class<T> expectedTy
10951098
return null;
10961099
}
10971100

1101+
/**
1102+
* Filters service capabilities to only include those for services present in the final services list.
1103+
* This ensures that when services are dropped during cloning, their associated capabilities are also removed.
1104+
*
1105+
* @param sourceServiceCapabilityList The original capability list from the source VPC offering
1106+
* in format: Map with keys like "0.service", "0.capabilitytype", "0.capabilityvalue"
1107+
* @param finalServices The list of service names that should be retained in the cloned offering
1108+
* @return Filtered map containing only capabilities for services in finalServices
1109+
*/
1110+
private Map<String, String> filterServiceCapabilities(Map<String, String> sourceServiceCapabilityList,
1111+
List<String> finalServices) {
1112+
Map<String, String> filteredCapabilities = new HashMap<>();
1113+
1114+
for (Map.Entry<String, String> entry : sourceServiceCapabilityList.entrySet()) {
1115+
String key = entry.getKey();
1116+
String value = entry.getValue();
1117+
1118+
// Check if this is a service key (e.g., "0.service", "1.service")
1119+
if (key.endsWith(".service")) {
1120+
String serviceName = value;
1121+
if (finalServices.contains(serviceName)) {
1122+
// Include this service and its associated capability entries
1123+
String prefix = key.substring(0, key.lastIndexOf('.'));
1124+
filteredCapabilities.put(key, value);
1125+
1126+
// Also include the capability type and value for this service
1127+
String capabilityTypeKey = prefix + ".capabilitytype";
1128+
String capabilityValueKey = prefix + ".capabilityvalue";
1129+
if (sourceServiceCapabilityList.containsKey(capabilityTypeKey)) {
1130+
filteredCapabilities.put(capabilityTypeKey, sourceServiceCapabilityList.get(capabilityTypeKey));
1131+
}
1132+
if (sourceServiceCapabilityList.containsKey(capabilityValueKey)) {
1133+
filteredCapabilities.put(capabilityValueKey, sourceServiceCapabilityList.get(capabilityValueKey));
1134+
}
1135+
}
1136+
}
1137+
}
1138+
1139+
return filteredCapabilities;
1140+
}
1141+
10981142
private void validateConnectivtyServiceCapabilities(final Set<Provider> providers, final Map serviceCapabilitystList) {
10991143
if (serviceCapabilitystList != null && !serviceCapabilitystList.isEmpty()) {
11001144
final Collection serviceCapabilityCollection = serviceCapabilitystList.values();

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.cloud.domain.dao.DomainDao;
2323
import com.cloud.exception.InvalidParameterValueException;
2424
import com.cloud.network.Network;
25-
import com.cloud.network.NetworkModel;
2625
import com.cloud.network.Networks;
2726
import com.cloud.offering.DiskOffering;
2827
import com.cloud.offering.NetworkOffering;
@@ -680,10 +679,9 @@ public void testCloneNetworkOfferingInheritsAllPropertiesFromSource() {
680679
when(sourceOffering.getAvailability()).thenReturn(NetworkOffering.Availability.Optional);
681680
when(sourceOffering.getState()).thenReturn(NetworkOffering.State.Enabled);
682681
when(sourceOffering.isDefault()).thenReturn(false);
683-
when(sourceOffering.getConserveMode()).thenReturn(true);
682+
when(sourceOffering.isConserveMode()).thenReturn(true);
684683
when(sourceOffering.isEgressDefaultPolicy()).thenReturn(false);
685684
when(sourceOffering.isPersistent()).thenReturn(false);
686-
when(sourceOffering.getInternetProtocol()).thenReturn("ipv4");
687685

688686
CloneNetworkOfferingCmd cmd = mock(CloneNetworkOfferingCmd.class);
689687
when(cmd.getSourceOfferingId()).thenReturn(sourceId);

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

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,9 @@
4040
import com.cloud.network.element.NsxProviderVO;
4141
import com.cloud.offering.DiskOffering;
4242
import com.cloud.offering.NetworkOffering;
43-
import com.cloud.offering.ServiceOffering;
4443
import com.cloud.offerings.NetworkOfferingVO;
4544
import com.cloud.offerings.dao.NetworkOfferingDao;
4645
import com.cloud.service.ServiceOfferingVO;
47-
import com.cloud.service.dao.ServiceOfferingDao;
4846
import com.cloud.storage.DiskOfferingVO;
4947
import com.cloud.storage.Storage;
5048
import com.cloud.storage.StorageManager;
@@ -1181,7 +1179,7 @@ public void testCloneServiceOfferingWithAllParameters() {
11811179
Long sourceOfferingId = 1L;
11821180
ServiceOfferingVO sourceOffering = Mockito.mock(ServiceOfferingVO.class);
11831181
DiskOfferingVO sourceDiskOffering = Mockito.mock(DiskOfferingVO.class);
1184-
1182+
11851183
when(sourceOffering.getId()).thenReturn(sourceOfferingId);
11861184
when(sourceOffering.getDisplayText()).thenReturn("Source Display Text");
11871185
when(sourceOffering.getCpu()).thenReturn(2);
@@ -1213,7 +1211,7 @@ public void testCloneServiceOfferingWithAllParameters() {
12131211
@Test
12141212
public void testCloneServiceOfferingValidatesSourceOfferingExists() {
12151213
Long sourceOfferingId = 999L;
1216-
1214+
12171215
try (MockedStatic<CallContext> callContextMock = Mockito.mockStatic(CallContext.class)) {
12181216
CallContext callContext = Mockito.mock(CallContext.class);
12191217
callContextMock.when(CallContext::current).thenReturn(callContext);
@@ -1227,7 +1225,7 @@ public void testCloneServiceOfferingValidatesSourceOfferingExists() {
12271225
public void testCloneDiskOfferingWithAllParameters() {
12281226
Long sourceOfferingId = 1L;
12291227
DiskOfferingVO sourceOffering = Mockito.mock(DiskOfferingVO.class);
1230-
1228+
12311229
when(sourceOffering.getId()).thenReturn(sourceOfferingId);
12321230
when(sourceOffering.getDisplayText()).thenReturn("Source Disk Display Text");
12331231
when(sourceOffering.getProvisioningType()).thenReturn(Storage.ProvisioningType.THIN);
@@ -1252,7 +1250,7 @@ public void testCloneDiskOfferingWithAllParameters() {
12521250
@Test
12531251
public void testCloneDiskOfferingValidatesSourceOfferingExists() {
12541252
Long sourceOfferingId = 999L;
1255-
1253+
12561254
try (MockedStatic<CallContext> callContextMock = Mockito.mockStatic(CallContext.class)) {
12571255
CallContext callContext = Mockito.mock(CallContext.class);
12581256
callContextMock.when(CallContext::current).thenReturn(callContext);
@@ -1262,18 +1260,11 @@ public void testCloneDiskOfferingValidatesSourceOfferingExists() {
12621260
}
12631261
}
12641262

1265-
@Test
1266-
public void testCloneNetworkOfferingValidatesSourceOfferingExists() {
1267-
Long sourceOfferingId = 999L;
1268-
1269-
Assert.assertNotNull(sourceOfferingId);
1270-
}
1271-
12721263
@Test
12731264
public void testCloneNetworkOfferingRequiresName() {
12741265
Long sourceOfferingId = 1L;
12751266
NetworkOfferingVO sourceOffering = Mockito.mock(NetworkOfferingVO.class);
1276-
1267+
12771268
when(sourceOffering.getId()).thenReturn(sourceOfferingId);
12781269
when(sourceOffering.getName()).thenReturn("Source Network Offering");
12791270

@@ -1284,42 +1275,42 @@ public void testCloneNetworkOfferingRequiresName() {
12841275
public void testGetOrDefaultReturnsCommandValueWhenNotNull() {
12851276
String cmdValue = "command-value";
12861277
String defaultValue = "default-value";
1287-
1278+
12881279
String result = configurationManagerImplSpy.getOrDefault(cmdValue, defaultValue);
1289-
1280+
12901281
Assert.assertEquals(cmdValue, result);
12911282
}
12921283

12931284
@Test
12941285
public void testGetOrDefaultReturnsDefaultWhenCommandValueIsNull() {
12951286
String cmdValue = null;
12961287
String defaultValue = "default-value";
1297-
1288+
12981289
String result = configurationManagerImplSpy.getOrDefault(cmdValue, defaultValue);
1299-
1290+
13001291
Assert.assertEquals(defaultValue, result);
13011292
}
13021293

13031294
@Test
13041295
public void testResolveBooleanParamUsesCommandValueWhenInRequestParams() {
13051296
Map<String, String> requestParams = new HashMap<>();
13061297
requestParams.put("offerha", "true");
1307-
1298+
13081299
Boolean result = configurationManagerImplSpy.resolveBooleanParam(
13091300
requestParams, "offerha", () -> true, false
13101301
);
1311-
1302+
13121303
Assert.assertTrue(result);
13131304
}
13141305

13151306
@Test
13161307
public void testResolveBooleanParamUsesDefaultWhenNotInRequestParams() {
13171308
Map<String, String> requestParams = new HashMap<>();
1318-
1309+
13191310
Boolean result = configurationManagerImplSpy.resolveBooleanParam(
13201311
requestParams, "offerha", () -> true, false
13211312
);
1322-
1313+
13231314
Assert.assertFalse(result);
13241315
}
13251316

@@ -1328,7 +1319,7 @@ public void testResolveBooleanParamUsesDefaultWhenRequestParamsIsNull() {
13281319
Boolean result = configurationManagerImplSpy.resolveBooleanParam(
13291320
null, "offerha", () -> true, false
13301321
);
1331-
1322+
13321323
Assert.assertFalse(result);
13331324
}
13341325
}

0 commit comments

Comments
 (0)