From 0593d92a9acf13dc9d578f82a00c0efa3f09c463 Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Thu, 2 Nov 2023 12:50:50 -0500 Subject: [PATCH] fix: Revert "fix: istio destionationrule subsets enforcement (#3126)" (#3147) Revert "fix: istio destionationrule subsets enforcement (#3126)" This reverts commit 04e11195fb4046e897574478c51fb14692a07578. Signed-off-by: zachaller --- USERS.md | 1 - rollout/trafficrouting/istio/istio.go | 17 ---- rollout/trafficrouting/istio/istio_test.go | 110 --------------------- 3 files changed, 128 deletions(-) diff --git a/USERS.md b/USERS.md index 5ca758b084..4ad9fb29bf 100644 --- a/USERS.md +++ b/USERS.md @@ -22,7 +22,6 @@ Organizations below are **officially** using Argo Rollouts. Please send a PR wit 1. [Ibotta](https://home.ibotta.com/) 1. [Intuit](https://www.intuit.com/) 1. [New Relic](https://newrelic.com/) -1. [Nextdoor](https://nextdoor.com/) 1. [Nitro](https://www.gonitro.com) 1. [Nozzle](https://nozzle.io) 1. [Opensurvey Inc.](https://opensurvey.co.kr) diff --git a/rollout/trafficrouting/istio/istio.go b/rollout/trafficrouting/istio/istio.go index 8a47a1011d..dd44da7b25 100644 --- a/rollout/trafficrouting/istio/istio.go +++ b/rollout/trafficrouting/istio/istio.go @@ -255,23 +255,6 @@ func (r *Reconciler) reconcileVirtualService(obj *unstructured.Unstructured, vsv if err = ValidateHTTPRoutes(r.rollout, vsvcRouteNames, httpRoutes); err != nil { return nil, false, err } - - host, err := r.getDestinationRuleHost() - if err != nil { - return nil, false, err - } - - if host != "" { - var routeDestinations []VirtualServiceRouteDestination - for i, routes := range httpRoutes { - for _, r := range routes.Route { - if r.Destination.Host == host { - routeDestinations = append(routeDestinations, VirtualServiceRouteDestination{Destination: r.Destination, Weight: r.Weight}) - } - } - httpRoutes[i].Route = routeDestinations - } - } } // TLS Routes diff --git a/rollout/trafficrouting/istio/istio_test.go b/rollout/trafficrouting/istio/istio_test.go index ead1b7e1ee..c18cdedae5 100644 --- a/rollout/trafficrouting/istio/istio_test.go +++ b/rollout/trafficrouting/istio/istio_test.go @@ -465,31 +465,6 @@ spec: subset: 'canary-subset' weight: 0` -const singleRouteSubsetMultipleDestRuleVsvc = `apiVersion: networking.istio.io/v1alpha3 -kind: VirtualService -metadata: - name: vsvc - namespace: default -spec: - gateways: - - istio-rollout-gateway - hosts: - - istio-rollout.dev.argoproj.io - http: - - route: - - destination: - host: rollout-service - subset: stable - weight: 100 - - destination: - host: rollout-service - subset: canary - weight: 0 - - destination: - host: additional-service - subset: stable-subset - weight: 20` - const singleRouteTlsVsvc = `apiVersion: networking.istio.io/v1alpha3 kind: VirtualService metadata: @@ -720,91 +695,6 @@ func TestHttpReconcileWeightsBaseCase(t *testing.T) { } } -func TestHttpReconcileMultipleDestRule(t *testing.T) { - ro := rolloutWithDestinationRule() - ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name = "vsvc" - ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes = nil - - dRule1 := unstructuredutil.StrToUnstructuredUnsafe(` -apiVersion: networking.istio.io/v1alpha3 -kind: DestinationRule -metadata: - name: istio-destrule - namespace: default -spec: - host: rollout-service - subsets: - - name: stable - - name: canary -`) - dRule2 := unstructuredutil.StrToUnstructuredUnsafe(` -apiVersion: networking.istio.io/v1alpha3 -kind: DestinationRule -metadata: - name: additional-istio-destrule - namespace: default -spec: - host: additional-service - subsets: - - name: stable-subset -`) - - obj := unstructuredutil.StrToUnstructuredUnsafe(singleRouteSubsetMultipleDestRuleVsvc) - client := testutil.NewFakeDynamicClient(obj, dRule1, dRule2) - vsvcLister, druleLister := getIstioListers(client) - r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) - client.ClearActions() - - vsvcRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes - vsvcTLSRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.TLSRoutes - vsvcTCPRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.TCPRoutes - modifiedObj, _, err := r.reconcileVirtualService(obj, vsvcRoutes, vsvcTLSRoutes, vsvcTCPRoutes, 10) - assert.Nil(t, err) - assert.NotNil(t, modifiedObj) - - httpRoutes := extractHttpRoutes(t, modifiedObj) - - // Assertions - assert.Equal(t, httpRoutes[0].Route[0].Destination.Host, "rollout-service") - assert.Equal(t, httpRoutes[0].Route[1].Destination.Host, "rollout-service") - if httpRoutes[0].Route[0].Destination.Subset == "stable" || httpRoutes[0].Route[1].Destination.Subset == "canary" { - assert.Equal(t, httpRoutes[0].Route[0].Weight, int64(90)) - assert.Equal(t, httpRoutes[0].Route[1].Weight, int64(10)) - } else { - assert.Equal(t, httpRoutes[0].Route[0].Weight, int64(10)) - assert.Equal(t, httpRoutes[0].Route[1].Weight, int64(90)) - } - - assert.Equal(t, httpRoutes[0].Route[2].Destination.Host, "additional-service") - assert.Equal(t, httpRoutes[0].Route[2].Destination.Subset, "stable-subset") - assert.Equal(t, httpRoutes[0].Route[2].Weight, int64(20)) -} - -func TestHttpReconcileInvalidMultipleDestRule(t *testing.T) { - ro := rolloutWithDestinationRule() - // set to invalid destination rule - ro.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule = &v1alpha1.IstioDestinationRule{ - Name: "invalid-destination-rule", - CanarySubsetName: "canary", - StableSubsetName: "stable", - } - - ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Name = "vsvc" - ro.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes = nil - - obj := unstructuredutil.StrToUnstructuredUnsafe(singleRouteSubsetMultipleDestRuleVsvc) - client := testutil.NewFakeDynamicClient(obj) - vsvcLister, druleLister := getIstioListers(client) - r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) - client.ClearActions() - - vsvcRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.Routes - vsvcTLSRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.TLSRoutes - vsvcTCPRoutes := r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.VirtualService.TCPRoutes - _, _, err := r.reconcileVirtualService(obj, vsvcRoutes, vsvcTLSRoutes, vsvcTCPRoutes, 10) - assert.Error(t, err, "expected destination rule not found") -} - func TestHttpReconcileHeaderRouteHostBased(t *testing.T) { ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"}) obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvc)