Skip to content

Commit

Permalink
fix: Revert "fix: istio destionationrule subsets enforcement (#3126)" (
Browse files Browse the repository at this point in the history
…#3147)

Revert "fix: istio destionationrule subsets enforcement (#3126)"

This reverts commit 04e1119.

Signed-off-by: zachaller <[email protected]>
  • Loading branch information
zachaller committed Nov 2, 2023
1 parent 3498a20 commit 2184a07
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 128 deletions.
1 change: 0 additions & 1 deletion USERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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)
Expand Down
17 changes: 0 additions & 17 deletions rollout/trafficrouting/istio/istio.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
110 changes: 0 additions & 110 deletions rollout/trafficrouting/istio/istio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 2184a07

Please sign in to comment.