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


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java:
##########
@@ -127,6 +139,18 @@ public class CreateNetworkOfferingCmd extends BaseCmd {
             description = "true if network offering is meant to be used for 
VPC, false otherwise.")
     private Boolean forVpc;
 
+    @Parameter(name = ApiConstants.FOR_NSX,
+            type = CommandType.BOOLEAN,
+            description = "true if network offering is meant to be used for 
NSX, false otherwise.",
+            since = "4.20.0")
+    private Boolean forNsx;
+
+    @Parameter(name = ApiConstants.MODE,
+            type = CommandType.STRING,
+            description = "Indicates the mode with which the network will 
operate. Valid option: NAT or Route",

Review Comment:
   Maybe we want to state its only when fornsx=true? We can also do a 
validation of the parameters (should be ignored if fornsx is not set?)



##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -120,21 +145,43 @@ public String getDisplayText() {
     }
 
     public List<String> getSupportedServices() {
+        if (!forNsx && CollectionUtils.isEmpty(supportedServices)) {
+            throw new InvalidParameterValueException("Supported services needs 
to be provided");
+        }
+        if (forNsx) {
+            return List.of(
+                    Dhcp.getName(),
+                    Dns.getName(),
+                    Lb.getName(),
+                    StaticNat.getName(),
+                    SourceNat.getName(),
+                    NetworkACL.getName(),
+                    PortForwarding.getName(),
+                    UserData.getName()
+                    );
+        }
         return supportedServices;
     }
 
+    public Boolean getForNsx() {

Review Comment:
   Same for this



##########
engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql:
##########
@@ -259,6 +262,7 @@ SELECT
     `network_offerings`.`for_vpc` AS `for_vpc`,
     `network_offerings`.`for_tungsten` AS `for_tungsten`,
     `network_offerings`.`for_nsx` AS `for_nsx`,
+    `network_offerings`.`mode` AS `mode`,

Review Comment:
   I think we can call it nsx_mode as other types will have this field null by 
default



##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java:
##########
@@ -241,6 +282,14 @@ public Boolean getForVpc() {
         return forVpc;
     }
 
+    public Boolean getForNsx() {

Review Comment:
   Very minor one, but can we name this `isForNsx` and make it return a boolean 
using the BooleanUtils to make it false when the parameter is null?



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