feat: Adding support for defining max destination weights in Istio VS

Signed-off-by: Rohit Agrawal <[email protected]>

docs/getting-started/istio/index.md

@@ -33,6 +33,8 @@ spec:
           virtualService:
             # Reference to a VirtualService which the controller updates with canary weights
             name: rollouts-demo-vsvc
+            # Optional if there are only two destinations in your routes or if you want to split 100% traffic between stable and canary services. If specified, this will be used as an upper bound for traffic between canary + stable services.
+            maxWeight: 80
             # Optional if there is a single HTTP route in the VirtualService, otherwise required
             routes:
             - http-primary
@@ -71,11 +73,19 @@ spec:
   - name: http-primary  # Should match spec.strategy.canary.trafficRouting.istio.virtualService.routes
     route:
     - destination:
+<<<<<<< Updated upstream
         host: rollouts-demo-stable  # Should match spec.strategy.canary.stableService
       weight: 100
+=======
+        host: rollouts-demo-stable  # Should match rollout.spec.strategy.canary.stableService
+      weight: 80
+>>>>>>> Stashed changes
     - destination:
         host: rollouts-demo-canary  # Should match spec.strategy.canary.canaryService
       weight: 0
+    - destination:
+        host: rollouts-demo-legacy
+      weight: 20
   tls:
   - match:
     - port: 3000  # Should match the port number of the route defined in spec.strategy.canary.trafficRouting.istio.virtualService.tlsRoutes
@@ -84,11 +94,19 @@ spec:
       - localhost
     route:
     - destination:
+<<<<<<< Updated upstream
         host: rollouts-demo-stable  # Should match spec.strategy.canary.stableService
       weight: 100
+=======
+        host: rollouts-demo-stable  # Should match rollout.spec.strategy.canary.stableService
+      weight: 80
+>>>>>>> Stashed changes
     - destination:
         host: rollouts-demo-canary  # Should match spec.strategy.canary.canaryService
       weight: 0
+    - destination:
+        host: rollouts-demo-legacy
+      weight: 20
 ```
 
 Run the following commands to deploy:

manifests/crds/rollout-crd.yaml

@@ -530,6 +530,9 @@ spec:
                                 - name
                                 - stableSubsetName
                                 type: object
+                              maxWeight:
+                                format: int64
+                                type: integer
                               virtualService:
                                 properties:
                                   name:

manifests/install.yaml

@@ -10228,6 +10228,9 @@ spec:
                                 - name
                                 - stableSubsetName
                                 type: object
+                              maxWeight:
+                                format: int64
+                                type: integer
                               virtualService:
                                 properties:
                                   name:

manifests/namespace-install.yaml

@@ -10228,6 +10228,9 @@ spec:
                                 - name
                                 - stableSubsetName
                                 type: object
+                              maxWeight:
+                                format: int64
+                                type: integer
                               virtualService:
                                 properties:
                                   name:

pkg/apiclient/rollout/rollout.swagger.json

@@ -833,6 +833,11 @@
         "destinationRule": {
           "$ref": "#/definitions/github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.IstioDestinationRule",
           "title": "DestinationRule references an Istio DestinationRule to modify to shape traffic"
+        },
+        "maxWeight": {
+          "type": "string",
+          "format": "int64",
+          "description": "Max weight that will be split between canary and stable services. If unset, it defaults to 100."
         }
       },
       "title": "IstioTrafficRouting configuration for Istio service mesh to enable fine grain configuration"

pkg/apis/rollouts/v1alpha1/types.go

@@ -368,6 +368,8 @@ type IstioTrafficRouting struct {
 	VirtualService IstioVirtualService `json:"virtualService" protobuf:"bytes,1,opt,name=virtualService"`
 	// DestinationRule references an Istio DestinationRule to modify to shape traffic
 	DestinationRule *IstioDestinationRule `json:"destinationRule,omitempty" protobuf:"bytes,2,opt,name=destinationRule"`
+	// Max weight that will be split between canary and stable services. If unset, it defaults to 100.
+	MaxWeight int64 `json:"maxWeight,omitempty" protobuf:"bytes,3,opt,name=maxWeight"`
 }
 
 // IstioVirtualService holds information on the virtual service the rollout needs to modify

rollout/trafficrouting/istio/istio.go

@@ -111,6 +111,12 @@ func (r *Reconciler) generateVirtualServicePatches(httpRoutes []VirtualServiceHT
 		stableSubset = r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.StableSubsetName
 	}
 
+	maxWeight := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.MaxWeight
+	// Ignore any invalid values for maxWeight by setting it to the default (100)
+	if maxWeight <= 0 || maxWeight >= 100 {
+		maxWeight = 100
+	}
+
 	// err can be ignored because we already called ValidateHTTPRoutes earlier
 	httpRouteIndexesToPatch, _ := getHttpRouteIndexesToPatch(r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes)
 	tlsRouteIndexesToPatch, _ := getTlsRouteIndexesToPatch(r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.TLSRoutes, tlsRoutes)
@@ -127,19 +133,19 @@ func (r *Reconciler) generateVirtualServicePatches(httpRoutes []VirtualServiceHT
 		if len(httpRoutes) <= routeIdx {
 			break
 		}
-		patches = processRoutes(Http, routeIdx, httpRoutes[routeIdx].Route, desiredWeight, svcSubsets, patches)
+		patches = processRoutes(Http, routeIdx, httpRoutes[routeIdx].Route, desiredWeight, maxWeight, svcSubsets, patches)
 	}
 	// Process TLS Routes
 	for _, routeIdx := range tlsRouteIndexesToPatch {
 		if len(tlsRoutes) <= routeIdx {
 			break
 		}
-		patches = processRoutes(Tls, routeIdx, tlsRoutes[routeIdx].Route, desiredWeight, svcSubsets, patches)
+		patches = processRoutes(Tls, routeIdx, tlsRoutes[routeIdx].Route, desiredWeight, maxWeight, svcSubsets, patches)
 	}
 	return patches
 }
 
-func processRoutes(routeType string, routeIdx int, destinations []VirtualServiceRouteDestination, desiredWeight int64, svcSubsets svcSubsets, patches virtualServicePatches) virtualServicePatches {
+func processRoutes(routeType string, routeIdx int, destinations []VirtualServiceRouteDestination, desiredWeight, maxWeight int64, svcSubsets svcSubsets, patches virtualServicePatches) virtualServicePatches {
 	for idx, destination := range destinations {
 		host := getHost(destination)
 		subset := destination.Destination.Subset
@@ -148,7 +154,7 @@ func processRoutes(routeType string, routeIdx int, destinations []VirtualService
 			patches = appendPatch(routeIdx, routeType, weight, desiredWeight, idx, patches)
 		}
 		if (host != "" && host == svcSubsets.stableSvc) || (subset != "" && subset == svcSubsets.stableSubset) {
-			patches = appendPatch(routeIdx, routeType, weight, 100-desiredWeight, idx, patches)
+			patches = appendPatch(routeIdx, routeType, weight, maxWeight-desiredWeight, idx, patches)
 		}
 	}
 	return patches
@@ -695,9 +701,6 @@ func ValidateTlsRoutes(r *v1alpha1.Rollout, tlsRoutes []VirtualServiceTLSRoute)
 // validateVirtualServiceRouteDestinations ensures there are two destinations within a route and
 // verifies that there is both a canary and a stable host or subset specified
 func validateVirtualServiceRouteDestinations(hr []VirtualServiceRouteDestination, stableSvc, canarySvc string, dRule *v1alpha1.IstioDestinationRule) error {
-	if len(hr) != 2 {
-		return fmt.Errorf("Route does not have exactly two route destinations.")
-	}
 	hasStableSvc := false
 	hasCanarySvc := false
 	hasStableSubset := false
@@ -727,17 +730,17 @@ func validateVirtualServiceRouteDestinations(hr []VirtualServiceRouteDestination
 func validateDestinationRule(dRule *v1alpha1.IstioDestinationRule, hasCanarySubset, hasStableSubset, hasCanarySvc, hasStableSvc bool, canarySvc, stableSvc string) error {
 	if dRule != nil {
 		if !hasCanarySubset {
-			return fmt.Errorf("Canary DestinationRule subset '%s' not found in route", dRule.CanarySubsetName)
+			return fmt.Errorf("Canary DestinationRule subset '%s' not found in the route.", dRule.CanarySubsetName)
 		}
 		if !hasStableSubset {
-			return fmt.Errorf("Stable DestinationRule subset '%s' not found in route", dRule.StableSubsetName)
+			return fmt.Errorf("Stable DestinationRule subset '%s' not found in the route.", dRule.StableSubsetName)
 		}
 	} else {
 		if !hasCanarySvc {
-			return fmt.Errorf("Canary Service '%s' not found in route", canarySvc)
+			return fmt.Errorf("Canary Service '%s' not found in the route.", canarySvc)
 		}
 		if !hasStableSvc {
-			return fmt.Errorf("Stable Service '%s' not found in route", stableSvc)
+			return fmt.Errorf("Stable Service '%s' not found in the route.", stableSvc)
 		}
 	}
 	return nil