Address review feedback for VPC public network rate

- Remove publicnetworkrate from updateVPCOffering; it is set only at
  create time
- createVPCOffering accepts -1 or 0 (unlimited, stored as -1) or a
  positive value; values below -1 are rejected
- Make vpc_offerings.public_nw_rate a signed int; NULL means the VPC
  uses the zone setting vpc.public.network.throttling.rate
- Omit publicnetworkrate in the VPC offering response when not set
- Default vpc.public.network.throttling.rate to -1 and reject values
  below -1
- Use since 24.0 and consistent API descriptions
- migrateVPC: copy VPC details after the tiers are migrated and skip
  publicnetworkrate, which left a stale rate on the migrated VPC
- UI: allow -1 on the Add VPC offering form, drop the edit field
- Add unit tests

Author: sudo87

api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java

@@ -166,8 +166,8 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd {
     private Boolean conserveMode;
 
     @Parameter(name = ApiConstants.PUBLIC_NETWORK_RATE, type = CommandType.INTEGER,
-            since = "4.24.0",
-            description = "Data transfer rate in megabits per second allowed for a VPC's public gateway (internet-facing network), created with this offering. Default is unlimited")
+            since = "24.0",
+            description = "Data transfer rate in megabits per second allowed for a VPC's public gateway (internet-facing network), created with this offering. Use -1 (or 0) for unlimited. If not specified, VPCs created with this offering use the zone level setting vpc.public.network.throttling.rate")
     private Integer publicNetworkRate;
 
 

api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCOfferingCmd.java

@@ -68,11 +68,6 @@ public class UpdateVPCOfferingCmd extends BaseAsyncCmd implements DomainAndZoneI
     @Parameter(name = ApiConstants.SORT_KEY, type = CommandType.INTEGER, description = "Sort key of the VPC offering, integer")
     private Integer sortKey;
 
-    @Parameter(name = ApiConstants.PUBLIC_NETWORK_RATE, type = CommandType.INTEGER,
-            since = "4.24.0",
-            description = "Data transfer rate in megabits per second allowed for a VPC's public gateway (internet-facing network), created with this offering. Use 0 for unlimited")
-    private Integer publicNetworkRate;
-
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -105,10 +100,6 @@ public Integer getSortKey() {
         return sortKey;
     }
 
-    public Integer getPublicNetworkRate() {
-        return publicNetworkRate;
-    }
-
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////

api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java

@@ -312,7 +312,7 @@ public class NetworkResponse extends BaseResponseWithAssociatedNetwork implement
     private Integer privateMtu;
 
     @SerializedName(ApiConstants.NETWORKRATE)
-    @Param(description = "Network rate (in Mb/s) configured for the Guest interface of this network; -1 if unlimited", since = "4.24.0")
+    @Param(description = "Network rate (in Mb/s) configured for the Guest interface of this network; -1 if unlimited", since = "24.0")
     private Integer networkRate;
 
     @SerializedName(ApiConstants.IP6_DNS1)

api/src/main/java/org/apache/cloudstack/api/response/NicResponse.java

@@ -139,7 +139,7 @@ public class NicResponse extends BaseResponse {
     private Integer mtu;
 
     @SerializedName(ApiConstants.NETWORKRATE)
-    @Param(description = "Network rate (in Mb/s) configured for the NIC; -1 if unlimited", since = "4.24.0")
+    @Param(description = "Network rate (in Mb/s) configured for the NIC; -1 if unlimited", since = "24.0")
     private Integer networkRate;
 
     @SerializedName(ApiConstants.PUBLIC_IP_ID)

api/src/main/java/org/apache/cloudstack/api/response/VpcOfferingResponse.java

@@ -107,7 +107,7 @@ public class VpcOfferingResponse extends BaseResponse {
     private Boolean conserveMode;
 
     @SerializedName(ApiConstants.PUBLIC_NETWORK_RATE)
-    @Param(description = "Data transfer rate in megabits per second allowed for a VPC's public gateway (internet-facing network), created with this offering; -1 if unset (falls back to the zone/global default) or unlimited", since = "4.24.0")
+    @Param(description = "Data transfer rate in megabits per second allowed for a VPC's public gateway (internet-facing network), created with this offering; -1 if unlimited. Not returned if no rate is set on the offering, in which case VPCs created with it use the zone level setting vpc.public.network.throttling.rate", since = "24.0")
     private Integer publicNetworkRate;
 
     public void setId(String id) {

api/src/main/java/org/apache/cloudstack/api/response/VpcResponse.java

@@ -78,7 +78,7 @@ public class VpcResponse extends BaseResponseWithAnnotations implements Controll
     private Boolean vpcOfferingConserveMode;
 
     @SerializedName(ApiConstants.PUBLIC_NETWORK_RATE)
-    @Param(description = "Data transfer rate in megabits per second allowed for this VPC's public gateway (internet-facing network); -1 if unlimited", since = "4.24.0")
+    @Param(description = "Data transfer rate in megabits per second allowed for this VPC's public gateway (internet-facing network); -1 if unlimited", since = "24.0")
     private Integer publicNetworkRate;
 
     @SerializedName(ApiConstants.CREATED)

engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java

@@ -138,8 +138,8 @@ public interface NetworkOrchestrationService {
     ConfigKey<Integer> VmNetworkThrottlingRate = new ConfigKey<Integer>("Network", Integer.class, "vm.network.throttling.rate", "200",
             "Default data transfer rate in megabits per second allowed in User vm's default network.", true, ConfigKey.Scope.Zone);
 
-    ConfigKey<Integer> VpcPublicNetworkThrottlingRate = new ConfigKey<>("Network", Integer.class, "vpc.public.network.throttling.rate", "0",
-            "Default data transfer rate in megabits per second allowed for a VPC's public/internet-facing network. 0 means unlimited.", true, ConfigKey.Scope.Zone);
+    ConfigKey<Integer> VpcPublicNetworkThrottlingRate = new ConfigKey<>("Network", Integer.class, "vpc.public.network.throttling.rate", "-1",
+            "Default data transfer rate in megabits per second allowed for a VPC's public/internet-facing network, used when the VPC offering does not specify a rate. -1 or 0 means unlimited.", true, ConfigKey.Scope.Zone);
 
     List<? extends Network> setupNetwork(Account owner, NetworkOffering offering, DeploymentPlan plan, String name, String displayText, boolean isDefault)
         throws ConcurrentOperationException;

engine/schema/src/main/resources/META-INF/db/schema-42300to2400.sql

@@ -21,4 +21,4 @@
 
 ALTER TABLE `cloud`.`nics` ADD COLUMN `network_rate` int DEFAULT NULL COMMENT 'effective network rate in Mb/s for this NIC, -1 means unlimited';
 
-ALTER TABLE `cloud`.`vpc_offerings` ADD COLUMN `public_nw_rate` int unsigned DEFAULT NULL COMMENT 'public gateway (internet-facing) network rate throttle mbits/s';
+ALTER TABLE `cloud`.`vpc_offerings` ADD COLUMN `public_nw_rate` int DEFAULT NULL COMMENT 'public gateway (internet-facing) network rate throttle mbits/s';

server/src/main/java/com/cloud/api/query/dao/VpcOfferingJoinDaoImpl.java

@@ -79,7 +79,7 @@ public VpcOfferingResponse newVpcOfferingResponse(VpcOffering offering) {
         }
         offeringResponse.setConserveMode(offering.isConserveMode());
         Integer pubNetworkRate = offering.getPublicNetworkRate();
-        offeringResponse.setPublicNetworkRate((pubNetworkRate == null || pubNetworkRate <= 0 ) ? -1 : pubNetworkRate);
+        offeringResponse.setPublicNetworkRate(pubNetworkRate);
         if (offering instanceof VpcOfferingJoinVO) {
             VpcOfferingJoinVO offeringJoinVO = (VpcOfferingJoinVO) offering;
             offeringResponse.setDomainId(offeringJoinVO.getDomainUuid());

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -1529,6 +1529,11 @@ protected String validateValueRange(String name, String value, Class<?> type, Co
                     return String.format("[%s] value should be between 0 and 255. 0 value will disable this feature.", name);
                 }
             }
+            if (NetworkOrchestrationService.VpcPublicNetworkThrottlingRate.key().equalsIgnoreCase(name)) {
+                if (val < -1) {
+                    return String.format("Please enter -1 or 0 (unlimited) or a positive value for the configuration parameter: [%s].", name);
+                }
+            }
             if (UnmanagedVMsManager.ThreadsOnMSToImportVMwareVMFiles.key().equalsIgnoreCase(name) ||
                     UnmanagedVMsManager.ThreadsOnKVMHostToImportVMwareVMFiles.key().equalsIgnoreCase(name)) {
                 if (val < -1 || val > 10) {
@@ -9080,9 +9085,7 @@ public Integer getVpcOfferingNetworkRate(final long vpcOfferingId, final Long da
             networkRate = NetworkOrchestrationService.VpcPublicNetworkThrottlingRate.valueIn(dataCenterId);
         }
 
-        // networkRate is unsigned int in vpc_offerings table, and can't be
-        // set to -1
-        // so 0 means unlimited; we convert it to -1, so we are consistent with
+        // 0 also means unlimited; we convert it to -1, so we are consistent with
         // all our other resources where -1 means unlimited
         if (networkRate == 0) {
             networkRate = -1;