diff --git a/docs/getting-started/istio/index.md b/docs/getting-started/istio/index.md index 03d965069a..b87a58843f 100644 --- a/docs/getting-started/istio/index.md +++ b/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,14 @@ spec: - name: http-primary # Should match spec.strategy.canary.trafficRouting.istio.virtualService.routes route: - destination: - 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 - 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 +89,14 @@ spec: - localhost route: - destination: - 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 - 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: diff --git a/manifests/crds/rollout-crd.yaml b/manifests/crds/rollout-crd.yaml index cdc7d557ec..7972e4567b 100644 --- a/manifests/crds/rollout-crd.yaml +++ b/manifests/crds/rollout-crd.yaml @@ -530,6 +530,9 @@ spec: - name - stableSubsetName type: object + maxWeight: + format: int64 + type: integer virtualService: properties: name: diff --git a/manifests/install.yaml b/manifests/install.yaml index 1961544028..63002a254b 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -10228,6 +10228,9 @@ spec: - name - stableSubsetName type: object + maxWeight: + format: int64 + type: integer virtualService: properties: name: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index 428dfc0515..c9fbbc8fee 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -10228,6 +10228,9 @@ spec: - name - stableSubsetName type: object + maxWeight: + format: int64 + type: integer virtualService: properties: name: diff --git a/pkg/apiclient/rollout/rollout.swagger.json b/pkg/apiclient/rollout/rollout.swagger.json index 117b49073b..ee131f5d68 100644 --- a/pkg/apiclient/rollout/rollout.swagger.json +++ b/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" diff --git a/pkg/apis/rollouts/v1alpha1/types.go b/pkg/apis/rollouts/v1alpha1/types.go index 193c7855c5..0dbacf144f 100644 --- a/pkg/apis/rollouts/v1alpha1/types.go +++ b/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 diff --git a/rollout/trafficrouting/istio/istio.go b/rollout/trafficrouting/istio/istio.go index c6298a43c2..9d3b874025 100644 --- a/rollout/trafficrouting/istio/istio.go +++ b/rollout/trafficrouting/istio/istio.go @@ -101,11 +101,21 @@ func (patches virtualServicePatches) patchVirtualService(httpRoutes []interface{ return nil } +func getSanitizedMaxWeight(maxWeight int64) int64 { + // Ignore any invalid values for maxWeight by setting it to the default (100) + sanitizedMaxWeight := maxWeight + if sanitizedMaxWeight <= 0 || sanitizedMaxWeight >= 100 { + sanitizedMaxWeight = 100 + } + return sanitizedMaxWeight +} + func (r *Reconciler) generateVirtualServicePatches(httpRoutes []VirtualServiceHTTPRoute, tlsRoutes []VirtualServiceTLSRoute, desiredWeight int64) virtualServicePatches { canarySvc := r.rollout.Spec.Strategy.Canary.CanaryService stableSvc := r.rollout.Spec.Strategy.Canary.StableService canarySubset := "" stableSubset := "" + maxWeight := getSanitizedMaxWeight(r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.MaxWeight) if r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule != nil { canarySubset = r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.CanarySubsetName stableSubset = r.rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.StableSubsetName @@ -127,19 +137,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 +158,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 @@ -659,7 +669,7 @@ func ValidateHTTPRoutes(r *v1alpha1.Rollout, httpRoutes []VirtualServiceHTTPRout } for _, routeIndex := range routeIndexesToPatch { route := httpRoutes[routeIndex] - err := validateVirtualServiceRouteDestinations(route.Route, stableSvc, canarySvc, r.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule) + err := validateVirtualServiceRouteDestinations(route.Route, stableSvc, canarySvc, r.Spec.Strategy.Canary.TrafficRouting.Istio.MaxWeight, r.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule) if err != nil { return err } @@ -681,7 +691,7 @@ func ValidateTlsRoutes(r *v1alpha1.Rollout, tlsRoutes []VirtualServiceTLSRoute) } for _, routeIndex := range routeIndexesToPatch { route := tlsRoutes[routeIndex] - err := validateVirtualServiceRouteDestinations(route.Route, stableSvc, canarySvc, r.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule) + err := validateVirtualServiceRouteDestinations(route.Route, stableSvc, canarySvc, r.Spec.Strategy.Canary.TrafficRouting.Istio.MaxWeight, r.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule) if err != nil { return err } @@ -694,50 +704,59 @@ 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.") - } +func validateVirtualServiceRouteDestinations(hr []VirtualServiceRouteDestination, stableSvc, canarySvc string, maxWeight int64, dRule *v1alpha1.IstioDestinationRule) error { hasStableSvc := false hasCanarySvc := false hasStableSubset := false hasCanarySubset := false + + var fixedWeight int64 = 0 for _, r := range hr { host := getHost(r) - - if stableSvc != "" && host == stableSvc { - hasStableSvc = true - } - - if canarySvc != "" && host == canarySvc { - hasCanarySvc = true - } if dRule != nil { if dRule.StableSubsetName != "" && r.Destination.Subset == dRule.StableSubsetName { hasStableSubset = true + continue } if dRule.CanarySubsetName != "" && r.Destination.Subset == dRule.CanarySubsetName { hasCanarySubset = true + continue } + } else if stableSvc != "" && host == stableSvc { + hasStableSvc = true + continue + } else if canarySvc != "" && host == canarySvc { + hasCanarySvc = true + continue } + // This VS route is not a stable/canary route. + fixedWeight += r.Weight } - return validateDestinationRule(dRule, hasCanarySubset, hasStableSubset, hasCanarySvc, hasStableSvc, canarySvc, stableSvc) + + err := validateDestinationRule(dRule, hasCanarySubset, hasStableSubset, hasCanarySvc, hasStableSvc, canarySvc, stableSvc) + if err != nil { + return err + } else if 100-getSanitizedMaxWeight(maxWeight) != fixedWeight { + // Assert whether fixed wight is equal to the difference of 100 - maxWeight + return fmt.Errorf("VirtualService destination weights doesn't add upto 100.") + } + return nil } 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 diff --git a/rollout/trafficrouting/istio/istio_test.go b/rollout/trafficrouting/istio/istio_test.go index 96d1098640..cbd2dbfa50 100644 --- a/rollout/trafficrouting/istio/istio_test.go +++ b/rollout/trafficrouting/istio/istio_test.go @@ -23,7 +23,7 @@ import ( unstructuredutil "github.com/argoproj/argo-rollouts/utils/unstructured" ) -const RouteMissingBothDestinationsError = "Route does not have exactly two route destinations." +const RouteMissingCanaryServiceError = "Canary Service 'canary' not found in the route." const NoTlsRouteFoundError = "No matching TLS routes found in the defined Virtual Service." func getIstioListers(client dynamic.Interface) (dynamiclister.Lister, dynamiclister.Lister) { @@ -41,7 +41,7 @@ func getIstioListers(client dynamic.Interface) (dynamiclister.Lister, dynamiclis return vsvcLister, druleLister } -func rollout(stableSvc, canarySvc string, istioVirtualService *v1alpha1.IstioVirtualService) *v1alpha1.Rollout { +func rollout(stableSvc, canarySvc string, maxWeight int64, istioVirtualService *v1alpha1.IstioVirtualService) *v1alpha1.Rollout { return &v1alpha1.Rollout{ ObjectMeta: metav1.ObjectMeta{ Name: "rollout", @@ -55,6 +55,7 @@ func rollout(stableSvc, canarySvc string, istioVirtualService *v1alpha1.IstioVir TrafficRouting: &v1alpha1.RolloutTrafficRouting{ Istio: &v1alpha1.IstioTrafficRouting{ VirtualService: *istioVirtualService, + MaxWeight: maxWeight, }, }, }, @@ -63,29 +64,29 @@ func rollout(stableSvc, canarySvc string, istioVirtualService *v1alpha1.IstioVir } } -func rolloutWithHttpRoutes(stableSvc, canarySvc, vsvc string, httpRoutes []string) *v1alpha1.Rollout { +func rolloutWithHttpRoutes(stableSvc, canarySvc, vsvc string, maxWeight int64, httpRoutes []string) *v1alpha1.Rollout { istioVirtualService := &v1alpha1.IstioVirtualService{ Name: vsvc, Routes: httpRoutes, } - return rollout(stableSvc, canarySvc, istioVirtualService) + return rollout(stableSvc, canarySvc, maxWeight, istioVirtualService) } -func rolloutWithTlsRoutes(stableSvc, canarySvc, vsvc string, tlsRoutes []v1alpha1.TLSRoute) *v1alpha1.Rollout { +func rolloutWithTlsRoutes(stableSvc, canarySvc, vsvc string, maxWeight int64, tlsRoutes []v1alpha1.TLSRoute) *v1alpha1.Rollout { istioVirtualService := &v1alpha1.IstioVirtualService{ Name: vsvc, TLSRoutes: tlsRoutes, } - return rollout(stableSvc, canarySvc, istioVirtualService) + return rollout(stableSvc, canarySvc, maxWeight, istioVirtualService) } -func rolloutWithHttpAndTlsRoutes(stableSvc, canarySvc, vsvc string, httpRoutes []string, tlsRoutes []v1alpha1.TLSRoute) *v1alpha1.Rollout { +func rolloutWithHttpAndTlsRoutes(stableSvc, canarySvc, vsvc string, maxWeight int64, httpRoutes []string, tlsRoutes []v1alpha1.TLSRoute) *v1alpha1.Rollout { istioVirtualService := &v1alpha1.IstioVirtualService{ Name: vsvc, Routes: httpRoutes, TLSRoutes: tlsRoutes, } - return rollout(stableSvc, canarySvc, istioVirtualService) + return rollout(stableSvc, canarySvc, maxWeight, istioVirtualService) } func checkDestination(t *testing.T, destinations []VirtualServiceRouteDestination, svc string, expectWeight int) { @@ -113,15 +114,18 @@ spec: - name: primary route: - destination: - host: 'stable' - weight: 100 + host: stable + weight: 80 - destination: host: canary weight: 0 + - destination: + host: legacy + weight: 20 - name: secondary route: - destination: - host: 'stable' + host: stable weight: 100 - destination: host: canary @@ -142,8 +146,11 @@ spec: - port: 3000 route: - destination: - host: 'stable' - weight: 100 + host: stable + weight: 80 + - destination: + host: legacy + weight: 20 - destination: host: canary weight: 0 @@ -151,7 +158,7 @@ spec: - port: 3001 route: - destination: - host: 'stable' + host: stable weight: 100 - destination: host: canary @@ -218,15 +225,18 @@ spec: - name: primary route: - destination: - host: 'stable' - weight: 100 + host: stable + weight: 80 + - destination: + host: legacy + weight: 20 - destination: host: canary weight: 0 - name: secondary route: - destination: - host: 'stable' + host: stable weight: 100 - destination: host: canary @@ -236,11 +246,14 @@ spec: - port: 3000 route: - destination: - host: 'stable' - weight: 100 + host: stable + weight: 80 - destination: host: canary - weight: 0` + weight: 0 + - destination: + host: legacy + weight: 20` const regularMixedVsvcTwoTlsRoutes = `apiVersion: networking.istio.io/v1alpha3 kind: VirtualService @@ -256,8 +269,11 @@ spec: - name: primary route: - destination: - host: 'stable' - weight: 100 + host: stable + weight: 80 + - destination: + host: legacy + weight: 20 - destination: host: canary weight: 0 @@ -266,8 +282,11 @@ spec: - port: 3000 route: - destination: - host: 'stable' - weight: 100 + host: stable + weight: 80 + - destination: + host: legacy + weight: 20 - destination: host: canary weight: 0 @@ -445,7 +464,7 @@ func assertTlsRouteWeightChanges(t *testing.T, tlsRoute VirtualServiceTLSRoute, func TestHttpReconcileWeightsBaseCase(t *testing.T) { r := &Reconciler{ - rollout: rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"}), + rollout: rolloutWithHttpRoutes("stable", "canary", "vsvc", 80, []string{"primary"}), } // Test for both the HTTP VS & Mixed VS @@ -459,14 +478,14 @@ func TestHttpReconcileWeightsBaseCase(t *testing.T) { httpRoutes := extractHttpRoutes(t, modifiedObj) // Assertions - assertHttpRouteWeightChanges(t, httpRoutes[0], "primary", 10, 90) + assertHttpRouteWeightChanges(t, httpRoutes[0], "primary", 10, 70) assertHttpRouteWeightChanges(t, httpRoutes[1], "secondary", 0, 100) } } func TestTlsReconcileWeightsBaseCase(t *testing.T) { r := &Reconciler{ - rollout: rolloutWithTlsRoutes("stable", "canary", "vsvc", + rollout: rolloutWithTlsRoutes("stable", "canary", "vsvc", 80, []v1alpha1.TLSRoute{ { Port: 3000, @@ -486,7 +505,7 @@ func TestTlsReconcileWeightsBaseCase(t *testing.T) { tlsRoutes := extractTlsRoutes(t, modifiedObj) // Assestions - assertTlsRouteWeightChanges(t, tlsRoutes[0], nil, 3000, 30, 70) + assertTlsRouteWeightChanges(t, tlsRoutes[0], nil, 3000, 30, 50) assertTlsRouteWeightChanges(t, tlsRoutes[1], nil, 3001, 0, 100) } } @@ -494,7 +513,7 @@ func TestTlsReconcileWeightsBaseCase(t *testing.T) { func TestTlsSniReconcileWeightsBaseCase(t *testing.T) { snis := []string{"foo.bar.com", "bar.foo.com", "localhost"} r := &Reconciler{ - rollout: rolloutWithTlsRoutes("stable", "canary", "vsvc", + rollout: rolloutWithTlsRoutes("stable", "canary", "vsvc", 0, []v1alpha1.TLSRoute{ { SNIHosts: snis, @@ -519,7 +538,7 @@ func TestTlsSniReconcileWeightsBaseCase(t *testing.T) { func TestTlsPortAndSniReconcileWeightsBaseCase(t *testing.T) { snis := []string{"localhost"} r := &Reconciler{ - rollout: rolloutWithTlsRoutes("stable", "canary", "vsvc", + rollout: rolloutWithTlsRoutes("stable", "canary", "vsvc", 100, []v1alpha1.TLSRoute{ { Port: 3001, @@ -544,7 +563,7 @@ func TestTlsPortAndSniReconcileWeightsBaseCase(t *testing.T) { func TestReconcileWeightsBaseCase(t *testing.T) { r := &Reconciler{ - rollout: rolloutWithHttpAndTlsRoutes("stable", "canary", "vsvc", []string{"primary"}, + rollout: rolloutWithHttpAndTlsRoutes("stable", "canary", "vsvc", 0, []string{"primary"}, []v1alpha1.TLSRoute{ { Port: 3000, @@ -573,12 +592,23 @@ func TestReconcileWeightsBaseCase(t *testing.T) { } func TestReconcileUpdateVirtualService(t *testing.T) { - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"}) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 80, []string{"primary"}) AssertReconcileUpdateVirtualService(t, regularVsvc, ro) } +func TestReconcileUpdateVirtualServiceWeightMismatch(t *testing.T) { + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 70, []string{"primary"}) + obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvc) + client := testutil.NewFakeDynamicClient(obj) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(10) + assert.Equal(t, "VirtualService destination weights doesn't add upto 100.", err.Error()) +} + func TestTlsReconcileUpdateVirtualService(t *testing.T) { - ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", + ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", 80, []v1alpha1.TLSRoute{ { Port: 3000, @@ -588,6 +618,23 @@ func TestTlsReconcileUpdateVirtualService(t *testing.T) { AssertReconcileUpdateVirtualService(t, regularTlsVsvc, ro) } +func TestTlsReconcileUpdateVirtualServiceWeightMismatch(t *testing.T) { + ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", 60, + []v1alpha1.TLSRoute{ + { + Port: 3000, + }, + }, + ) + obj := unstructuredutil.StrToUnstructuredUnsafe(regularTlsVsvc) + client := testutil.NewFakeDynamicClient(obj) + vsvcLister, druleLister := getIstioListers(client) + r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) + client.ClearActions() + err := r.SetWeight(10) + assert.Equal(t, "VirtualService destination weights doesn't add upto 100.", err.Error()) +} + func AssertReconcileUpdateVirtualService(t *testing.T, vsvc string, ro *v1alpha1.Rollout) *dynamicfake.FakeDynamicClient { obj := unstructuredutil.StrToUnstructuredUnsafe(vsvc) client := testutil.NewFakeDynamicClient(obj) @@ -605,7 +652,7 @@ func AssertReconcileUpdateVirtualService(t *testing.T, vsvc string, ro *v1alpha1 func TestReconcileNoChanges(t *testing.T) { obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvc) client := testutil.NewFakeDynamicClient(obj) - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"}) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 80, []string{"primary"}) r := NewReconciler(ro, client, record.NewFakeEventRecorder(), nil, nil) err := r.SetWeight(0) assert.Nil(t, err) @@ -616,7 +663,7 @@ func TestReconcileNoChanges(t *testing.T) { func TestTlsReconcileNoChanges(t *testing.T) { obj := unstructuredutil.StrToUnstructuredUnsafe(regularTlsVsvc) client := testutil.NewFakeDynamicClient(obj) - ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", + ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", -1, []v1alpha1.TLSRoute{ { Port: 3001, @@ -633,7 +680,7 @@ func TestTlsReconcileNoChanges(t *testing.T) { func TestReconcileInvalidValidation(t *testing.T) { obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvc) client := testutil.NewFakeDynamicClient(obj) - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"route-not-found"}) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 100, []string{"route-not-found"}) vsvcLister, druleLister := getIstioListers(client) r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) client.ClearActions() @@ -644,7 +691,7 @@ func TestReconcileInvalidValidation(t *testing.T) { func TestTlsReconcileInvalidValidation(t *testing.T) { obj := unstructuredutil.StrToUnstructuredUnsafe(regularTlsVsvc) client := testutil.NewFakeDynamicClient(obj) - ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", + ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", 1000, []v1alpha1.TLSRoute{ { Port: 1001, @@ -660,7 +707,7 @@ func TestTlsReconcileInvalidValidation(t *testing.T) { func TestReconcileVirtualServiceNotFound(t *testing.T) { client := testutil.NewFakeDynamicClient() - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"}) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 0, []string{"primary"}) vsvcLister, druleLister := getIstioListers(client) r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) client.ClearActions() @@ -673,7 +720,7 @@ func TestReconcileVirtualServiceNotFound(t *testing.T) { func TestReconcileAmbiguousRoutes(t *testing.T) { obj := unstructuredutil.StrToUnstructuredUnsafe(regularVsvc) client := testutil.NewFakeDynamicClient(obj) - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", nil) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 80, nil) vsvcLister, druleLister := getIstioListers(client) r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) client.ClearActions() @@ -684,7 +731,7 @@ func TestReconcileAmbiguousRoutes(t *testing.T) { func TestTlsReconcileAmbiguousRoutes(t *testing.T) { obj := unstructuredutil.StrToUnstructuredUnsafe(regularTlsVsvc) client := testutil.NewFakeDynamicClient(obj) - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", nil) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 80, nil) vsvcLister, druleLister := getIstioListers(client) r := NewReconciler(ro, client, record.NewFakeEventRecorder(), vsvcLister, druleLister) client.ClearActions() @@ -694,7 +741,7 @@ func TestTlsReconcileAmbiguousRoutes(t *testing.T) { // TestReconcileInferredSingleRoute we can support case where we infer the only route in the VirtualService func TestHttpReconcileInferredSingleRoute(t *testing.T) { - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", nil) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", -100, nil) client := AssertReconcileUpdateVirtualService(t, singleRouteVsvc, ro) // Verify we actually made the correct change @@ -711,7 +758,7 @@ func TestHttpReconcileInferredSingleRoute(t *testing.T) { } func TestTlsReconcileInferredSingleRoute(t *testing.T) { - ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", nil) + ro := rolloutWithTlsRoutes("stable", "canary", "vsvc", -1, nil) client := AssertReconcileUpdateVirtualService(t, singleRouteTlsVsvc, ro) // Verify we actually made the correct change @@ -728,7 +775,7 @@ func TestTlsReconcileInferredSingleRoute(t *testing.T) { } func TestReconcileInferredSingleRoute(t *testing.T) { - ro := rolloutWithHttpAndTlsRoutes("stable", "canary", "vsvc", nil, nil) + ro := rolloutWithHttpAndTlsRoutes("stable", "canary", "vsvc", 100, nil, nil) client := AssertReconcileUpdateVirtualService(t, singleRouteMixedVsvc, ro) // Verify we actually made the correct change @@ -758,7 +805,7 @@ func TestReconcileInferredSingleRoute(t *testing.T) { func TestType(t *testing.T) { client := testutil.NewFakeDynamicClient() - ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", []string{"primary"}) + ro := rolloutWithHttpRoutes("stable", "canary", "vsvc", 1000, []string{"primary"}) r := NewReconciler(ro, client, record.NewFakeEventRecorder(), nil, nil) assert.Equal(t, Type, r.Type()) } @@ -831,7 +878,7 @@ func TestValidateHTTPRoutes(t *testing.T) { }} rollout := newRollout([]string{"test"}) err := ValidateHTTPRoutes(rollout, httpRoutes) - assert.Equal(t, fmt.Errorf(RouteMissingBothDestinationsError), err) + assert.Equal(t, fmt.Errorf(RouteMissingCanaryServiceError), err) httpRoutes[0].Route = []VirtualServiceRouteDestination{{ Destination: VirtualServiceDestination{ @@ -899,7 +946,7 @@ func TestValidateTLSRoutes(t *testing.T) { }, ) err = ValidateTlsRoutes(rollout, tlsRoutes) - assert.Equal(t, fmt.Errorf(RouteMissingBothDestinationsError), err) + assert.Equal(t, fmt.Errorf(RouteMissingCanaryServiceError), err) tlsRoutes[0].Route = []VirtualServiceRouteDestination{{ Destination: VirtualServiceDestination{ @@ -933,8 +980,8 @@ func TestValidateHosts(t *testing.T) { }, }}, } - err := validateVirtualServiceRouteDestinations(hr.Route, "stable", "canary", nil) - assert.Equal(t, fmt.Errorf(RouteMissingBothDestinationsError), err) + err := validateVirtualServiceRouteDestinations(hr.Route, "stable", "canary", 0, nil) + assert.Equal(t, fmt.Errorf(RouteMissingCanaryServiceError), err) hr.Route = []VirtualServiceRouteDestination{{ Destination: VirtualServiceDestination{ @@ -945,14 +992,14 @@ func TestValidateHosts(t *testing.T) { Host: "canary", }, }} - err = validateVirtualServiceRouteDestinations(hr.Route, "stable", "canary", nil) + err = validateVirtualServiceRouteDestinations(hr.Route, "stable", "canary", 0, nil) assert.Nil(t, err) - err = validateVirtualServiceRouteDestinations(hr.Route, "not-found-stable", "canary", nil) - assert.Equal(t, fmt.Errorf("Stable Service 'not-found-stable' not found in route"), err) + err = validateVirtualServiceRouteDestinations(hr.Route, "not-found-stable", "canary", -1, nil) + assert.Equal(t, fmt.Errorf("Stable Service 'not-found-stable' not found in the route."), err) - err = validateVirtualServiceRouteDestinations(hr.Route, "stable", "not-found-canary", nil) - assert.Equal(t, fmt.Errorf("Canary Service 'not-found-canary' not found in route"), err) + err = validateVirtualServiceRouteDestinations(hr.Route, "stable", "not-found-canary", 1000, nil) + assert.Equal(t, fmt.Errorf("Canary Service 'not-found-canary' not found in the route."), err) hr.Route = []VirtualServiceRouteDestination{{ Destination: VirtualServiceDestination{ @@ -963,7 +1010,7 @@ func TestValidateHosts(t *testing.T) { Host: "canary.namespace", }, }} - err = validateVirtualServiceRouteDestinations(hr.Route, "stable", "canary", nil) + err = validateVirtualServiceRouteDestinations(hr.Route, "stable", "canary", 100, nil) assert.Nil(t, err) } @@ -1016,14 +1063,14 @@ func TestValidateHTTPRoutesSubsets(t *testing.T) { rollout = rollout.DeepCopy() rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.StableSubsetName = "doesntexist" err := ValidateHTTPRoutes(rollout, httpRoutes) - assert.EqualError(t, err, "Stable DestinationRule subset 'doesntexist' not found in route") + assert.EqualError(t, err, "Stable DestinationRule subset 'doesntexist' not found in the route.") } { // the canary subset doesnt exist rollout = rollout.DeepCopy() rollout.Spec.Strategy.Canary.TrafficRouting.Istio.DestinationRule.CanarySubsetName = "doesntexist" err := ValidateHTTPRoutes(rollout, httpRoutes) - assert.EqualError(t, err, "Canary DestinationRule subset 'doesntexist' not found in route") + assert.EqualError(t, err, "Canary DestinationRule subset 'doesntexist' not found in the route.") } }