From d2c0b6ae771d78719972e8d9bebd9224c2c7e099 Mon Sep 17 00:00:00 2001 From: Darshan Hassan Shashikumar Date: Thu, 29 Jul 2021 13:41:04 -0700 Subject: [PATCH] Multiple VirtualService objects support for canary strategy of Argo Rollouts --- manifests/crds/rollout-crd.yaml | 15 +- manifests/install.yaml | 15 +- manifests/namespace-install.yaml | 15 +- pkg/apiclient/rollout/rollout.swagger.json | 7 + pkg/apis/api-rules/violation_exceptions.list | 1 + pkg/apis/rollouts/v1alpha1/types.go | 4 +- .../v1alpha1/zz_generated.deepcopy.go | 7 + .../validation/validation_references.go | 58 ++-- .../validation/validation_references_test.go | 39 ++- rollout/trafficrouting/istio/controller.go | 61 +++- .../trafficrouting/istio/controller_test.go | 113 ++++++++ rollout/trafficrouting/istio/istio.go | 75 ++--- rollout/trafficrouting/istio/istio_test.go | 267 +++++++++++++++++- utils/istio/istio.go | 33 ++- utils/istio/istio_test.go | 40 +++ 15 files changed, 665 insertions(+), 85 deletions(-) diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index a54aaa4891..a1ee3d7928 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/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: diff --git a/manifests/install.yaml b/manifests/install.yaml index 84c413ad56..f96d0a6dd0 100644 --- a/manifests/install.yaml +++ b/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: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 48f30b6761..03ce4c3922 100644 --- a/manifests/namespace-install.yaml +++ b/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: diff --git a/pkg/apiclient/rollout/rollout.swagger.json b/pkg/apiclient/rollout/rollout.swagger.json index 9e20548068..bc98c83946 100644 --- a/pkg/apiclient/rollout/rollout.swagger.json +++ b/pkg/apiclient/rollout/rollout.swagger.json @@ -817,6 +817,13 @@ "destinationRule": { "$ref": "#/definitions/github.com.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.com.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" diff --git a/pkg/apis/api-rules/violation_exceptions.list b/pkg/apis/api-rules/violation_exceptions.list index 63d5e8fe27..00fa06730e 100644 --- a/pkg/apis/api-rules/violation_exceptions.list +++ b/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 diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index 589fdc821d..13f5b06556 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/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 diff --git a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go index ab24ca447e..adf16c0da2 100644 --- a/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/rollouts/v1alpha1/zz_generated.deepcopy.go @@ -921,6 +921,13 @@ func (in *IstioTrafficRouting) DeepCopyInto(out *IstioTrafficRouting) { *out = new(IstioDestinationRule) **out = **in } + if in.VirtualServices != nil { + in, out := &in.VirtualServices, &out.VirtualServices + *out = make([]IstioVirtualService, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/apis/rollouts/validation/validation_references.go b/pkg/apis/rollouts/validation/validation_references.go index 35ae142cd1..5afa45988f 100644 --- a/pkg/apis/rollouts/validation/validation_references.go +++ b/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 } diff --git a/pkg/apis/rollouts/validation/validation_references_test.go b/pkg/apis/rollouts/validation/validation_references_test.go index 734db40ba4..a196c34355 100644 --- a/pkg/apis/rollouts/validation/validation_references_test.go +++ b/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()) }) diff --git a/rollout/trafficrouting/istio/controller.go b/rollout/trafficrouting/istio/controller.go index b49132758c..2f84493010 100644 --- a/rollout/trafficrouting/istio/controller.go +++ b/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 diff --git a/rollout/trafficrouting/istio/controller_test.go b/rollout/trafficrouting/istio/controller_test.go index 19ee95af42..ed8121e45a 100644 --- a/rollout/trafficrouting/istio/controller_test.go +++ b/rollout/trafficrouting/istio/controller_test.go @@ -2,6 +2,7 @@ package istio import ( "context" + "fmt" "testing" "time" @@ -78,6 +79,118 @@ func TestGetReferencedVirtualServices(t *testing.T) { }) } +const istiovsvc1 = `apiVersion: networking.istio.io/v1alpha3 +kind: VirtualService +metadata: + name: istio-vsvc1-name + namespace: default +spec: + gateways: + - istio-rollout-gateway + hosts: + - istio-rollout.dev.argoproj.io + http: + - name: primary + route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0 + - name: secondary + route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0` + +func TestGetReferencedMultipleVirtualServices(t *testing.T) { + + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "istio-vsvc1-name", Routes: nil}, {Name: "istio-vsvc2-name", Routes: nil}} + + ro := v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + Canary: &v1alpha1.CanaryStrategy{}, + }, + }, + } + ro.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{ + Istio: &v1alpha1.IstioTrafficRouting{ + VirtualServices: multipleVirtualService, + }, + } + ro.Namespace = metav1.NamespaceDefault + + vService := unstructuredutil.StrToUnstructuredUnsafe(istiovsvc1) + + t.Run("get referenced virtualService - fail", func(t *testing.T) { + c := NewFakeIstioController(vService) + _, err := c.GetReferencedVirtualServices(&ro) + expectedErr := field.Invalid(field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio", "virtualServices", "name"), "istio-vsvc2-name", "virtualservices.networking.istio.io \"istio-vsvc2-name\" not found") + assert.Equal(t, expectedErr.Error(), err.Error()) + }) +} + +func TestGetReferencedMultipleVirtualServicesInvalidConfig(t *testing.T) { + + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "istio-vsvc1-name", Routes: nil}} + + ro := v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + Canary: &v1alpha1.CanaryStrategy{}, + }, + }, + } + ro.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{ + Istio: &v1alpha1.IstioTrafficRouting{ + VirtualService: v1alpha1.IstioVirtualService{ + Name: "istio-vsvc1-name", + }, + VirtualServices: multipleVirtualService, + }, + } + ro.Namespace = metav1.NamespaceDefault + + vService := unstructuredutil.StrToUnstructuredUnsafe(istiovsvc1) + + t.Run("get referenced virtualService - fail", func(t *testing.T) { + c := NewFakeIstioController(vService) + _, err := c.GetReferencedVirtualServices(&ro) + fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio") + expected := fmt.Sprintf("%s: Internal error: either VirtualService or VirtualServices must be configured", fldPath) + assert.Equal(t, expected, err.Error()) + }) +} + +func TestGetReferencedNoVirtualServicesConfig(t *testing.T) { + ro := v1alpha1.Rollout{ + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + Canary: &v1alpha1.CanaryStrategy{}, + }, + }, + } + ro.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{ + Istio: &v1alpha1.IstioTrafficRouting{}, + } + ro.Namespace = metav1.NamespaceDefault + + vService := unstructuredutil.StrToUnstructuredUnsafe(istiovsvc1) + + t.Run("get referenced virtualService - fail", func(t *testing.T) { + c := NewFakeIstioController(vService) + _, err := c.GetReferencedVirtualServices(&ro) + fldPath := field.NewPath("spec", "strategy", "canary", "trafficRouting", "istio") + expected := fmt.Sprintf("%s: Internal error: either VirtualService or VirtualServices must be configured", fldPath) + assert.Equal(t, expected, err.Error()) + }) +} + func TestSyncDestinationRule(t *testing.T) { ro := &v1alpha1.Rollout{ ObjectMeta: metav1.ObjectMeta{ diff --git a/rollout/trafficrouting/istio/istio.go b/rollout/trafficrouting/istio/istio.go index 6119f20d88..2f09f7d624 100644 --- a/rollout/trafficrouting/istio/istio.go +++ b/rollout/trafficrouting/istio/istio.go @@ -80,7 +80,7 @@ func (patches virtualServicePatches) patchVirtualService(httpRoutes []interface{ return nil } -func (r *Reconciler) generateVirtualServicePatches(httpRoutes []VirtualServiceHTTPRoute, desiredWeight int64) virtualServicePatches { +func (r *Reconciler) generateVirtualServicePatches(routeNames []string, httpRoutes []VirtualServiceHTTPRoute, desiredWeight int64) virtualServicePatches { canarySvc := r.rollout.Spec.Strategy.Canary.CanaryService stableSvc := r.rollout.Spec.Strategy.Canary.StableService canarySubset := "" @@ -91,7 +91,7 @@ func (r *Reconciler) generateVirtualServicePatches(httpRoutes []VirtualServiceHT } // err can be ignored because we already called ValidateHTTPRoutes earlier - routeIndexesToPatch, _ := getRouteIndexesToPatch(r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) + routeIndexesToPatch, _ := getRouteIndexesToPatch(routeNames, httpRoutes) patches := virtualServicePatches{} for _, routeIdx := range routeIndexesToPatch { @@ -133,7 +133,7 @@ func (r *Reconciler) generateVirtualServicePatches(httpRoutes []VirtualServiceHT return patches } -func (r *Reconciler) reconcileVirtualService(obj *unstructured.Unstructured, desiredWeight int32) (*unstructured.Unstructured, bool, error) { +func (r *Reconciler) reconcileVirtualService(obj *unstructured.Unstructured, routeNames []string, desiredWeight int32) (*unstructured.Unstructured, bool, error) { newObj := obj.DeepCopy() httpRoutesI, err := GetHttpRoutesI(newObj) if err != nil { @@ -144,11 +144,11 @@ func (r *Reconciler) reconcileVirtualService(obj *unstructured.Unstructured, des return nil, false, err } - if err := ValidateHTTPRoutes(r.rollout, httpRoutes); err != nil { + if err := ValidateHTTPRoutes(r.rollout, routeNames, httpRoutes); err != nil { return nil, false, err } - patches := r.generateVirtualServicePatches(httpRoutes, int64(desiredWeight)) + patches := r.generateVirtualServicePatches(routeNames, httpRoutes, int64(desiredWeight)) err = patches.patchVirtualService(httpRoutesI) if err != nil { return nil, false, err @@ -436,34 +436,45 @@ func (r *Reconciler) SetWeight(desiredWeight int32) error { ctx := context.TODO() var vsvc *unstructured.Unstructured var err error + var virtualServices []v1alpha1.IstioVirtualService - namespace, vsvcName := istioutil.GetVirtualServiceNamespaceName(r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name) - if namespace == "" { - namespace = r.rollout.Namespace - } - client := r.client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(namespace) - if r.virtualServiceLister != nil { - vsvc, err = r.virtualServiceLister.Namespace(namespace).Get(vsvcName) + if istioutil.MultipleVirtualServiceConfigured(r.rollout) { + virtualServices = r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices } else { - vsvc, err = client.Get(ctx, vsvcName, metav1.GetOptions{}) + virtualServices = []v1alpha1.IstioVirtualService{r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService} } - if err != nil { - if k8serrors.IsNotFound(err) { - r.recorder.Warnf(r.rollout, record.EventOptions{EventReason: "VirtualServiceNotFound"}, "VirtualService `%s` not found", vsvcName) + + for _, virtualService := range virtualServices { + name := virtualService.Name + namespace, vsvcName := istioutil.GetVirtualServiceNamespaceName(name) + if namespace == "" { + namespace = r.rollout.Namespace + } + + client := r.client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(namespace) + if r.virtualServiceLister != nil { + vsvc, err = r.virtualServiceLister.Namespace(namespace).Get(vsvcName) + } else { + vsvc, err = client.Get(ctx, vsvcName, metav1.GetOptions{}) + } + if err != nil { + if k8serrors.IsNotFound(err) { + r.recorder.Warnf(r.rollout, record.EventOptions{EventReason: "VirtualServiceNotFound"}, "VirtualService `%s` not found", vsvcName) + } + return err + } + modifiedVirtualService, modified, err := r.reconcileVirtualService(vsvc, virtualService.Routes, desiredWeight) + if err != nil { + return err + } + if !modified { + continue + } + _, err = client.Update(ctx, modifiedVirtualService, metav1.UpdateOptions{}) + if err == nil { + r.log.Debugf("UpdatedVirtualService: %s", modifiedVirtualService) + r.recorder.Eventf(r.rollout, record.EventOptions{EventReason: "UpdatedVirtualService"}, "VirtualService `%s` set to desiredWeight '%d'", vsvcName, desiredWeight) } - return err - } - modifiedVsvc, modified, err := r.reconcileVirtualService(vsvc, desiredWeight) - if err != nil { - return err - } - if !modified { - return nil - } - _, err = client.Update(ctx, modifiedVsvc, metav1.UpdateOptions{}) - if err == nil { - r.log.Debugf("UpdatedVirtualService: %s", modifiedVsvc) - r.recorder.Eventf(r.rollout, record.EventOptions{EventReason: "UpdatedVirtualService"}, "VirtualService `%s` set to desiredWeight '%d'", vsvcName, desiredWeight) } return err } @@ -498,12 +509,12 @@ func getRouteIndexesToPatch(routeNames []string, httpRoutes []VirtualServiceHTTP return routeIndexesToPatch, nil } -// validateHTTPRoutes ensures that all the routes in the rollout exist and they only have two destinations -func ValidateHTTPRoutes(r *v1alpha1.Rollout, httpRoutes []VirtualServiceHTTPRoute) error { +// ValidateHTTPRoutes ensures that all the routes in the rollout exist and they only have two destinations +func ValidateHTTPRoutes(r *v1alpha1.Rollout, routeNames []string, httpRoutes []VirtualServiceHTTPRoute) error { stableSvc := r.Spec.Strategy.Canary.StableService canarySvc := r.Spec.Strategy.Canary.CanaryService - routeIndexesToPatch, err := getRouteIndexesToPatch(r.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) + routeIndexesToPatch, err := getRouteIndexesToPatch(routeNames, httpRoutes) if err != nil { return err } diff --git a/rollout/trafficrouting/istio/istio_test.go b/rollout/trafficrouting/istio/istio_test.go index 9c416ad465..d97e4ab513 100644 --- a/rollout/trafficrouting/istio/istio_test.go +++ b/rollout/trafficrouting/istio/istio_test.go @@ -129,8 +129,9 @@ func TestReconcileWeightsBaseCase(t *testing.T) { r := &Reconciler{ rollout: rollout("stable", "canary", "vsvc", []string{"primary"}), } + obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvc) - modifiedObj, _, err := r.reconcileVirtualService(obj, 10) + modifiedObj, _, err := r.reconcileVirtualService(obj, r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, 10) assert.Nil(t, err) assert.NotNil(t, modifiedObj) routes, ok, err := unstructured.NestedSlice(modifiedObj.Object, "spec", "http") @@ -298,7 +299,7 @@ func TestValidateHTTPRoutes(t *testing.T) { }}, }} rollout := newRollout([]string{"test"}) - err := ValidateHTTPRoutes(rollout, httpRoutes) + err := ValidateHTTPRoutes(rollout, rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) assert.Equal(t, fmt.Errorf("Route 'test' does not have exactly two routes"), err) httpRoutes[0].Route = []VirtualServiceHTTPRouteDestination{{ @@ -310,11 +311,11 @@ func TestValidateHTTPRoutes(t *testing.T) { Host: "canary", }, }} - err = ValidateHTTPRoutes(rollout, httpRoutes) + err = ValidateHTTPRoutes(rollout, rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) assert.Nil(t, err) rolloutWithNotFoundRoute := newRollout([]string{"not-found-route"}) - err = ValidateHTTPRoutes(rolloutWithNotFoundRoute, httpRoutes) + err = ValidateHTTPRoutes(rolloutWithNotFoundRoute, rolloutWithNotFoundRoute.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) assert.Equal(t, "Route 'not-found-route' is not found", err.Error()) } @@ -403,21 +404,21 @@ func TestValidateHTTPRoutesSubsets(t *testing.T) { { // the good case - err := ValidateHTTPRoutes(rollout, httpRoutes) + err := ValidateHTTPRoutes(rollout, rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) assert.NoError(t, err) } { // the stable subset doesnt exist rollout = rollout.DeepCopy() rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.StableSubsetName = "doesntexist" - err := ValidateHTTPRoutes(rollout, httpRoutes) + err := ValidateHTTPRoutes(rollout, rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) assert.EqualError(t, err, "Stable DestinationRule subset 'doesntexist' not found in route") } { // the canary subset doesnt exist rollout = rollout.DeepCopy() rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.CanarySubsetName = "doesntexist" - err := ValidateHTTPRoutes(rollout, httpRoutes) + err := ValidateHTTPRoutes(rollout, rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes, httpRoutes) assert.EqualError(t, err, "Canary DestinationRule subset 'doesntexist' not found in route") } } @@ -616,3 +617,255 @@ func TestUpdateHashDestinationRuleNotFound(t *testing.T) { assert.Len(t, actions, 0) assert.EqualError(t, err, "destinationrules.networking.istio.io \"istio-destrule\" not found") } + +//Multiple Virtual Service Support Unit Tests + +func multiVsRollout(stableSvc string, canarySvc string, multipleVirtualService []v1alpha1.IstioVirtualService) *v1alpha1.Rollout { + return &v1alpha1.Rollout{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rollout", + Namespace: "default", + }, + Spec: v1alpha1.RolloutSpec{ + Strategy: v1alpha1.RolloutStrategy{ + Canary: &v1alpha1.CanaryStrategy{ + StableService: stableSvc, + CanaryService: canarySvc, + TrafficRouting: &v1alpha1.RolloutTrafficRouting{ + Istio: &v1alpha1.IstioTrafficRouting{ + VirtualServices: multipleVirtualService, + }, + }, + }, + }, + }, + } +} + +const sampleRouteVirtualService1 = `apiVersion: networking.istio.io/v1alpha3 +kind: VirtualService +metadata: + name: vsvc1 + namespace: default +spec: + gateways: + - istio-rollout-gateway + hosts: + - istio-rollout.dev.argoproj.io + http: + - name: primary + route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0 + - name: secondary + route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0` + +const sampleRouteVirtualService2 = `apiVersion: networking.istio.io/v1alpha3 +kind: VirtualService +metadata: + name: vsvc2 + namespace: default +spec: + gateways: + - istio-rollout-gateway + hosts: + - istio-rollout.dev.argoproj.io + http: + - name: blue-green + route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0` + +const singleRouteMultipleVirtualService1 = `apiVersion: networking.istio.io/v1alpha3 +kind: VirtualService +metadata: + name: vsvc1 + namespace: default +spec: + gateways: + - istio-rollout-gateway + hosts: + - istio-rollout.dev.argoproj.io + http: + - route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0` + +const singleRouteMultipleVirtualService2 = `apiVersion: networking.istio.io/v1alpha3 +kind: VirtualService +metadata: + name: vsvc2 + namespace: default +spec: + gateways: + - istio-rollout-gateway + hosts: + - istio-rollout.dev.argoproj.io + http: + - route: + - destination: + host: 'stable' + weight: 100 + - destination: + host: canary + weight: 0` + +func TestMultipleVirtualServiceConfigured(t *testing.T) { + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: []string{"primary", "secondary"}}, {Name: "vsvc2", Routes: []string{"blue-green"}}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + mvsvc := istioutil.MultipleVirtualServiceConfigured(ro) + assert.Equal(t, true, mvsvc) + ro = rollout("stable", "canary", "vsvc", []string{"primary"}) + mvsvc = istioutil.MultipleVirtualServiceConfigured(ro) + assert.Equal(t, false, mvsvc) +} + +//This Testcase validates the reconcileVirtualService using VirtualServices configuration +func TestMultipleVirtualServiceReconcileWeightsBaseCase(t *testing.T) { + + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: []string{"secondary"}}} + mr := &Reconciler{ + rollout: multiVsRollout("stable", "canary", multipleVirtualService), + } + + obj := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService1) + + // Choosing the second virtual service i.e., secondary + vsvc := mr.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices[0] + modifiedObj, _, err := mr.reconcileVirtualService(obj, vsvc.Routes, 10) + assert.Nil(t, err) + assert.NotNil(t, modifiedObj) + routes, ok, err := unstructured.NestedSlice(modifiedObj.Object, "spec", "http") + assert.Nil(t, err) + assert.True(t, ok) + route := routes[1].(map[string]interface{}) + assert.Equal(t, route["name"].(string), "secondary") + checkDestination(t, route, "stable", 90) + checkDestination(t, route, "canary", 10) + unmodifiedRoute := routes[0].(map[string]interface{}) + assert.Equal(t, unmodifiedRoute["name"].(string), "primary") + checkDestination(t, unmodifiedRoute, "stable", 100) + checkDestination(t, unmodifiedRoute, "canary", 0) +} + +func TestMultipleVirtualServiceReconcileUpdateVirtualServices(t *testing.T) { + obj1 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService1) + obj2 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService2) + client := testutil.NewFakeDynamicClient(obj1, obj2) + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: []string{"primary", "secondary"}}, {Name: "vsvc2", Routes: []string{"blue-green"}}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(10) + assert.Nil(t, err) + actions := client.Actions() + assert.Len(t, actions, 2) + assert.Equal(t, "update", actions[0].GetVerb()) + assert.Equal(t, "update", actions[1].GetVerb()) +} + +func TestMultipleVirtualServiceReconcileNoChanges(t *testing.T) { + obj1 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService1) + obj2 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService2) + client := testutil.NewFakeDynamicClient(obj1, obj2) + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: []string{"primary", "secondary"}}, {Name: "vsvc2", Routes: []string{"blue-green"}}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), nil, nil) + err := r.SetWeight(0) + assert.Nil(t, err) + assert.Len(t, client.Actions(), 2) + assert.Equal(t, "get", client.Actions()[0].GetVerb()) + assert.Equal(t, "get", client.Actions()[1].GetVerb()) +} + +func TestMultipleVirtualServiceReconcileInvalidValidation(t *testing.T) { + obj1 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService1) + obj2 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService2) + client := testutil.NewFakeDynamicClient(obj1, obj2) + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: []string{"route-not-found"}}, {Name: "vsvc2", Routes: []string{"route-not-found"}}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(0) + assert.Equal(t, "Route 'route-not-found' is not found", err.Error()) +} + +func TestMultipleVirtualServiceReconcileVirtualServiceNotFound(t *testing.T) { + obj := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService1) + client := testutil.NewFakeDynamicClient(obj) + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: []string{"primary", "secondary"}}, {Name: "vsvc2", Routes: []string{"blue-green"}}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(10) + assert.NotNil(t, err) + assert.True(t, k8serrors.IsNotFound(err)) +} + +// TestReconcileAmbiguousRoutes tests when we omit route names and there are multiple routes in the VirtualService +func TestMultipleVirtualServiceReconcileAmbiguousRoutes(t *testing.T) { + obj1 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService1) + obj2 := unstructuredutil.StrToUnstructuredUnsafe(sampleRouteVirtualService2) + client := testutil.NewFakeDynamicClient(obj1, obj2) + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: nil}, {Name: "vsvc2", Routes: nil}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(0) + assert.Equal(t, "VirtualService spec.http[] must have exactly one route when omitting spec.strategy.canary.trafficRouting.istio.virtualService.routes", err.Error()) +} + +// TestReconcileInferredSingleRoute we can support case where we infer the only route in the VirtualService +func TestMultipleVirtualServiceReconcileInferredSingleRoute(t *testing.T) { + obj1 := unstructuredutil.StrToUnstructuredUnsafe(singleRouteMultipleVirtualService1) + obj2 := unstructuredutil.StrToUnstructuredUnsafe(singleRouteMultipleVirtualService2) + client := testutil.NewFakeDynamicClient(obj1, obj2) + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "vsvc1", Routes: nil}, {Name: "vsvc2", Routes: nil}} + ro := multiVsRollout("stable", "canary", multipleVirtualService) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(10) + assert.NoError(t, err) + actions := client.Actions() + assert.Len(t, actions, 2) + assert.Equal(t, "update", actions[0].GetVerb()) + assert.Equal(t, "update", actions[1].GetVerb()) + + // Verify we actually made the correct change + vsvcUn1, err := client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(ro.Namespace).Get(context.TODO(), "vsvc1", metav1.GetOptions{}) + assert.NoError(t, err) + routes1, _, _ := unstructured.NestedSlice(vsvcUn1.Object, "spec", "http") + route1 := routes1[0].(map[string]interface{}) + checkDestination(t, route1, "stable", 90) + checkDestination(t, route1, "canary", 10) + + vsvcUn2, err := client.Resource(istioutil.GetIstioVirtualServiceGVR()).Namespace(ro.Namespace).Get(context.TODO(), "vsvc2", metav1.GetOptions{}) + assert.NoError(t, err) + routes2, _, _ := unstructured.NestedSlice(vsvcUn2.Object, "spec", "http") + route2 := routes2[0].(map[string]interface{}) + checkDestination(t, route2, "stable", 90) + checkDestination(t, route2, "canary", 10) +} diff --git a/utils/istio/istio.go b/utils/istio/istio.go index bc5e4a3c85..be09ee8d15 100644 --- a/utils/istio/istio.go +++ b/utils/istio/istio.go @@ -64,19 +64,42 @@ func GetVirtualServiceNamespaceName(vsv string) (string, string) { return namespace, name } +func MultipleVirtualServiceConfigured(rollout *v1alpha1.Rollout) bool { + if rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices != nil { + return true + } + return false +} + // GetRolloutVirtualServiceKeys gets the referenced VirtualService and its namespace from a Rollout func GetRolloutVirtualServiceKeys(ro *v1alpha1.Rollout) []string { + var virtualServices []v1alpha1.IstioVirtualService + var virtualServiceKeys []string canary := ro.Spec.Strategy.Canary - if canary == nil || canary.TrafficRouting == nil || canary.TrafficRouting.Istio == nil || canary.TrafficRouting.Istio.VirtualService.Name == "" { + + if canary == nil || canary.TrafficRouting == nil || canary.TrafficRouting.Istio == nil || + (canary.TrafficRouting.Istio.VirtualServices == nil && canary.TrafficRouting.Istio.VirtualService.Name == "") { return []string{} } - namespace, name := GetVirtualServiceNamespaceName(canary.TrafficRouting.Istio.VirtualService.Name) - if namespace == "" { - namespace = ro.Namespace + if MultipleVirtualServiceConfigured(ro) { + if canary.TrafficRouting.Istio.VirtualService.Name != "" { + return []string{} + } + virtualServices = ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices + } else { + virtualServices = []v1alpha1.IstioVirtualService{ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService} + } + + for _, virtualService := range virtualServices { + namespace, name := GetVirtualServiceNamespaceName(virtualService.Name) + if namespace == "" { + namespace = ro.Namespace + } + virtualServiceKeys = append(virtualServiceKeys, fmt.Sprintf("%s/%s", namespace, name)) } - return []string{fmt.Sprintf("%s/%s", namespace, name)} + return virtualServiceKeys } // GetRolloutDesinationRuleKeys gets the referenced DestinationRule and its namespace from a Rollout diff --git a/utils/istio/istio_test.go b/utils/istio/istio_test.go index 6eb78f0188..2c4e78df9a 100644 --- a/utils/istio/istio_test.go +++ b/utils/istio/istio_test.go @@ -66,6 +66,17 @@ func TestGetRolloutVirtualServiceKeys(t *testing.T) { VirtualService: v1alpha1.IstioVirtualService{}, } assert.Len(t, GetRolloutVirtualServiceKeys(ro), 0) + ro.Spec.Strategy.Canary.TrafficRouting.Istio = &v1alpha1.IstioTrafficRouting{ + VirtualServices: []v1alpha1.IstioVirtualService{}, + } + assert.Len(t, GetRolloutVirtualServiceKeys(ro), 0) + + multipleVirtualService := []v1alpha1.IstioVirtualService{{Name: "test1", Routes: nil}, {Name: "test2", Routes: nil}} + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name = "test" + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices = multipleVirtualService + assert.Len(t, GetRolloutVirtualServiceKeys(ro), 0) + + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices = nil ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name = "test" keys := GetRolloutVirtualServiceKeys(ro) assert.Len(t, keys, 1) @@ -80,6 +91,35 @@ func TestGetRolloutVirtualServiceKeys(t *testing.T) { keys = GetRolloutVirtualServiceKeys(ro) assert.Len(t, keys, 1) assert.Equal(t, keys[0], "namespace/test") + + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name = "" + virtualServices := []v1alpha1.IstioVirtualService{{Name: "test", Routes: nil}} + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices = virtualServices + keys = GetRolloutVirtualServiceKeys(ro) + assert.Len(t, keys, 1) + assert.Equal(t, keys[0], "default/test") + + multipleVirtualService = []v1alpha1.IstioVirtualService{{Name: "test1", Routes: nil}, {Name: "test2", Routes: nil}} + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices = multipleVirtualService + keys = GetRolloutVirtualServiceKeys(ro) + assert.Len(t, keys, 2) + assert.Equal(t, keys[0], "default/test1") + assert.Equal(t, keys[1], "default/test2") + + multipleVirtualService = []v1alpha1.IstioVirtualService{{Name: "test1.namespace", Routes: nil}, {Name: "test2.namespace", Routes: nil}} + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices = multipleVirtualService + keys = GetRolloutVirtualServiceKeys(ro) + assert.Len(t, keys, 2) + assert.Equal(t, keys[0], "namespace/test1") + assert.Equal(t, keys[1], "namespace/test2") + + multipleVirtualService = []v1alpha1.IstioVirtualService{{Name: "test1.namespace.cluster.local", Routes: nil}, {Name: "test2.namespace.cluster.local", Routes: nil}} + ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualServices = multipleVirtualService + keys = GetRolloutVirtualServiceKeys(ro) + assert.Len(t, keys, 2) + assert.Equal(t, keys[0], "namespace/test1") + assert.Equal(t, keys[1], "namespace/test2") + } func TestGetRolloutDesinationRuleKeys(t *testing.T) {