From 86c32e4941af7928a745007b79d9ec3154b2381e Mon Sep 17 00:00:00 2001 From: yunbo Date: Tue, 9 Apr 2024 19:14:30 +0800 Subject: [PATCH 1/2] generate istio destinationRule subset from pod-template-hash && add fields to allow user set canary/stable subset name Signed-off-by: yunbo --- api/v1alpha1/conversion.go | 1 + api/v1alpha1/trafficrouting_types.go | 6 + api/v1beta1/trafficrouting.go | 9 + .../bases/rollouts.kruise.io_rollouts.yaml | 10 ++ .../rollouts.kruise.io_trafficroutings.yaml | 5 + .../convert_test_case_to_lua_object.go | 26 +-- .../traffic_routing_with_a_match.yaml | 11 +- .../DestinationRule/trafficRouting.lua | 51 +++++- .../traffic_routing_with_a_match.yaml | 3 + .../traffic_routing_with_matches.yaml | 3 + .../testdata/traffic_routing_with_weight.yaml | 3 + .../VirtualService/trafficRouting.lua | 4 +- pkg/controller/rollout/rollout_canary.go | 10 +- pkg/trafficrouting/manager.go | 18 +- pkg/trafficrouting/manager_test.go | 165 ++++++++++++++---- .../custom_network_provider.go | 49 +++++- .../custom_network_provider_test.go | 44 +++-- .../DestinationRule/trafficRouting.lua | 51 +++++- .../VirtualService/trafficRouting.lua | 4 +- 19 files changed, 375 insertions(+), 98 deletions(-) diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go index 5164c1ca..8923a2cd 100644 --- a/api/v1alpha1/conversion.go +++ b/api/v1alpha1/conversion.go @@ -126,6 +126,7 @@ func (src *Rollout) ConvertTo(dst conversion.Hub) error { func ConversionToV1beta1TrafficRoutingRef(src TrafficRoutingRef) (dst v1beta1.TrafficRoutingRef) { dst.Service = src.Service dst.GracePeriodSeconds = src.GracePeriodSeconds + dst.AdditionalParams = src.AdditionalParams if src.Ingress != nil { dst.Ingress = &v1beta1.IngressTrafficRouting{ ClassType: src.Ingress.ClassType, diff --git a/api/v1alpha1/trafficrouting_types.go b/api/v1alpha1/trafficrouting_types.go index ba5fbf10..f69138cb 100644 --- a/api/v1alpha1/trafficrouting_types.go +++ b/api/v1alpha1/trafficrouting_types.go @@ -23,6 +23,8 @@ import ( const ( ProgressingRolloutFinalizerPrefix = "progressing.rollouts.kruise.io" + IstioStableSubsetName = "istio.destinationRule.stableSubsetName" + IstioCanarySubsetName = "istio.destinationRule.canarySubsetName" ) // TrafficRoutingRef hosts all the different configuration for supported service meshes to enable more fine-grained traffic routing @@ -38,6 +40,10 @@ type TrafficRoutingRef struct { Gateway *GatewayTrafficRouting `json:"gateway,omitempty"` // CustomNetworkRefs hold a list of custom providers to route traffic CustomNetworkRefs []CustomNetworkRef `json:"customNetworkRefs,omitempty"` + // vaild keys: + // + IstioStableSubsetName + // + IstioCanarySubsetName + AdditionalParams map[string]string `json:"additionalParams,omitempty"` } // IngressTrafficRouting configuration for ingress controller to control traffic routing diff --git a/api/v1beta1/trafficrouting.go b/api/v1beta1/trafficrouting.go index 3cd2c94b..aa9ad0e7 100644 --- a/api/v1beta1/trafficrouting.go +++ b/api/v1beta1/trafficrouting.go @@ -16,6 +16,11 @@ limitations under the License. package v1beta1 +const ( + IstioStableSubsetName = "istio.destinationRule.stableSubsetName" + IstioCanarySubsetName = "istio.destinationRule.canarySubsetName" +) + // TrafficRoutingRef hosts all the different configuration for supported service meshes to enable more fine-grained traffic routing type TrafficRoutingRef struct { // Service holds the name of a service which selects pods with stable version and don't select any pods with canary version. @@ -29,6 +34,10 @@ type TrafficRoutingRef struct { Gateway *GatewayTrafficRouting `json:"gateway,omitempty"` // CustomNetworkRefs hold a list of custom providers to route traffic CustomNetworkRefs []ObjectRef `json:"customNetworkRefs,omitempty"` + // vaild keys: + // + IstioStableSubsetName + // + IstioCanarySubsetName + AdditionalParams map[string]string `json:"additionalParams,omitempty"` } // IngressTrafficRouting configuration for ingress controller to control traffic routing diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/config/crd/bases/rollouts.kruise.io_rollouts.yaml index 62c072a6..939a59f9 100644 --- a/config/crd/bases/rollouts.kruise.io_rollouts.yaml +++ b/config/crd/bases/rollouts.kruise.io_rollouts.yaml @@ -344,6 +344,11 @@ spec: for supported service meshes to enable more fine-grained traffic routing properties: + additionalParams: + additionalProperties: + type: string + description: 'vaild keys:' + type: object customNetworkRefs: description: CustomNetworkRefs hold a list of custom providers to route traffic @@ -834,6 +839,11 @@ spec: for supported service meshes to enable more fine-grained traffic routing properties: + additionalParams: + additionalProperties: + type: string + description: 'vaild keys:' + type: object customNetworkRefs: description: CustomNetworkRefs hold a list of custom providers to route traffic diff --git a/config/crd/bases/rollouts.kruise.io_trafficroutings.yaml b/config/crd/bases/rollouts.kruise.io_trafficroutings.yaml index a3d0ad39..431b6990 100644 --- a/config/crd/bases/rollouts.kruise.io_trafficroutings.yaml +++ b/config/crd/bases/rollouts.kruise.io_trafficroutings.yaml @@ -54,6 +54,11 @@ spec: for supported service meshes to enable more fine-grained traffic routing properties: + additionalParams: + additionalProperties: + type: string + description: 'vaild keys:' + type: object customNetworkRefs: description: CustomNetworkRefs hold a list of custom providers to route traffic diff --git a/lua_configuration/convert_test_case_to_lua_object.go b/lua_configuration/convert_test_case_to_lua_object.go index 686bea18..6000ea0a 100644 --- a/lua_configuration/convert_test_case_to_lua_object.go +++ b/lua_configuration/convert_test_case_to_lua_object.go @@ -100,11 +100,14 @@ func objectToTable(path string) error { Annotations: testCase.Original.GetAnnotations(), Spec: testCase.Original.Object["spec"], }, - Matches: step.TrafficRoutingStrategy.Matches, - CanaryWeight: *weight, - StableWeight: 100 - *weight, - CanaryService: canaryService, - StableService: stableService, + Matches: step.TrafficRoutingStrategy.Matches, + CanaryWeight: *weight, + StableWeight: 100 - *weight, + CanaryService: canaryService, + StableService: stableService, + StableRevision: "podtemplatehash-v1", + CanaryRevision: "podtemplatehash-v2", + RevisionLabelKey: "pod-template-hash", } uList[fmt.Sprintf("step_%d", i)] = data } @@ -128,11 +131,14 @@ func objectToTable(path string) error { Annotations: testCase.Original.GetAnnotations(), Spec: testCase.Original.Object["spec"], }, - Matches: matches, - CanaryWeight: *weight, - StableWeight: 100 - *weight, - CanaryService: canaryService, - StableService: stableService, + Matches: matches, + CanaryWeight: *weight, + StableWeight: 100 - *weight, + CanaryService: canaryService, + StableService: stableService, + StableRevision: "podtemplatehash-v1", + CanaryRevision: "podtemplatehash-v2", + RevisionLabelKey: "pod-template-hash", } uList["steps_0"] = data } else { diff --git a/lua_configuration/networking.istio.io/DestinationRule/testdata/traffic_routing_with_a_match.yaml b/lua_configuration/networking.istio.io/DestinationRule/testdata/traffic_routing_with_a_match.yaml index 3d5e3be4..b8f81b37 100644 --- a/lua_configuration/networking.istio.io/DestinationRule/testdata/traffic_routing_with_a_match.yaml +++ b/lua_configuration/networking.istio.io/DestinationRule/testdata/traffic_routing_with_a_match.yaml @@ -16,6 +16,9 @@ trafficRouting: - apiVersion: networking.istio.io/v1beta1 kind: DestinationRule name: ds-demo + additionalParams: + istio.destinationRule.stableSubsetName: "version-base" + istio.destinationRule.canarySubsetName: "canary" original: apiVersion: networking.istio.io/v1beta1 kind: DestinationRule @@ -27,9 +30,7 @@ original: loadBalancer: simple: ROUND_ROBIN subsets: - - labels: - version: base - name: version-base + - name: version-base expected: - apiVersion: networking.istio.io/v1beta1 kind: DestinationRule @@ -42,8 +43,8 @@ expected: simple: ROUND_ROBIN subsets: - labels: - version: base + pod-template-hash: "podtemplatehash-v1" name: version-base - labels: - istio.service.tag: gray + pod-template-hash: "podtemplatehash-v2" name: canary \ No newline at end of file diff --git a/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua b/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua index 3df3aace..1d45cb6e 100644 --- a/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua +++ b/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua @@ -1,8 +1,51 @@ local spec = obj.data.spec +local podLabelKey = obj.revisionLabelKey + +if spec.subsets == nil then + spec.subsets = {} +end + +-- selector lables might come from pod-template-hash and patchPodTemplateMetadata +-- now we only support pod-template-hash +local stableLabels = {} +if obj.stableRevision ~= nil and obj.stableRevision ~= "" then + stableLabels[podLabelKey] = obj.stableRevision +end +local canaryLabels = {} +if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canaryLabels[podLabelKey] = obj.canaryRevision +end +local StableNameAlreadyExist = false + +-- if stableName already exists, just appened the lables +for _, subset in ipairs(spec.subsets) do + if subset.name == obj.stableName then + StableNameAlreadyExist = true + if next(stableLabels) ~= nil then + subset.labels = subset.labels or {} + for key, value in pairs(stableLabels) do + subset.labels[key] = value + end + end + end +end +-- if stableName doesn't exist, create it and its labels +if not StableNameAlreadyExist then + local stable = {} + stable.name = obj.stableName + if next(stableLabels) ~= nil then + stable.labels = stableLabels + end + table.insert(spec.subsets, stable) +end + +-- Aussue the canaryName never exist, create it and its labels local canary = {} -canary.labels = {} -canary.name = "canary" -local podLabelKey = "istio.service.tag" -canary.labels[podLabelKey] = "gray" +canary.name = obj.canaryName +if next(canaryLabels) ~= nil then + canary.labels = canaryLabels +end table.insert(spec.subsets, canary) + + return obj.data \ No newline at end of file diff --git a/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_a_match.yaml b/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_a_match.yaml index cd5b99cd..d5698c32 100644 --- a/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_a_match.yaml +++ b/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_a_match.yaml @@ -19,6 +19,9 @@ trafficRouting: - apiVersion: networking.istio.io/v1alpha3 kind: VirtualService name: vs-demo + additionalParams: + istio.destinationRule.stableSubsetName: "base" + istio.destinationRule.canarySubsetName: "canary" original: apiVersion: networking.istio.io/v1alpha3 kind: VirtualService diff --git a/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_matches.yaml b/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_matches.yaml index 26c7d7db..0254c4db 100644 --- a/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_matches.yaml +++ b/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_matches.yaml @@ -20,6 +20,9 @@ trafficRouting: - apiVersion: networking.istio.io/v1alpha3 kind: VirtualService name: vs-demo + additionalParams: + istio.destinationRule.stableSubsetName: "base" + istio.destinationRule.canarySubsetName: "canary" original: apiVersion: networking.istio.io/v1alpha3 kind: VirtualService diff --git a/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_weight.yaml b/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_weight.yaml index 9ab4bbb8..fb9b4e7c 100644 --- a/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_weight.yaml +++ b/lua_configuration/networking.istio.io/VirtualService/testdata/traffic_routing_with_weight.yaml @@ -12,6 +12,9 @@ trafficRouting: - apiVersion: networking.istio.io/v1alpha3 kind: VirtualService name: vs-demo + additionalParams: + istio.destinationRule.stableSubsetName: "base" + istio.destinationRule.canarySubsetName: "canary" original: apiVersion: networking.istio.io/v1alpha3 kind: VirtualService diff --git a/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua b/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua index 1581c794..aae945f9 100644 --- a/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua +++ b/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua @@ -75,7 +75,7 @@ function GenerateRoutesWithMatches(spec, matches, stableService, canaryService) -- stableService == canaryService indicates DestinationRule exists and subset is set to be canary by default if stableService == canaryService then route.route[1].destination.host = stableService - route.route[1].destination.subset = "canary" + route.route[1].destination.subset = obj.canaryName else route.route[1].destination.host = canaryService end @@ -99,7 +99,7 @@ function GenerateRoutes(spec, stableService, canaryService, stableWeight, canary canary = { destination = { host = stableService, - subset = "canary", + subset = obj.canaryName, }, weight = canaryWeight, } diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index 41ed5f90..f9aeb13b 100644 --- a/pkg/controller/rollout/rollout_canary.go +++ b/pkg/controller/rollout/rollout_canary.go @@ -250,14 +250,14 @@ func (m *canaryReleaseManager) doCanaryFinalising(c *RolloutContext) (bool, erro if err != nil || !done { return done, err } - // 3. set workload.pause=false; set workload.partition=0 - done, err = m.finalizingBatchRelease(c) + // 3. modify network api(ingress or gateway api) configuration, and route 100% traffic to stable pods. + done, err = m.trafficRoutingManager.FinalisingTrafficRouting(tr, false) + c.NewStatus.CanaryStatus.LastUpdateTime = tr.LastUpdateTime if err != nil || !done { return done, err } - // 4. modify network api(ingress or gateway api) configuration, and route 100% traffic to stable pods. - done, err = m.trafficRoutingManager.FinalisingTrafficRouting(tr, false) - c.NewStatus.CanaryStatus.LastUpdateTime = tr.LastUpdateTime + // 4. set workload.pause=false; set workload.partition=0 + done, err = m.finalizingBatchRelease(c) if err != nil || !done { return done, err } diff --git a/pkg/trafficrouting/manager.go b/pkg/trafficrouting/manager.go index ad8bc57a..1e5cd04e 100644 --- a/pkg/trafficrouting/manager.go +++ b/pkg/trafficrouting/manager.go @@ -278,14 +278,16 @@ func newNetworkProvider(c client.Client, con *TrafficRoutingContext, sService, c trafficRouting := con.ObjectRef[0] if trafficRouting.CustomNetworkRefs != nil { return custom.NewCustomController(c, custom.Config{ - Key: con.Key, - RolloutNs: con.Namespace, - CanaryService: cService, - StableService: sService, - TrafficConf: trafficRouting.CustomNetworkRefs, - OwnerRef: con.OwnerRef, - //only set for CustomController, never work for Ingress and Gateway - DisableGenerateCanaryService: con.DisableGenerateCanaryService, + Key: con.Key, + RolloutNs: con.Namespace, + CanaryService: cService, + StableService: sService, + TrafficConf: trafficRouting.CustomNetworkRefs, + OwnerRef: con.OwnerRef, + AdditionalParams: trafficRouting.AdditionalParams, + RevisionLabelKey: con.RevisionLabelKey, + StableRevision: con.StableRevision, + CanaryRevision: con.CanaryRevision, }) } if trafficRouting.Ingress != nil { diff --git a/pkg/trafficrouting/manager_test.go b/pkg/trafficrouting/manager_test.go index 366585e1..a07e9b7c 100644 --- a/pkg/trafficrouting/manager_test.go +++ b/pkg/trafficrouting/manager_test.go @@ -319,12 +319,12 @@ var ( Data: map[string]string{ "lua.traffic.routing.VirtualService.networking.istio.io": ` spec = obj.data.spec - + if obj.canaryWeight == -1 then obj.canaryWeight = 100 obj.stableWeight = 0 end - + function GetHost(destination) local host = destination.destination.host dot_position = string.find(host, ".", 1, true) @@ -333,7 +333,7 @@ var ( end return host end - + -- find routes of VirtualService with stableService function GetRulesToPatch(spec, stableService, protocol) local matchedRoutes = {} @@ -351,7 +351,7 @@ var ( end return matchedRoutes end - + function CalculateWeight(route, stableWeight, n) local weight if (route.weight) then @@ -361,7 +361,7 @@ var ( end return weight end - + -- generate routes with matches, insert a rule before other rules, only support http headers, cookies etc. function GenerateRoutesWithMatches(spec, matches, stableService, canaryService) for _, match in ipairs(matches) do @@ -395,14 +395,14 @@ var ( -- stableService == canaryService indicates DestinationRule exists and subset is set to be canary by default if stableService == canaryService then route.route[1].destination.host = stableService - route.route[1].destination.subset = "canary" + route.route[1].destination.subset = obj.canaryName else route.route[1].destination.host = canaryService end table.insert(spec.http, 1, route) end end - + -- generate routes without matches, change every rule whose host is stableService function GenerateRoutes(spec, stableService, canaryService, stableWeight, canaryWeight, protocol) local matchedRules = GetRulesToPatch(spec, stableService, protocol) @@ -419,12 +419,12 @@ var ( canary = { destination = { host = stableService, - subset = "canary", + subset = obj.canaryName, }, weight = canaryWeight, } end - + -- incase there are multiple versions traffic already, do a for-loop for _, route in ipairs(rule.route) do -- update stable service weight @@ -433,7 +433,7 @@ var ( table.insert(rule.route, canary) end end - + if (obj.matches and next(obj.matches) ~= nil) then GenerateRoutesWithMatches(spec, obj.matches, obj.stableService, obj.canaryService) @@ -443,22 +443,66 @@ var ( GenerateRoutes(spec, obj.stableService, obj.canaryService, obj.stableWeight, obj.canaryWeight, "tls") end return obj.data + `, "lua.traffic.routing.DestinationRule.networking.istio.io": ` local spec = obj.data.spec + local podLabelKey = obj.revisionLabelKey + + if spec.subsets == nil then + spec.subsets = {} + end + + -- selector lables might come from pod-template-hash and patchPodTemplateMetadata + -- now we only support pod-template-hash + local stableLabels = {} + if obj.stableRevision ~= nil and obj.stableRevision ~= "" then + stableLabels[podLabelKey] = obj.stableRevision + end + local canaryLabels = {} + if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canaryLabels[podLabelKey] = obj.canaryRevision + end + local StableNameAlreadyExist = false + + -- if stableName already exists, just appened the lables + for _, subset in ipairs(spec.subsets) do + if subset.name == obj.stableName then + StableNameAlreadyExist = true + if next(stableLabels) ~= nil then + subset.labels = subset.labels or {} + for key, value in pairs(stableLabels) do + subset.labels[key] = value + end + end + end + end + -- if stableName doesn't exist, create it and its labels + if not StableNameAlreadyExist then + local stable = {} + stable.name = obj.stableName + if next(stableLabels) ~= nil then + stable.labels = stableLabels + end + table.insert(spec.subsets, stable) + end + + -- Aussue the canaryName never exist, create it and its labels local canary = {} - canary.labels = {} - canary.name = "canary" - local podLabelKey = "istio.service.tag" - canary.labels[podLabelKey] = "gray" + canary.name = obj.canaryName + if next(canaryLabels) ~= nil then + canary.labels = canaryLabels + end table.insert(spec.subsets, canary) + + return obj.data `, }, } - virtualServiceDemo = ` + virtualServiceDemo1 = ` { "apiVersion": "networking.istio.io/v1alpha3", "kind": "VirtualService", @@ -486,7 +530,54 @@ var ( } } ` - destinationRuleDemo = ` + destinationRuleDemo1 = ` + { + "apiVersion": "networking.istio.io/v1alpha3", + "kind": "DestinationRule", + "metadata": { + "name": "dr-demo" + }, + "spec": { + "host": "echoserver", + "trafficPolicy": { + "loadBalancer": { + "simple": "ROUND_ROBIN" + } + } + } + } + ` + + virtualServiceDemo2 = ` + { + "apiVersion": "networking.istio.io/v1alpha3", + "kind": "VirtualService", + "metadata": { + "name": "echoserver", + "annotations": { + "virtual": "test" + } + }, + "spec": { + "hosts": [ + "echoserver.example.com" + ], + "http": [ + { + "route": [ + { + "destination": { + "host": "echoserver", + "subset": "hello" + } + } + ] + } + ] + } + } + ` + destinationRuleDemo2 = ` { "apiVersion": "networking.istio.io/v1alpha3", "kind": "DestinationRule", @@ -497,10 +588,7 @@ var ( "host": "echoserver", "subsets": [ { - "labels": { - "version": "base" - }, - "name": "version-base" + "name":"hello" } ], "trafficPolicy": { @@ -757,12 +845,12 @@ func TestDoTrafficRoutingWithIstio(t *testing.T) { getUnstructureds: func() []*unstructured.Unstructured { objects := make([]*unstructured.Unstructured, 0) u := &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(virtualServiceDemo)) + _ = u.UnmarshalJSON([]byte(virtualServiceDemo1)) u.SetAPIVersion("networking.istio.io/v1alpha3") objects = append(objects, u) u = &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) + _ = u.UnmarshalJSON([]byte(destinationRuleDemo1)) u.SetAPIVersion("networking.istio.io/v1alpha3") objects = append(objects, u) return objects @@ -770,12 +858,17 @@ func TestDoTrafficRoutingWithIstio(t *testing.T) { getRollout: func() (*v1beta1.Rollout, *util.Workload) { obj := demoIstioRollout.DeepCopy() obj.Status.CanaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now().Add(-10 * time.Second)} + obj.Spec.Strategy.Canary.TrafficRoutings[0].AdditionalParams = map[string]string{ + // map is empty, thus the subset names will be default value: stable and canary + // v1beta1.IstioStableSubsetName: "version-base", + // v1beta1.IstioCanarySubsetName: "canary", + } return obj, &util.Workload{RevisionLabelKey: apps.DefaultDeploymentUniqueLabelKey} }, expectUnstructureds: func() []*unstructured.Unstructured { objects := make([]*unstructured.Unstructured, 0) u := &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(virtualServiceDemo)) + _ = u.UnmarshalJSON([]byte(virtualServiceDemo1)) annotations := map[string]string{ OriginalSpecAnnotation: `{"spec":{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]},"annotations":{"virtual":"test"}}`, "virtual": "test", @@ -788,12 +881,12 @@ func TestDoTrafficRoutingWithIstio(t *testing.T) { objects = append(objects, u) u = &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) + _ = u.UnmarshalJSON([]byte(destinationRuleDemo1)) annotations = map[string]string{ - OriginalSpecAnnotation: `{"spec":{"host":"echoserver","subsets":[{"labels":{"version":"base"},"name":"version-base"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, + OriginalSpecAnnotation: `{"spec":{"host":"echoserver","trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, } u.SetAnnotations(annotations) - specStr = `{"host":"echoserver","subsets":[{"labels":{"version":"base"},"name":"version-base"},{"labels":{"istio.service.tag":"gray"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` + specStr = `{"host":"echoserver","subsets":[{"labels":{"pod-template-hash":"podtemplatehash-v1"},"name":"stable"},{"labels":{"pod-template-hash":"podtemplatehash-v2"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` _ = json.Unmarshal([]byte(specStr), &spec) u.Object["spec"] = spec objects = append(objects, u) @@ -815,12 +908,12 @@ func TestDoTrafficRoutingWithIstio(t *testing.T) { getUnstructureds: func() []*unstructured.Unstructured { objects := make([]*unstructured.Unstructured, 0) u := &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(virtualServiceDemo)) + _ = u.UnmarshalJSON([]byte(virtualServiceDemo2)) u.SetAPIVersion("networking.istio.io/v1alpha3") objects = append(objects, u) u = &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) + _ = u.UnmarshalJSON([]byte(destinationRuleDemo2)) u.SetAPIVersion("networking.istio.io/v1alpha3") objects = append(objects, u) return objects @@ -830,30 +923,34 @@ func TestDoTrafficRoutingWithIstio(t *testing.T) { // set DisableGenerateCanaryService as true obj.Spec.Strategy.Canary.DisableGenerateCanaryService = true obj.Status.CanaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now().Add(-10 * time.Second)} + obj.Spec.Strategy.Canary.TrafficRoutings[0].AdditionalParams = map[string]string{ + v1beta1.IstioStableSubsetName: "hello", + v1beta1.IstioCanarySubsetName: "world", + } return obj, &util.Workload{RevisionLabelKey: apps.DefaultDeploymentUniqueLabelKey} }, expectUnstructureds: func() []*unstructured.Unstructured { objects := make([]*unstructured.Unstructured, 0) u := &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(virtualServiceDemo)) + _ = u.UnmarshalJSON([]byte(virtualServiceDemo2)) annotations := map[string]string{ - OriginalSpecAnnotation: `{"spec":{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]},"annotations":{"virtual":"test"}}`, + OriginalSpecAnnotation: `{"spec":{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver","subset":"hello"}}]}]},"annotations":{"virtual":"test"}}`, "virtual": "test", } u.SetAnnotations(annotations) - specStr := `{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver"},"weight":95},{"destination":{"host":"echoserver","subset":"canary"},"weight":5}]}]}` + specStr := `{"hosts":["echoserver.example.com"],"http":[{"route":[{"destination":{"host":"echoserver","subset":"hello"},"weight":95},{"destination":{"host":"echoserver","subset":"world"},"weight":5}]}]}` var spec interface{} _ = json.Unmarshal([]byte(specStr), &spec) u.Object["spec"] = spec objects = append(objects, u) u = &unstructured.Unstructured{} - _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) + _ = u.UnmarshalJSON([]byte(destinationRuleDemo2)) annotations = map[string]string{ - OriginalSpecAnnotation: `{"spec":{"host":"echoserver","subsets":[{"labels":{"version":"base"},"name":"version-base"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, + OriginalSpecAnnotation: `{"spec":{"host":"echoserver","subsets":[{"name":"hello"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, } u.SetAnnotations(annotations) - specStr = `{"host":"echoserver","subsets":[{"labels":{"version":"base"},"name":"version-base"},{"labels":{"istio.service.tag":"gray"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` + specStr = `{"host":"echoserver","subsets":[{"labels":{"pod-template-hash":"podtemplatehash-v1"},"name":"hello"},{"labels":{"pod-template-hash":"podtemplatehash-v2"},"name":"world"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` _ = json.Unmarshal([]byte(specStr), &spec) u.Object["spec"] = spec objects = append(objects, u) diff --git a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go index 5778e4a3..59f4b2d9 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go +++ b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go @@ -53,6 +53,16 @@ type LuaData struct { Matches []v1beta1.HttpRouteMatch CanaryService string StableService string + // workload.RevisionLabelKey + RevisionLabelKey string + // status.CanaryStatus.StableRevision + StableRevision string + // status.CanaryStatus.PodTemplateHash + CanaryRevision string + // specify the name of subset used as stable subset, default 'stable'; if not found, create a subset named stable + StableName string + // sepcify the name of canary subset that will be created, default 'canary' + CanaryName string } type Data struct { Spec interface{} `json:"spec,omitempty"` @@ -72,9 +82,16 @@ type Config struct { CanaryService string StableService string // network providers need to be created - TrafficConf []v1beta1.ObjectRef - OwnerRef metav1.OwnerReference - DisableGenerateCanaryService bool + TrafficConf []v1beta1.ObjectRef + OwnerRef metav1.OwnerReference + // DisableGenerateCanaryService bool + AdditionalParams map[string]string + // workload.RevisionLabelKey + RevisionLabelKey string + // status.CanaryStatus.StableRevision + StableRevision string + // status.CanaryStatus.PodTemplateHash + CanaryRevision string } func NewCustomController(client client.Client, conf Config) (network.NetworkProvider, error) { @@ -268,13 +285,27 @@ func (r *customController) executeLuaForCanary(spec Data, strategy *v1beta1.Traf // so we need to pass weight=-1 to indicate the case where weight is nil. weight = utilpointer.Int32(-1) } + // default value + stableSubsetName, ok := r.conf.AdditionalParams[v1beta1.IstioStableSubsetName] + if !ok { + stableSubsetName = "stable" //default value + } + canarySubsetName, ok := r.conf.AdditionalParams[v1beta1.IstioCanarySubsetName] + if !ok { + canarySubsetName = "canary" //default value + } data := &LuaData{ - Data: spec, - CanaryWeight: *weight, - StableWeight: 100 - *weight, - Matches: matches, - CanaryService: r.conf.CanaryService, - StableService: r.conf.StableService, + Data: spec, + CanaryWeight: *weight, + StableWeight: 100 - *weight, + Matches: matches, + CanaryService: r.conf.CanaryService, + StableService: r.conf.StableService, + RevisionLabelKey: r.conf.RevisionLabelKey, + StableRevision: r.conf.StableRevision, + CanaryRevision: r.conf.CanaryRevision, + StableName: stableSubsetName, + CanaryName: canarySubsetName, } unObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(data) diff --git a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go index f263c177..bf54c4d7 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go +++ b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider_test.go @@ -89,9 +89,6 @@ var ( "host": "mockb", "subsets": [ { - "labels": { - "version": "base" - }, "name": "version-base" } ], @@ -281,6 +278,13 @@ func TestEnsureRoutes(t *testing.T) { Name: "dr-demo", }, }, + AdditionalParams: map[string]string{ + v1beta1.IstioStableSubsetName: "version-base", + v1beta1.IstioCanarySubsetName: "canary", + }, + StableRevision: "podtemplatehash-v1", + CanaryRevision: "podtemplatehash-v2", + RevisionLabelKey: "pod-template-hash", } }, expectUnstructureds: func() []*unstructured.Unstructured { @@ -301,10 +305,10 @@ func TestEnsureRoutes(t *testing.T) { u = &unstructured.Unstructured{} _ = u.UnmarshalJSON([]byte(destinationRuleDemo)) annotations = map[string]string{ - OriginalSpecAnnotation: `{"spec":{"host":"mockb","subsets":[{"labels":{"version":"base"},"name":"version-base"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, + OriginalSpecAnnotation: `{"spec":{"host":"mockb","subsets":[{"name":"version-base"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}`, } u.SetAnnotations(annotations) - specStr = `{"host":"mockb","subsets":[{"labels":{"version":"base"},"name":"version-base"},{"labels":{"istio.service.tag":"gray"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` + specStr = `{"host":"mockb","subsets":[{"labels":{"pod-template-hash":"podtemplatehash-v1"},"name":"version-base"},{"labels":{"pod-template-hash":"podtemplatehash-v2"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` _ = json.Unmarshal([]byte(specStr), &spec) u.Object["spec"] = spec objects = append(objects, u) @@ -540,11 +544,16 @@ func TestLuaScript(t *testing.T) { Annotations: testCase.Original.GetAnnotations(), Spec: testCase.Original.Object["spec"], }, - Matches: step.TrafficRoutingStrategy.Matches, - CanaryWeight: *weight, - StableWeight: 100 - *weight, - CanaryService: canaryService, - StableService: stableService, + Matches: step.TrafficRoutingStrategy.Matches, + CanaryWeight: *weight, + StableWeight: 100 - *weight, + CanaryService: canaryService, + StableService: stableService, + StableRevision: "podtemplatehash-v1", + CanaryRevision: "podtemplatehash-v2", + RevisionLabelKey: "pod-template-hash", + StableName: rollout.Spec.Strategy.Canary.TrafficRoutings[0].AdditionalParams[v1beta1.IstioStableSubsetName], + CanaryName: rollout.Spec.Strategy.Canary.TrafficRoutings[0].AdditionalParams[v1beta1.IstioCanarySubsetName], } nSpec, err := executeLua(data, script) if err != nil { @@ -580,11 +589,16 @@ func TestLuaScript(t *testing.T) { Annotations: testCase.Original.GetAnnotations(), Spec: testCase.Original.Object["spec"], }, - Matches: matches, - CanaryWeight: *weight, - StableWeight: 100 - *weight, - CanaryService: canaryService, - StableService: stableService, + Matches: matches, + CanaryWeight: *weight, + StableWeight: 100 - *weight, + CanaryService: canaryService, + StableService: stableService, + StableRevision: "podtemplatehash-v1", + CanaryRevision: "podtemplatehash-v2", + RevisionLabelKey: "pod-template-hash", + StableName: trafficRouting.Spec.ObjectRef[0].AdditionalParams[v1beta1.IstioStableSubsetName], + CanaryName: trafficRouting.Spec.ObjectRef[0].AdditionalParams[v1beta1.IstioCanarySubsetName], } nSpec, err := executeLua(data, script) if err != nil { diff --git a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua index 3df3aace..1d45cb6e 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua +++ b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua @@ -1,8 +1,51 @@ local spec = obj.data.spec +local podLabelKey = obj.revisionLabelKey + +if spec.subsets == nil then + spec.subsets = {} +end + +-- selector lables might come from pod-template-hash and patchPodTemplateMetadata +-- now we only support pod-template-hash +local stableLabels = {} +if obj.stableRevision ~= nil and obj.stableRevision ~= "" then + stableLabels[podLabelKey] = obj.stableRevision +end +local canaryLabels = {} +if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canaryLabels[podLabelKey] = obj.canaryRevision +end +local StableNameAlreadyExist = false + +-- if stableName already exists, just appened the lables +for _, subset in ipairs(spec.subsets) do + if subset.name == obj.stableName then + StableNameAlreadyExist = true + if next(stableLabels) ~= nil then + subset.labels = subset.labels or {} + for key, value in pairs(stableLabels) do + subset.labels[key] = value + end + end + end +end +-- if stableName doesn't exist, create it and its labels +if not StableNameAlreadyExist then + local stable = {} + stable.name = obj.stableName + if next(stableLabels) ~= nil then + stable.labels = stableLabels + end + table.insert(spec.subsets, stable) +end + +-- Aussue the canaryName never exist, create it and its labels local canary = {} -canary.labels = {} -canary.name = "canary" -local podLabelKey = "istio.service.tag" -canary.labels[podLabelKey] = "gray" +canary.name = obj.canaryName +if next(canaryLabels) ~= nil then + canary.labels = canaryLabels +end table.insert(spec.subsets, canary) + + return obj.data \ No newline at end of file diff --git a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua index d4d52c23..57a64945 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua +++ b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/VirtualService/trafficRouting.lua @@ -159,7 +159,7 @@ function GenerateMatchedRoutes(spec, matches, stableService, canaryService, stab -- if stableService == canaryService, then do e2e release if stableService == canaryService then route.route[1].destination.host = stableService - route.route[1].destination.subset = "canary" + route.route[1].destination.subset = obj.canaryName else route.route[1].destination.host = canaryService end @@ -189,7 +189,7 @@ function GenerateRoutes(spec, stableService, canaryService, stableWeight, canary canary = { destination = { host = stableService, - subset = "canary", + subset = obj.canaryName, }, weight = canaryWeight, } From 5df31bd4d8a604aae2314c8fe369dee001e8ca02 Mon Sep 17 00:00:00 2001 From: yunbo Date: Wed, 10 Apr 2024 15:46:26 +0800 Subject: [PATCH 2/2] amend: update e2e and improve DR lua script logic Signed-off-by: yunbo --- api/v1alpha1/zz_generated.deepcopy.go | 7 + api/v1beta1/zz_generated.deepcopy.go | 7 + .../DestinationRule/trafficRouting.lua | 68 ++++---- pkg/trafficrouting/manager_test.go | 75 ++++----- .../DestinationRule/trafficRouting.lua | 68 ++++---- test/e2e/rollout_test.go | 2 +- .../lua_script_configmap.yaml | 146 ++++++++++++++---- 7 files changed, 222 insertions(+), 151 deletions(-) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 09217f76..7fbf7ab0 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -921,6 +921,13 @@ func (in *TrafficRoutingRef) DeepCopyInto(out *TrafficRoutingRef) { *out = make([]CustomNetworkRef, len(*in)) copy(*out, *in) } + if in.AdditionalParams != nil { + in, out := &in.AdditionalParams, &out.AdditionalParams + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TrafficRoutingRef. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 408b544f..627be709 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -602,6 +602,13 @@ func (in *TrafficRoutingRef) DeepCopyInto(out *TrafficRoutingRef) { *out = make([]ObjectRef, len(*in)) copy(*out, *in) } + if in.AdditionalParams != nil { + in, out := &in.AdditionalParams, &out.AdditionalParams + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TrafficRoutingRef. diff --git a/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua b/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua index 1d45cb6e..9721bf31 100644 --- a/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua +++ b/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua @@ -1,51 +1,41 @@ -local spec = obj.data.spec -local podLabelKey = obj.revisionLabelKey +local function updateOrCreateSubset(subsets, name, labels) + for _, subset in ipairs(subsets) do + if subset.name == name then + if next(labels) ~= nil then + subset.labels = subset.labels or {} + for key, value in pairs(labels) do + subset.labels[key] = value + end + end + return -- Do not need to continue if name exists,as we update the first occurrence + end + end + table.insert(subsets, { + name = name, + labels = next(labels) ~= nil and labels or nil + }) +end +local spec = obj.data.spec +local pod_label_key = obj.revisionLabelKey if spec.subsets == nil then spec.subsets = {} end --- selector lables might come from pod-template-hash and patchPodTemplateMetadata --- now we only support pod-template-hash -local stableLabels = {} +local stable_labels = {} if obj.stableRevision ~= nil and obj.stableRevision ~= "" then - stableLabels[podLabelKey] = obj.stableRevision -end -local canaryLabels = {} -if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then - canaryLabels[podLabelKey] = obj.canaryRevision + stable_labels[pod_label_key] = obj.stableRevision end -local StableNameAlreadyExist = false --- if stableName already exists, just appened the lables -for _, subset in ipairs(spec.subsets) do - if subset.name == obj.stableName then - StableNameAlreadyExist = true - if next(stableLabels) ~= nil then - subset.labels = subset.labels or {} - for key, value in pairs(stableLabels) do - subset.labels[key] = value - end - end - end -end --- if stableName doesn't exist, create it and its labels -if not StableNameAlreadyExist then - local stable = {} - stable.name = obj.stableName - if next(stableLabels) ~= nil then - stable.labels = stableLabels - end - table.insert(spec.subsets, stable) +local canary_labels = {} +if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canary_labels[pod_label_key] = obj.canaryRevision end --- Aussue the canaryName never exist, create it and its labels -local canary = {} -canary.name = obj.canaryName -if next(canaryLabels) ~= nil then - canary.labels = canaryLabels -end -table.insert(spec.subsets, canary) +-- Process stable subset +updateOrCreateSubset(spec.subsets, obj.stableName, stable_labels) +-- Process canary subset +updateOrCreateSubset(spec.subsets, obj.canaryName, canary_labels) -return obj.data \ No newline at end of file +return obj.data diff --git a/pkg/trafficrouting/manager_test.go b/pkg/trafficrouting/manager_test.go index a07e9b7c..9974b94a 100644 --- a/pkg/trafficrouting/manager_test.go +++ b/pkg/trafficrouting/manager_test.go @@ -447,57 +447,48 @@ var ( `, "lua.traffic.routing.DestinationRule.networking.istio.io": ` - local spec = obj.data.spec - local podLabelKey = obj.revisionLabelKey - - if spec.subsets == nil then - spec.subsets = {} - end - - -- selector lables might come from pod-template-hash and patchPodTemplateMetadata - -- now we only support pod-template-hash - local stableLabels = {} - if obj.stableRevision ~= nil and obj.stableRevision ~= "" then - stableLabels[podLabelKey] = obj.stableRevision - end - local canaryLabels = {} - if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then - canaryLabels[podLabelKey] = obj.canaryRevision - end - local StableNameAlreadyExist = false - - -- if stableName already exists, just appened the lables - for _, subset in ipairs(spec.subsets) do - if subset.name == obj.stableName then - StableNameAlreadyExist = true - if next(stableLabels) ~= nil then + local function updateOrCreateSubset(subsets, name, labels) + for _, subset in ipairs(subsets) do + if subset.name == name then + if next(labels) ~= nil then subset.labels = subset.labels or {} - for key, value in pairs(stableLabels) do + for key, value in pairs(labels) do subset.labels[key] = value end end + return -- Do not need to continue if name exists,as we update the first occurrence end end - -- if stableName doesn't exist, create it and its labels - if not StableNameAlreadyExist then - local stable = {} - stable.name = obj.stableName - if next(stableLabels) ~= nil then - stable.labels = stableLabels - end - table.insert(spec.subsets, stable) - end + table.insert(subsets, { + name = name, + labels = next(labels) ~= nil and labels or nil + }) + end - -- Aussue the canaryName never exist, create it and its labels - local canary = {} - canary.name = obj.canaryName - if next(canaryLabels) ~= nil then - canary.labels = canaryLabels - end - table.insert(spec.subsets, canary) + local spec = obj.data.spec + local pod_label_key = obj.revisionLabelKey + if spec.subsets == nil then + spec.subsets = {} + end + local stable_labels = {} + if obj.stableRevision ~= nil and obj.stableRevision ~= "" then + stable_labels[pod_label_key] = obj.stableRevision + end + + local canary_labels = {} + if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canary_labels[pod_label_key] = obj.canaryRevision + end + + -- Process stable subset + updateOrCreateSubset(spec.subsets, obj.stableName, stable_labels) + + -- Process canary subset + updateOrCreateSubset(spec.subsets, obj.canaryName, canary_labels) + + return obj.data - return obj.data `, }, } diff --git a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua index 1d45cb6e..9721bf31 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua +++ b/pkg/trafficrouting/network/customNetworkProvider/lua_configuration/networking.istio.io/DestinationRule/trafficRouting.lua @@ -1,51 +1,41 @@ -local spec = obj.data.spec -local podLabelKey = obj.revisionLabelKey +local function updateOrCreateSubset(subsets, name, labels) + for _, subset in ipairs(subsets) do + if subset.name == name then + if next(labels) ~= nil then + subset.labels = subset.labels or {} + for key, value in pairs(labels) do + subset.labels[key] = value + end + end + return -- Do not need to continue if name exists,as we update the first occurrence + end + end + table.insert(subsets, { + name = name, + labels = next(labels) ~= nil and labels or nil + }) +end +local spec = obj.data.spec +local pod_label_key = obj.revisionLabelKey if spec.subsets == nil then spec.subsets = {} end --- selector lables might come from pod-template-hash and patchPodTemplateMetadata --- now we only support pod-template-hash -local stableLabels = {} +local stable_labels = {} if obj.stableRevision ~= nil and obj.stableRevision ~= "" then - stableLabels[podLabelKey] = obj.stableRevision -end -local canaryLabels = {} -if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then - canaryLabels[podLabelKey] = obj.canaryRevision + stable_labels[pod_label_key] = obj.stableRevision end -local StableNameAlreadyExist = false --- if stableName already exists, just appened the lables -for _, subset in ipairs(spec.subsets) do - if subset.name == obj.stableName then - StableNameAlreadyExist = true - if next(stableLabels) ~= nil then - subset.labels = subset.labels or {} - for key, value in pairs(stableLabels) do - subset.labels[key] = value - end - end - end -end --- if stableName doesn't exist, create it and its labels -if not StableNameAlreadyExist then - local stable = {} - stable.name = obj.stableName - if next(stableLabels) ~= nil then - stable.labels = stableLabels - end - table.insert(spec.subsets, stable) +local canary_labels = {} +if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canary_labels[pod_label_key] = obj.canaryRevision end --- Aussue the canaryName never exist, create it and its labels -local canary = {} -canary.name = obj.canaryName -if next(canaryLabels) ~= nil then - canary.labels = canaryLabels -end -table.insert(spec.subsets, canary) +-- Process stable subset +updateOrCreateSubset(spec.subsets, obj.stableName, stable_labels) +-- Process canary subset +updateOrCreateSubset(spec.subsets, obj.canaryName, canary_labels) -return obj.data \ No newline at end of file +return obj.data diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index c8a7cae2..70cb7e3c 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -2819,7 +2819,7 @@ var _ = SIGDescribe("Rollout", func() { expectedVSSpec := `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"match":[{"headers":{"user-agent":{"exact":"pc"}}}],"route":[{"destination":{"host":"echoserver","subset":"canary"}}]},{"route":[{"destination":{"host":"echoserver"}}]}]}` Expect(util.DumpJSON(vs.Object["spec"])).Should(Equal(expectedVSSpec)) Expect(GetObject(dr.GetName(), dr)).NotTo(HaveOccurred()) - expectedDRSpec := `{"host":"svc-demo","subsets":[{"labels":{"version":"base"},"name":"echoserver"},{"labels":{"istio.service.tag":"gray"},"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` + expectedDRSpec := `{"host":"svc-demo","subsets":[{"labels":{"version":"base"},"name":"echoserver"},{"name":"stable"},{"name":"canary"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` Expect(util.DumpJSON(dr.Object["spec"])).Should(Equal(expectedDRSpec)) // check original spec annotation expectedVSAnno := `{"spec":{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]}}` diff --git a/test/e2e/test_data/customNetworkProvider/lua_script_configmap.yaml b/test/e2e/test_data/customNetworkProvider/lua_script_configmap.yaml index 5c4866d6..2782ae8b 100644 --- a/test/e2e/test_data/customNetworkProvider/lua_script_configmap.yaml +++ b/test/e2e/test_data/customNetworkProvider/lua_script_configmap.yaml @@ -12,11 +12,25 @@ data: obj.stableWeight = 0 end - function FindAllRules(spec, protocol) + function FindRules(spec, protocol) local rules = {} - if (spec[protocol] ~= nil) then - for _, proto in ipairs(spec[protocol]) do - table.insert(rules, proto) + if (protocol == "http") then + if (spec.http ~= nil) then + for _, http in ipairs(spec.http) do + table.insert(rules, http) + end + end + elseif (protocol == "tcp") then + if (spec.tcp ~= nil) then + for _, http in ipairs(spec.tcp) do + table.insert(rules, http) + end + end + elseif (protocol == "tls") then + if (spec.tls ~= nil) then + for _, http in ipairs(spec.tls) do + table.insert(rules, http) + end end end return rules @@ -25,33 +39,42 @@ data: -- find matched route of VirtualService spec with stable svc function FindMatchedRules(spec, stableService, protocol) local matchedRoutes = {} - local rules = FindAllRules(spec, protocol) + local rules = FindRules(spec, protocol) -- a rule contains 'match' and 'route' for _, rule in ipairs(rules) do - -- skip routes with matches - if (rule.match == nil) then - for _, route in ipairs(rule.route) do - if route.destination.host == stableService then - table.insert(matchedRoutes, rule) - break - end + for _, route in ipairs(rule.route) do + if route.destination.host == stableService then + table.insert(matchedRoutes, rule) + break end end end return matchedRoutes end - function HasMatchedRule(spec, stableService, protocol) - local rules = FindAllRules(spec, protocol) + function FindStableServiceSubsets(spec, stableService, protocol) + local stableSubsets = {} + local rules = FindRules(spec, protocol) + local hasRule = false -- a rule contains 'match' and 'route' for _, rule in ipairs(rules) do for _, route in ipairs(rule.route) do if route.destination.host == stableService then - return true + hasRule = true + local contains = false + for _, v in ipairs(stableSubsets) do + if v == route.destination.subset then + contains = true + break + end + end + if not contains and route.destination.subset ~= nil then + table.insert(stableSubsets, route.destination.subset) + end end end end - return false + return hasRule, stableSubsets end function DeepCopy(original) @@ -79,12 +102,14 @@ data: -- generate routes with matches, insert a rule before other rules function GenerateMatchedRoutes(spec, matches, stableService, canaryService, stableWeight, canaryWeight, protocol) + local hasRule, stableServiceSubsets = FindStableServiceSubsets(spec, stableService, protocol) + if (not hasRule) then + return + end for _, match in ipairs(matches) do local route = {} route["match"] = {} - if (not HasMatchedRule(spec, stableService, protocol)) then - return - end + for key, value in pairs(match) do local vsMatch = {} vsMatch[key] = {} @@ -110,10 +135,38 @@ data: destination = {} } } + -- if stableWeight != 0, then add stable service destinations + -- incase there are multiple subsets in stable service + if stableWeight ~= 0 then + local nRoute = {} + if #stableServiceSubsets ~= 0 then + local weight = CalculateWeight(nRoute, stableWeight, #stableServiceSubsets) + for _, r in ipairs(stableServiceSubsets) do + nRoute = { + destination = { + host = stableService, + subset = r + }, + weight = weight + } + table.insert(route.route, nRoute) + end + else + nRoute = { + destination = { + host = stableService + }, + weight = stableWeight + } + table.insert(route.route, nRoute) + end + -- update every matched route + route.route[1].weight = canaryWeight + end -- if stableService == canaryService, then do e2e release if stableService == canaryService then route.route[1].destination.host = stableService - route.route[1].destination.subset = "canary" + route.route[1].destination.subset = obj.canaryName else route.route[1].destination.host = canaryService end @@ -143,7 +196,7 @@ data: canary = { destination = { host = stableService, - subset = "canary", + subset = obj.canaryName, }, weight = canaryWeight, } @@ -158,8 +211,7 @@ data: end end - if (obj.matches) - then + if (obj.matches) then GenerateMatchedRoutes(spec, obj.matches, obj.stableService, obj.canaryService, obj.stableWeight, obj.canaryWeight, "http") GenerateMatchedRoutes(spec, obj.matches, obj.stableService, obj.canaryService, obj.stableWeight, obj.canaryWeight, "tcp") GenerateMatchedRoutes(spec, obj.matches, obj.stableService, obj.canaryService, obj.stableWeight, obj.canaryWeight, "tls") @@ -170,12 +222,46 @@ data: end return obj.data + lua.traffic.routing.DestinationRule.networking.istio.io: | + local function updateOrCreateSubset(subsets, name, labels) + for _, subset in ipairs(subsets) do + if subset.name == name then + if next(labels) ~= nil then + subset.labels = subset.labels or {} + for key, value in pairs(labels) do + subset.labels[key] = value + end + end + return -- Do not need to continue if name exists,as we update the first occurrence + end + end + table.insert(subsets, { + name = name, + labels = next(labels) ~= nil and labels or nil + }) + end + local spec = obj.data.spec - local canary = {} - canary.labels = {} - canary.name = "canary" - local podLabelKey = "istio.service.tag" - canary.labels[podLabelKey] = "gray" - table.insert(spec.subsets, canary) - return obj.data \ No newline at end of file + local pod_label_key = obj.revisionLabelKey + if spec.subsets == nil then + spec.subsets = {} + end + + local stable_labels = {} + if obj.stableRevision ~= nil and obj.stableRevision ~= "" then + stable_labels[pod_label_key] = obj.stableRevision + end + + local canary_labels = {} + if obj.canaryRevision ~= nil and obj.canaryRevision ~= "" then + canary_labels[pod_label_key] = obj.canaryRevision + end + + -- Process stable subset + updateOrCreateSubset(spec.subsets, obj.stableName, stable_labels) + + -- Process canary subset + updateOrCreateSubset(spec.subsets, obj.canaryName, canary_labels) + + return obj.data