DaanHoogland commented on code in PR #10081:
URL: https://github.com/apache/cloudstack/pull/10081#discussion_r1879709139


##########
server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java:
##########
@@ -3402,6 +3402,15 @@ public boolean isSrcNatIpRequired(long vpcOfferingId) {
                 && 
vpcOffSvcProvidersMap.get(Service.Gateway).contains(Network.Provider.VPCVirtualRouter));
     }
 
+    @Override
+    public boolean isSrcNatIpRequiredForVpcVr(long vpcOfferingId) {
+        final Map<Network.Service, Set<Network.Provider>> 
vpcOffSvcProvidersMap = getVpcOffSvcProvidersMap(vpcOfferingId);
+        return 
(Objects.nonNull(vpcOffSvcProvidersMap.get(Network.Service.SourceNat))
+                && 
vpcOffSvcProvidersMap.get(Network.Service.SourceNat).contains(Network.Provider.VPCVirtualRouter))
+                || 
(Objects.nonNull(vpcOffSvcProvidersMap.get(Network.Service.Gateway))
+                && 
vpcOffSvcProvidersMap.get(Service.Gateway).contains(Network.Provider.VPCVirtualRouter));
+    }

Review Comment:
   ok, might work. As you create a method `isSrcNatIpRequiredForVpcVr` the 
contents seems appropriate, but its use is more like `needsPublicInterface` 
does it? and in that case we would also want to test if the VR needs to provide 
`StaticNat`. I think a `Gateway` could be on a static ip as well, in theory at 
least.
   
   maybe rename the method?



-- 
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: commits-unsubscr...@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to