Skip to content

Commit 8ae1856

Browse files
committed
api/server: revert changes on updatePhysicalNetwork
1 parent 02e415d commit 8ae1856

File tree

7 files changed

+6
-109
lines changed

7 files changed

+6
-109
lines changed

api/src/main/java/com/cloud/network/NetworkService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ PhysicalNetwork createPhysicalNetwork(Long zoneId, String vnetRange, String netw
155155

156156
Pair<List<? extends PhysicalNetwork>, Integer> searchPhysicalNetworks(Long id, Long zoneId, String keyword, Long startIndex, Long pageSize, String name);
157157

158-
PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state, Map<String, String> externalDetails);
158+
PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state);
159159

160160
boolean deletePhysicalNetwork(Long id);
161161

api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdatePhysicalNetworkCmd.java

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.apache.cloudstack.api.command.admin.network;
1818

1919
import java.util.List;
20-
import java.util.Map;
2120

2221

2322
import org.apache.cloudstack.api.APICommand;
@@ -54,12 +53,6 @@ public class UpdatePhysicalNetworkCmd extends BaseAsyncCmd {
5453
@Parameter(name = ApiConstants.VLAN, type = CommandType.STRING, description = "The VLAN for the physical Network")
5554
private String vlan;
5655

57-
@Parameter(name = ApiConstants.EXTERNAL_DETAILS,
58-
type = CommandType.MAP,
59-
description = "Details in key/value pairs to be added to the extension-resource mapping. Use the format externaldetails[i].<key>=<value>. Example: externaldetails[0].endpoint.url=https://example.com",
60-
since = "4.23.0")
61-
protected Map externalDetails;
62-
6356
/////////////////////////////////////////////////////
6457
/////////////////// Accessors ///////////////////////
6558
/////////////////////////////////////////////////////
@@ -84,10 +77,6 @@ public String getVlan() {
8477
return vlan;
8578
}
8679

87-
public Map<String, String> getExternalDetails() {
88-
return convertDetailsToMap(externalDetails);
89-
}
90-
9180
/////////////////////////////////////////////////////
9281
/////////////// API Implementation///////////////////
9382
/////////////////////////////////////////////////////
@@ -99,7 +88,7 @@ public long getEntityOwnerId() {
9988

10089
@Override
10190
public void execute() {
102-
PhysicalNetwork result = _networkService.updatePhysicalNetwork(getId(), getNetworkSpeed(), getTags(), getVlan(), getState(), getExternalDetails());
91+
PhysicalNetwork result = _networkService.updatePhysicalNetwork(getId(), getNetworkSpeed(), getTags(), getVlan(), getState());
10392
if (result != null) {
10493
PhysicalNetworkResponse response = _responseGenerator.createPhysicalNetworkResponse(result);
10594
response.setResponseName(getCommandName());

plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/ManagementServerMock.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ private void locatePhysicalNetwork() {
328328
}
329329
}
330330
if (_znet.getState() != PhysicalNetwork.State.Enabled) {
331-
_znet = _networkService.updatePhysicalNetwork(_znet.getId(), null, null, null, PhysicalNetwork.State.Enabled.toString(), null);
331+
_znet = _networkService.updatePhysicalNetwork(_znet.getId(), null, null, null, PhysicalNetwork.State.Enabled.toString());
332332
}
333333

334334
// Ensure that the physical network supports Guest traffic.

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@
6868
import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse;
6969
import org.apache.cloudstack.context.CallContext;
7070
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
71-
import org.apache.cloudstack.extension.ExtensionResourceMap;
7271
import org.apache.cloudstack.framework.extensions.manager.ExtensionsManager;
7372
import org.apache.cloudstack.framework.config.ConfigKey;
7473
import org.apache.cloudstack.framework.config.Configurable;
@@ -4376,7 +4375,7 @@ public Pair<List<? extends PhysicalNetwork>, Integer> searchPhysicalNetworks(Lon
43764375
@Override
43774376
@DB
43784377
@ActionEvent(eventType = EventTypes.EVENT_PHYSICAL_NETWORK_UPDATE, eventDescription = "updating physical network", async = true)
4379-
public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRange, String state, Map<String, String> externalDetails) {
4378+
public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRange, String state) {
43804379

43814380
// verify input parameters
43824381
PhysicalNetworkVO network = _physicalNetworkDao.findById(id);
@@ -4431,28 +4430,6 @@ public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<
44314430
}
44324431
_physicalNetworkDao.update(id, network);
44334432

4434-
// If external details provided, and an extension is registered on this physical network,
4435-
// update the extension_resource_map_details accordingly.
4436-
try {
4437-
if (externalDetails != null && !externalDetails.isEmpty()) {
4438-
Pair<Boolean, ExtensionResourceMap> needDetailsUpdateMapPair =
4439-
extensionsManager.extensionResourceMapDetailsNeedUpdate(id,
4440-
ExtensionResourceMap.ResourceType.PhysicalNetwork, externalDetails);
4441-
if (Boolean.TRUE.equals(needDetailsUpdateMapPair.first())) {
4442-
ExtensionResourceMap extensionResourceMap = needDetailsUpdateMapPair.second();
4443-
if (extensionResourceMap == null) {
4444-
throw new InvalidParameterValueException(
4445-
String.format("Physical network: %s is not registered with any extension, details cannot be updated",
4446-
network.getId()));
4447-
}
4448-
extensionsManager.updateExtensionResourceMapDetails(extensionResourceMap.getId(), externalDetails);
4449-
}
4450-
}
4451-
} catch (Exception e) {
4452-
// Log warning but don't fail the update
4453-
logger.warn("Failed to update external details for physical network {}: {}", id, e.getMessage());
4454-
}
4455-
44564433
return network;
44574434

44584435
}

server/src/test/java/com/cloud/network/NetworkServiceImplTest.java

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@
4747
import org.apache.cloudstack.api.command.user.network.UpdateNetworkCmd;
4848
import org.apache.cloudstack.context.CallContext;
4949
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
50-
import org.apache.cloudstack.extension.ExtensionResourceMap;
5150
import org.apache.cloudstack.framework.config.ConfigKey;
52-
import org.apache.cloudstack.framework.extensions.manager.ExtensionsManager;
5351
import org.apache.cloudstack.network.RoutedIpv4Manager;
5452
import org.junit.After;
5553
import org.junit.Assert;
@@ -240,9 +238,6 @@ public class NetworkServiceImplTest {
240238
@Mock
241239
private NsxProviderDao nsxProviderDao;
242240

243-
@Mock
244-
ExtensionsManager extensionsManager;
245-
246241
private static Date beforeDate;
247242

248243
private static Date afterDate;
@@ -327,7 +322,6 @@ public void setup() throws Exception {
327322
service.networkHelper = networkHelper;
328323
service._ipAddrMgr = ipAddressManagerMock;
329324
service.nsxProviderDao = nsxProviderDao;
330-
ReflectionTestUtils.setField(service, "extensionsManager", extensionsManager);
331325
callContextMocked = Mockito.mockStatic(CallContext.class);
332326
CallContext callContextMock = Mockito.mock(CallContext.class);
333327
callContextMocked.when(CallContext::current).thenReturn(callContextMock);
@@ -1336,67 +1330,4 @@ public void addProjectNetworksConditionToSearch_includesSpecificProjectWhenProje
13361330
Mockito.verify(accountJoin).addAnd("type", SearchCriteria.Op.EQ, Account.Type.PROJECT);
13371331
Mockito.verify(sc).addAnd("id", SearchCriteria.Op.SC, accountJoin);
13381332
}
1339-
1340-
// -----------------------------------------------------------------------
1341-
// Tests for updatePhysicalNetwork extension external details handling
1342-
// -----------------------------------------------------------------------
1343-
1344-
@Test
1345-
public void updatePhysicalNetworkUpdatesExtensionResourceMapDetailsWhenDetailsProvided() {
1346-
Long physNetId = 100L;
1347-
PhysicalNetworkVO physNet = Mockito.mock(PhysicalNetworkVO.class);
1348-
Mockito.when(physicalNetworkDao.findById(physNetId)).thenReturn(physNet);
1349-
Mockito.when(physicalNetworkDao.update(Mockito.anyLong(), any())).thenReturn(true);
1350-
1351-
Map<String, String> externalDetails = Map.of("host", "10.0.0.1", "port", "443");
1352-
1353-
ExtensionResourceMap resourceMap = Mockito.mock(ExtensionResourceMap.class);
1354-
Mockito.when(resourceMap.getId()).thenReturn(50L);
1355-
Pair<Boolean, ExtensionResourceMap> pair =
1356-
new Pair<>(true, resourceMap);
1357-
Mockito.when(extensionsManager.extensionResourceMapDetailsNeedUpdate(physNetId,
1358-
ExtensionResourceMap.ResourceType.PhysicalNetwork, externalDetails))
1359-
.thenReturn(pair);
1360-
1361-
service.updatePhysicalNetwork(physNetId, null, null, null, null, externalDetails);
1362-
1363-
Mockito.verify(extensionsManager).updateExtensionResourceMapDetails(50L, externalDetails);
1364-
}
1365-
1366-
@Test
1367-
public void updatePhysicalNetworkDoesNotUpdateExtensionWhenNoDetailsChange() {
1368-
Long physNetId = 101L;
1369-
PhysicalNetworkVO physNet = Mockito.mock(PhysicalNetworkVO.class);
1370-
Mockito.when(physicalNetworkDao.findById(physNetId)).thenReturn(physNet);
1371-
Mockito.when(physicalNetworkDao.update(Mockito.anyLong(), any())).thenReturn(true);
1372-
1373-
Map<String, String> externalDetails = Map.of("host", "10.0.0.2");
1374-
1375-
Pair<Boolean, ExtensionResourceMap> pair =
1376-
new Pair<>(false, null);
1377-
Mockito.when(extensionsManager.extensionResourceMapDetailsNeedUpdate(physNetId,
1378-
ExtensionResourceMap.ResourceType.PhysicalNetwork, externalDetails))
1379-
.thenReturn(pair);
1380-
1381-
service.updatePhysicalNetwork(physNetId, null, null, null, null, externalDetails);
1382-
1383-
Mockito.verify(extensionsManager, Mockito.never()).updateExtensionResourceMapDetails(Mockito.anyLong(), any());
1384-
}
1385-
1386-
@Test
1387-
public void updatePhysicalNetworkLogsWarningWhenExtensionUpdateFailsButDoesNotThrow() {
1388-
Long physNetId = 102L;
1389-
PhysicalNetworkVO physNet = Mockito.mock(PhysicalNetworkVO.class);
1390-
Mockito.when(physicalNetworkDao.findById(physNetId)).thenReturn(physNet);
1391-
Mockito.when(physicalNetworkDao.update(Mockito.anyLong(), any())).thenReturn(true);
1392-
1393-
Map<String, String> externalDetails = Map.of("host", "10.0.0.3");
1394-
1395-
Mockito.when(extensionsManager.extensionResourceMapDetailsNeedUpdate(physNetId,
1396-
ExtensionResourceMap.ResourceType.PhysicalNetwork, externalDetails))
1397-
.thenThrow(new RuntimeException("Test exception"));
1398-
1399-
// Should not throw
1400-
service.updatePhysicalNetwork(physNetId, null, null, null, null, externalDetails);
1401-
}
14021333
}

server/src/test/java/com/cloud/network/UpdatePhysicalNetworkTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public void updatePhysicalNetworkTest() {
6161
when(_datacenterDao.findById(anyLong())).thenReturn(datacentervo);
6262
when(_physicalNetworkDao.update(anyLong(), any(physicalNetworkVO.getClass()))).thenReturn(true);
6363
when(_datacenterVnetDao.listVnetsByPhysicalNetworkAndDataCenter(anyLong(), anyLong())).thenReturn(existingRange);
64-
networkService.updatePhysicalNetwork(1l, null, null, "524-524,525-530", null, null);
64+
networkService.updatePhysicalNetwork(1l, null, null, "524-524,525-530", null);
6565
txn.close("updatePhysicalNetworkTest");
6666
verify(physicalNetworkVO).setVnet(argumentCaptor.capture());
6767
assertEquals("524-530", argumentCaptor.getValue());

server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ public Pair<List<? extends PhysicalNetwork>, Integer> searchPhysicalNetworks(Lon
345345
* @see com.cloud.network.NetworkService#updatePhysicalNetwork(java.lang.Long, java.lang.String, java.util.List, java.lang.String, java.lang.String)
346346
*/
347347
@Override
348-
public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state, Map<String, String> externalDetails) {
348+
public PhysicalNetwork updatePhysicalNetwork(Long id, String networkSpeed, List<String> tags, String newVnetRangeString, String state) {
349349
// TODO Auto-generated method stub
350350
return null;
351351
}

0 commit comments

Comments
 (0)