Multiple VirtualService objects support for canary strategy of Argo R…

…ollouts

manifests/crds/rollout-crd.yaml

@@ -532,8 +532,19 @@ spec:
                                 required:
                                 - name
                                 type: object
-                            required:
-                            - virtualService
+                              virtualServices:
+                                items:
+                                  properties:
+                                    name:
+                                      type: string
+                                    routes:
+                                      items:
+                                        type: string
+                                      type: array
+                                  required:
+                                  - name
+                                  type: object
+                                type: array
                             type: object
                           nginx:
                             properties:

manifests/install.yaml

@@ -10230,8 +10230,19 @@ spec:
                                 required:
                                 - name
                                 type: object
-                            required:
-                            - virtualService
+                              virtualServices:
+                                items:
+                                  properties:
+                                    name:
+                                      type: string
+                                    routes:
+                                      items:
+                                        type: string
+                                      type: array
+                                  required:
+                                  - name
+                                  type: object
+                                type: array
                             type: object
                           nginx:
                             properties:

manifests/namespace-install.yaml

@@ -10230,8 +10230,19 @@ spec:
                                 required:
                                 - name
                                 type: object
-                            required:
-                            - virtualService
+                              virtualServices:
+                                items:
+                                  properties:
+                                    name:
+                                      type: string
+                                    routes:
+                                      items:
+                                        type: string
+                                      type: array
+                                  required:
+                                  - name
+                                  type: object
+                                type: array
                             type: object
                           nginx:
                             properties:

pkg/apiclient/rollout/rollout.swagger.json

@@ -817,6 +817,13 @@
         "destinationRule": {
           "$ref": "#/definitions/github.jparrowsec.cn.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.IstioDestinationRule",
           "title": "DestinationRule references an Istio DestinationRule to modify to shape traffic"
+        },
+        "virtualServices": {
+          "type": "array",
+          "items": {
+            "$ref": "#/definitions/github.jparrowsec.cn.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.IstioVirtualService"
+          },
+          "title": "VirtualServices references a list of Istio VirtualService to modify to shape traffic"
         }
       },
       "title": "IstioTrafficRouting configuration for Istio service mesh to enable fine grain configuration"

pkg/apis/api-rules/violation_exceptions.list

@@ -11,6 +11,7 @@ API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,ExperimentStatus,AnalysisRuns
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,ExperimentStatus,Conditions
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,ExperimentStatus,TemplateStatuses
+API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,IstioTrafficRouting,VirtualServices
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,IstioVirtualService,Routes
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,KayentaMetric,Scopes
 API rule violation: list_type_missing,github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1,MetricResult,Measurements

pkg/apis/rollouts/v1alpha1/types.go

@@ -354,9 +354,11 @@ type NginxTrafficRouting struct {
 // IstioTrafficRouting configuration for Istio service mesh to enable fine grain configuration
 type IstioTrafficRouting struct {
 	// VirtualService references an Istio VirtualService to modify to shape traffic
-	VirtualService IstioVirtualService `json:"virtualService" protobuf:"bytes,1,opt,name=virtualService"`
+	VirtualService IstioVirtualService `json:"virtualService,omitempty" 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"`
+	// VirtualServices references a list of Istio VirtualService to modify to shape traffic
+	VirtualServices []IstioVirtualService `json:"virtualServices,omitempty" protobuf:"bytes,3,opt,name=virtualServices"`
 }
 
 // IstioVirtualService holds information on the virtual service the rollout needs to modify

pkg/apis/rollouts/validation/validation_references.go

@@ -3,12 +3,12 @@ package validation
 import (
 	"fmt"
 
-	appsv1 "k8s.io/api/apps/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	analysisutil "github.com/argoproj/argo-rollouts/utils/analysis"
 	"github.com/argoproj/argo-rollouts/utils/conditions"
+	istioutil "github.com/argoproj/argo-rollouts/utils/istio"
 	serviceutil "github.com/argoproj/argo-rollouts/utils/service"
+	appsv1 "k8s.io/api/apps/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	ingressutil "github.com/argoproj/argo-rollouts/utils/ingress"
 	corev1 "k8s.io/api/core/v1"
@@ -214,25 +214,45 @@ func ValidateIngress(rollout *v1alpha1.Rollout, ingress v1beta1.Ingress) field.E
 }
 
 func ValidateVirtualService(rollout *v1alpha1.Rollout, obj unstructured.Unstructured) field.ErrorList {
-	allErrs := field.ErrorList{}
+	var fldPath *field.Path
+	var virtualServices []v1alpha1.IstioVirtualService
 	newObj := obj.DeepCopy()
-	fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name")
-	vsvcName := rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name
-	httpRoutesI, err := istio.GetHttpRoutesI(newObj)
-	if err != nil {
-		msg := fmt.Sprintf("Unable to get HTTP routes for Istio VirtualService")
-		allErrs = append(allErrs, field.Invalid(fldPath, vsvcName, msg))
-	}
-	httpRoutes, err := istio.GetHttpRoutes(newObj, httpRoutesI)
-	if err != nil {
-		msg := fmt.Sprintf("Unable to get HTTP routes for Istio VirtualService")
-		allErrs = append(allErrs, field.Invalid(fldPath, vsvcName, msg))
+	allErrs := field.ErrorList{}
+
+	if istioutil.MultipleVirtualServiceConfigured(rollout) {
+		fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualServices", "name")
+		virtualServices = rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices
+	} else {
+		fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name")
+		virtualServices = []v1alpha1.IstioVirtualService{rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService}
 	}
-	err = istio.ValidateHTTPRoutes(rollout, httpRoutes)
-	if err != nil {
-		msg := fmt.Sprintf("Istio VirtualService has invalid HTTP routes. Error: %s", err.Error())
-		allErrs = append(allErrs, field.Invalid(fldPath, vsvcName, msg))
+
+	virtualServiceRecordName := obj.GetName()
+
+	for _, virtualService := range virtualServices {
+		name := virtualService.Name
+		_, vsvcName := istioutil.GetVirtualServiceNamespaceName(name)
+		if vsvcName == virtualServiceRecordName {
+			httpRoutesI, err := istio.GetHttpRoutesI(newObj)
+			if err != nil {
+				msg := fmt.Sprintf("Unable to get HTTP routes for Istio VirtualService")
+				allErrs = append(allErrs, field.Invalid(fldPath, vsvcName, msg))
+			}
+			httpRoutes, err := istio.GetHttpRoutes(newObj, httpRoutesI)
+			if err != nil {
+				msg := fmt.Sprintf("Unable to get HTTP routes for Istio VirtualService")
+				allErrs = append(allErrs, field.Invalid(fldPath, vsvcName, msg))
+			}
+
+			err = istio.ValidateHTTPRoutes(rollout, virtualService.Routes, httpRoutes)
+			if err != nil {
+				msg := fmt.Sprintf("Istio VirtualService has invalid HTTP routes. Error: %s", err.Error())
+				allErrs = append(allErrs, field.Invalid(fldPath, vsvcName, msg))
+			}
+			break
+		}
 	}
+
 	return allErrs
 }
 

pkg/apis/rollouts/validation/validation_references_test.go

@@ -356,7 +356,7 @@ func TestValidateVirtualService(t *testing.T) {
 					TrafficRouting: &v1alpha1.RolloutTrafficRouting{
 						Istio: &v1alpha1.IstioTrafficRouting{
 							VirtualService: v1alpha1.IstioVirtualService{
-								Name: "istio-vsvc-name",
+								Name: "istio-vsvc",
 								Routes: []string{
 									"primary",
 									"secondary",
@@ -379,7 +379,42 @@ func TestValidateVirtualService(t *testing.T) {
 		vsvc := unstructured.StrToUnstructuredUnsafe(failCaseVsvc)
 		allErrs := ValidateVirtualService(ro, *vsvc)
 		assert.Len(t, allErrs, 1)
-		expectedErr := field.Invalid(field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name"), "istio-vsvc-name", "Istio VirtualService has invalid HTTP routes. Error: Stable Service 'stable' not found in route")
+		expectedErr := field.Invalid(field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name"), "istio-vsvc", "Istio VirtualService has invalid HTTP routes. Error: Stable Service 'stable' not found in route")
+		assert.Equal(t, expectedErr.Error(), allErrs[0].Error())
+
+	})
+}
+
+func TestValidateVirtualServices(t *testing.T) {
+	multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "istio-vsvc", Routes: []string{"primary", "secondary"}}}
+
+	ro := &v1alpha1.Rollout{
+		Spec: v1alpha1.RolloutSpec{
+			Strategy: v1alpha1.RolloutStrategy{
+				Canary: &v1alpha1.CanaryStrategy{
+					StableService: "stable",
+					CanaryService: "canary",
+					TrafficRouting: &v1alpha1.RolloutTrafficRouting{
+						Istio: &v1alpha1.IstioTrafficRouting{
+							VirtualServices: multipleVirtualService,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	t.Run("validate virtualService - success", func(t *testing.T) {
+		vsvc := unstructured.StrToUnstructuredUnsafe(successCaseVsvc)
+		allErrs := ValidateVirtualService(ro, *vsvc)
+		assert.Empty(t, allErrs)
+	})
+
+	t.Run("validate virtualService - failure", func(t *testing.T) {
+		vsvc := unstructured.StrToUnstructuredUnsafe(failCaseVsvc)
+		allErrs := ValidateVirtualService(ro, *vsvc)
+		assert.Len(t, allErrs, 1)
+		expectedErr := field.Invalid(field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualServices", "name"), "istio-vsvc", "Istio VirtualService has invalid HTTP routes. Error: Stable Service 'stable' not found in route")
 		assert.Equal(t, expectedErr.Error(), allErrs[0].Error())
 
 	})

rollout/trafficrouting/istio/controller.go

@@ -174,32 +174,67 @@ func (c *IstioController) EnqueueRolloutFromIstioVirtualService(vsvc interface{}
 	}
 }
 
+func (c *IstioController) validateRolloutVirtualServicesConfig(r *v1alpha1.Rollout) error {
+	//Either VirtualService or VirtualServices must be configured.
+	//If both configured then it is an invalid configuration
+	errorString := "either VirtualService or VirtualServices must be configured"
+	if istioutil.MultipleVirtualServiceConfigured(r) {
+		if r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name != "" {
+			return fmt.Errorf(errorString)
+		}
+	} else {
+		if r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name == "" {
+			return fmt.Errorf(errorString)
+		}
+	}
+	return nil
+}
+
 func (c *IstioController) GetReferencedVirtualServices(ro *v1alpha1.Rollout) (*[]unstructured.Unstructured, error) {
+	var fldPath *field.Path
+	fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio")
 	ctx := context.TODO()
 	virtualServices := []unstructured.Unstructured{}
-	fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name")
 	if ro.Spec.Strategy.Canary != nil {
 		canary := ro.Spec.Strategy.Canary
 		if canary.TrafficRouting != nil && canary.TrafficRouting.Istio != nil {
 			var vsvc *unstructured.Unstructured
 			var err error
-			vsvcNamespace, vsvcName := istioutil.GetVirtualServiceNamespaceName(canary.TrafficRouting.Istio.VirtualService.Name)
-			if vsvcNamespace == "" {
-				vsvcNamespace = ro.Namespace
+			var vsvcs []v1alpha1.IstioVirtualService
+
+			err = c.validateRolloutVirtualServicesConfig(ro)
+			if err != nil {
+				return nil, field.InternalError(fldPath, err)
 			}
-			if c.VirtualServiceInformer.HasSynced() {
-				vsvc, err = c.VirtualServiceLister.Namespace(vsvcNamespace).Get(vsvcName)
+
+			if istioutil.MultipleVirtualServiceConfigured(ro) {
+				vsvcs = canary.TrafficRouting.Istio.VirtualServices
+				fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualServices", "name")
 			} else {
-				vsvc, err = c.DynamicClientSet.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(vsvcNamespace).Get(ctx, vsvcName, metav1.GetOptions{})
+				vsvcs = []v1alpha1.IstioVirtualService{canary.TrafficRouting.Istio.VirtualService}
+				fldPath = field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualService", "name")
 			}
 
-			if k8serrors.IsNotFound(err) {
-				return nil, field.Invalid(fldPath, vsvcName, err.Error())
-			}
-			if err != nil {
-				return nil, err
+			for _, eachVsvc := range vsvcs {
+				vsvcNamespace, vsvcName := istioutil.GetVirtualServiceNamespaceName(eachVsvc.Name)
+				if vsvcNamespace == "" {
+					vsvcNamespace = ro.Namespace
+				}
+				if c.VirtualServiceInformer.HasSynced() {
+					vsvc, err = c.VirtualServiceLister.Namespace(vsvcNamespace).Get(vsvcName)
+				} else {
+					vsvc, err = c.DynamicClientSet.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(vsvcNamespace).Get(ctx, vsvcName, metav1.GetOptions{})
+				}
+
+				if k8serrors.IsNotFound(err) {
+					return nil, field.Invalid(fldPath, vsvcName, err.Error())
+				}
+				if err != nil {
+					return nil, err
+				}
+
+				virtualServices = append(virtualServices, *vsvc)
 			}
-			virtualServices = append(virtualServices, *vsvc)
 		}
 	}
 	return &virtualServices, nil