madhukar93 commented on code in PR #8151:
URL: https://github.com/apache/cloudstack/pull/8151#discussion_r1373104999
##########
server/src/main/java/com/cloud/network/NetworkServiceImpl.java:
##########
@@ -4997,6 +4997,7 @@ public PhysicalNetworkTrafficType
addTrafficTypeToPhysicalNetwork(Long physicalN
if (TrafficType.Public.equals(trafficType)) {
List<String> isolationMethods = network.getIsolationMethods();
if ((isolationMethods.size() == 1 &&
isolationMethods.get(0).toLowerCase().equals("vxlan"))
+ // TODO: what happens if there are multiple isolation
methods, why are multiple isolation methods allowed per pnet? They seem to be
filterable by provider/preifx (PhysicalNetwork.java). Documentation says only
two are allowed but code and tests contradict this.
Review Comment:
I see
[checks](https://github.com/apache/cloudstack/blob/a06f8a8763dc0d30b14e41d579ec73d40613e1d9/server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3947C13-L3947C22)
to ensure only one isolation method can exist at this time. @rohityadavcloud
@DaanHoogland I would like to confirm if the API field should be plural for
futureproofing, or should we maintain parity with with
[addTrafficType](https://cloudstack.apache.org/api/apidocs-4.17/apis/addTrafficType.html),
and use singular?
There seem to be [legitimate
cases](https://github.com/apache/cloudstack/blob/a06f8a8763dc0d30b14e41d579ec73d40613e1d9/plugins/network-elements/nicira-nvp/src/test/java/com/cloud/network/guru/NiciraNvpGuestNetworkGuruTest.java#L117C1-L117C1)
where we can have multiple values.
--
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]
