Copilot commented on code in PR #13287:
URL: https://github.com/apache/cloudstack/pull/13287#discussion_r3324517936
##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -1897,7 +1896,7 @@ public Network createGuestNetwork(long networkOfferingId,
String name, String di
return _networkMgr.createGuestNetwork(networkOfferingId, name,
displayText,
null, null, null, false, null, owner, null, physicalNetwork,
zoneId,
aclType, null, null, null, null, true, null,
- null, null, null, null, null, null, null, null, vrIfaceMTUs,
null);
+ null, null, null, null, null, null, null, null, null);
Review Comment:
The overload that accepts `vrIfaceMTUs` now drops that argument and always
passes `null`, so callers using this method silently lose their requested
public/private router interface MTUs. Pass `vrIfaceMTUs` in the penultimate
argument position after removing `externalId`.
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java:
##########
@@ -2722,30 +2722,30 @@ public void removeNics(final VirtualMachineProfile vm) {
public Network createPrivateNetwork(final long networkOfferingId, final
String name, final String displayText, final String gateway, final String cidr,
final String vlanId, final boolean bypassVlanOverlapCheck, final Account owner,
final PhysicalNetwork pNtwk, final Long vpcId) throws
ConcurrentOperationException, InsufficientCapacityException,
ResourceAllocationException {
// create network for private gateway
return createGuestNetwork(networkOfferingId, name, displayText,
gateway, cidr, vlanId,
- bypassVlanOverlapCheck, null, owner, null, pNtwk,
pNtwk.getDataCenterId(), ACLType.Account, null,
- vpcId, null, null, true, null, null, null, true, null, null,
- null, null, null, null, null, null);
+ bypassVlanOverlapCheck, null, owner, null, pNtwk,
pNtwk.getDataCenterId(),
+ ACLType.Account, null, vpcId, null, null, true, null, null,
+ null, null, null, null, null, null, null, null);
Review Comment:
This call now resolves to the public `createGuestNetwork` overload instead
of the private overload with `isPrivateNetwork`, so private gateway networks
are created without `isPrivateNetwork=true`. Keep the explicit boolean argument
after the isolated PVLAN arguments when removing `externalId`.
##########
server/src/test/java/com/cloud/network/NetworkServiceImplTest.java:
##########
@@ -822,7 +841,7 @@ public void testCreateVpcTier() throws
InsufficientCapacityException, ResourceAl
Mockito.verify(vpcMgr,
times(1)).createVpcGuestNetwork(networkOfferingId, "Vpc 1 -- testNetwork",
"Test Network", null, null,
null, null, accountMock, null, phyNet, zoneId, null, null,
vpcId, null, accountMock, true,
- null, null, null, null, null, null, null, new Pair<>(0,
privateMtu), null);
+ null, null, null, null, null, null, null, null);
Review Comment:
This verification no longer matches the behavior under test:
`createGuestNetwork` still computes `interfaceMTUs` from `privateMtu` and
passes it to `createVpcGuestNetwork`. Verifying `null` here will fail (and
would stop covering the MTU propagation this test was checking).
##########
server/src/main/java/com/cloud/api/ApiResponseHelper.java:
##########
@@ -2805,7 +2805,6 @@ public NetworkResponse createNetworkResponse(ResponseView
view, Network network)
}
response.setNetworkSpannedZones(networkSpannedZones);
}
Review Comment:
After this setter is removed, `NetworkResponse` still declares and documents
an `externalid` response property, but it can never be populated because
`Network` no longer exposes the value. Please either remove the response
field/API documentation as part of the cleanup or keep response population
until the response contract is intentionally removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
