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/main/server/src/main/java/com/cloud/network/NetworkServiceImpl.java#L3824-L3824)
 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?



-- 
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]

Reply via email to