nvazquez commented on code in PR #8126:
URL: https://github.com/apache/cloudstack/pull/8126#discussion_r1368041653


##########
engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql:
##########
@@ -285,3 +285,5 @@ GROUP BY
 
 -- Set removed state for all removed accounts
 UPDATE `cloud`.`account` SET state='removed' WHERE `removed` IS NOT NULL;
+
+--

Review Comment:
   Another minor nit :)



##########
api/src/main/java/org/apache/cloudstack/api/ApiConstants.java:
##########
@@ -1054,6 +1054,8 @@ public class ApiConstants {
     public static final String SOURCE_NAT_IP_ID = "sourcenatipaddressid";
     public static final String HAS_RULES = "hasrules";
 
+    public static final String nsxDetail = "forNsx";

Review Comment:
   Minor comment: could this be named `NSX_DETAIL` or `NSX_DETAIL_KEY` instead?



##########
server/src/main/java/com/cloud/network/router/VpcNetworkHelperImpl.java:
##########
@@ -87,7 +87,7 @@ public void reallocateRouterNetworks(final 
RouterDeploymentDefinition vpcRouterD
         final TreeSet<String> publicVlans = new TreeSet<String>();
         if (vpcRouterDeploymentDefinition.isPublicNetwork()) {
             publicVlans.add(vpcRouterDeploymentDefinition.getSourceNatIP()
-                                                         .getVlanTag());
+                                                         .getVlanTag()+"");

Review Comment:
   This can be reverted



##########
ui/src/views/infra/zone/IpAddressRangeForm.vue:
##########
@@ -210,6 +226,7 @@ export default {
     const prefilledIpRangesKey = this.traffic + '-ipranges'
     if (this.prefillContent[prefilledIpRangesKey]) {
       this.ipRanges = this.prefillContent[prefilledIpRangesKey]
+      console.log(this.ipRanges)

Review Comment:
   We can remove this



##########
ui/src/views/infra/zone/ZoneWizardLaunchZone.vue:
##########
@@ -905,6 +905,8 @@ export default {
 
         let stopNow = false
         this.stepData.returnedPublicTraffic = 
this.stepData?.returnedPublicTraffic || []
+        console.log(this.prefillContent['public-ipranges'])
+        console.log('pir')

Review Comment:
   Also these :)



##########
server/src/main/java/com/cloud/network/router/CommandSetupHelper.java:
##########
@@ -840,10 +844,18 @@ public int compare(final PublicIpAddress o1, final 
PublicIpAddress o2) {
 
             for (final PublicIpAddress ipAddr : ipAddrList) {
                 final boolean add = ipAddr.getState() == 
IpAddress.State.Releasing ? false : true;
+                String vlanTag = ipAddr.getVlanTag();
+                String key = null;
+                if (Objects.isNull(vlanTag)) {
+                    key = "nsx-" + ipAddr.getAddress().addr();

Review Comment:
   Why the nsx prefix is needed? Is there any other case in which the vlan tag 
may be empty but is not an NSX VPC?



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