From 0be92dc28e3890a788e9de0d0fc023f7d23f7610 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Fri, 28 Jun 2024 18:38:31 +0800 Subject: [PATCH] fix nginx lua script and add E2E Signed-off-by: Megrez Lu --- .../trafficrouting_ingress/nginx.lua | 24 +- pkg/trafficrouting/network/composite.go | 4 +- test/Makefile | 19 ++ test/e2e/rollout_test.go | 234 ++++++++++++++++++ .../rollout_with_multi_trafficrouting.yaml | 37 +++ .../rollout_with_trafficrouting.yaml | 6 - test/e2e/test_data/rollout/deployment.yaml | 2 +- 7 files changed, 307 insertions(+), 19 deletions(-) create mode 100644 test/Makefile create mode 100644 test/e2e/test_data/customNetworkProvider/rollout_with_multi_trafficrouting.yaml diff --git a/lua_configuration/trafficrouting_ingress/nginx.lua b/lua_configuration/trafficrouting_ingress/nginx.lua index 16ec4c6..5f0b5d8 100644 --- a/lua_configuration/trafficrouting_ingress/nginx.lua +++ b/lua_configuration/trafficrouting_ingress/nginx.lua @@ -26,19 +26,21 @@ end -- headers & cookie apis -- traverse matches for _,match in ipairs(obj.matches) do - local header = match.headers[1] - -- cookie - if ( header.name == "canary-by-cookie" ) - then - annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = header.value - else - annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name - -- if regular expression - if ( header.type == "RegularExpression" ) + if match.headers and next(match.headers) ~= nil then + local header = match.headers[1] + -- cookie + if ( header.name == "canary-by-cookie" ) then - annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = header.value + annotations["nginx.ingress.kubernetes.io/canary-by-cookie"] = header.value else - annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = header.value + annotations["nginx.ingress.kubernetes.io/canary-by-header"] = header.name + -- if regular expression + if ( header.type == "RegularExpression" ) + then + annotations["nginx.ingress.kubernetes.io/canary-by-header-pattern"] = header.value + else + annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = header.value + end end end end diff --git a/pkg/trafficrouting/network/composite.go b/pkg/trafficrouting/network/composite.go index cce1f26..afe50fb 100644 --- a/pkg/trafficrouting/network/composite.go +++ b/pkg/trafficrouting/network/composite.go @@ -28,6 +28,7 @@ var ( _ NetworkProvider = (CompositeController)(nil) ) +// CompositeController is a set of NetworkProvider type CompositeController []NetworkProvider func (c CompositeController) Initialize(ctx context.Context) error { @@ -44,8 +45,9 @@ func (c CompositeController) EnsureRoutes(ctx context.Context, strategy *v1beta1 innerDone, innerErr := provider.EnsureRoutes(ctx, strategy) if innerErr != nil { return false, innerErr + } else if !innerDone { + done = false } - done = done && !innerDone } return done, nil } diff --git a/test/Makefile b/test/Makefile new file mode 100644 index 0000000..b57d7c4 --- /dev/null +++ b/test/Makefile @@ -0,0 +1,19 @@ +GITHUB_RUN_ID ?= latest +KIND_CLUSTER_NAME ?= ci-testing +IMAGE ?= openkruise/kruise-rollout:e2e-$(GITHUB_RUN_ID) + +.PHONY: build-image +build-image: + docker build --pull --no-cache ../ -t $(IMAGE) + +.PHONY: load-image +load-image: build-image + kind load docker-image --name=$(KIND_CLUSTER_NAME) $(IMAGE) + +.PHONY: install-kruise-rollout +install-kruise-rollout: + @pushd .. > /dev/null && IMG=$(IMAGE) ./scripts/deploy_kind.sh && popd > /dev/null + kubectl wait --namespace kruise-rollout \ + --for=condition=ready pod \ + --selector=control-plane=controller-manager \ + --timeout=600s \ No newline at end of file diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index 45fd3ce..bbf8d75 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -2807,6 +2807,240 @@ var _ = SIGDescribe("Rollout", func() { }) }) + KruiseDescribe("Canary rollout with multiple network providers", func() { + It("V1->V2: Route traffic with header/queryParams/path matches and weight using rollout for Istio and Ingress", func() { + By("Creating Rollout...") + rollout := &v1beta1.Rollout{} + Expect(ReadYamlToObject("./test_data/customNetworkProvider/rollout_with_multi_trafficrouting.yaml", rollout)).ToNot(HaveOccurred()) + CreateObject(rollout) + + By("Creating workload and waiting for all pods ready...") + // service + service := &v1.Service{} + Expect(ReadYamlToObject("./test_data/rollout/service.yaml", service)).ToNot(HaveOccurred()) + CreateObject(service) + // istio api + vs := &unstructured.Unstructured{} + Expect(ReadYamlToObject("./test_data/customNetworkProvider/virtualservice_without_destinationrule.yaml", vs)).ToNot(HaveOccurred()) + vs.SetAPIVersion("networking.istio.io/v1alpha3") + vs.SetKind("VirtualService") + CreateObject(vs) + // ingress + ingress := &netv1.Ingress{} + Expect(ReadYamlToObject("./test_data/rollout/nginx_ingress.yaml", ingress)).ToNot(HaveOccurred()) + CreateObject(ingress) + // workload + workload := &apps.Deployment{} + Expect(ReadYamlToObject("./test_data/rollout/deployment.yaml", workload)).ToNot(HaveOccurred()) + workload.Spec.Replicas = utilpointer.Int32(4) + CreateObject(workload) + WaitDeploymentAllPodsReady(workload) + + // check rollout status + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + Expect(rollout.Status.Phase).Should(Equal(v1beta1.RolloutPhaseHealthy)) + By("check rollout status & paused success") + + // v1 -> v2, start rollout action + newEnvs := mergeEnvVar(workload.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "NODE_NAME", Value: "version2"}) + workload.Spec.Template.Spec.Containers[0].Env = newEnvs + UpdateDeployment(workload) + By("Update deployment env NODE_NAME from(version1) -> to(version2), routing traffic with header agent:pc to new version pods") + time.Sleep(time.Second * 2) + + Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) + Expect(workload.Spec.Paused).Should(BeTrue()) + // wait step 1 complete + WaitRolloutCanaryStepPaused(rollout.Name, 1) + // check rollout status + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + Expect(rollout.Status.CanaryStatus.CanaryReplicas).Should(BeNumerically("==", 1)) + Expect(rollout.Status.CanaryStatus.CanaryReadyReplicas).Should(BeNumerically("==", 1)) + // check virtualservice spec + Expect(GetObject(vs.GetName(), vs)).NotTo(HaveOccurred()) + expectedSpec := `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"match":[{"uri":{"prefix":"/pc"}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"match":[{"queryParams":{"user-agent":{"exact":"pc"}}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"match":[{"headers":{"user-agent":{"exact":"pc"}}}],"route":[{"destination":{"host":"echoserver-canary"}}]},{"route":[{"destination":{"host":"echoserver"}}]}]}` + Expect(util.DumpJSON(vs.Object["spec"])).Should(Equal(expectedSpec)) + // check original spec annotation + expectedAnno := `{"spec":{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]}}` + Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal(expectedAnno)) + // check canary-ingress spec + cIngress := &netv1.Ingress{} + Expect(GetObject(service.Name+"-canary", cIngress)).NotTo(HaveOccurred()) + Expect(cIngress.Annotations[fmt.Sprintf("%s/canary", nginxIngressAnnotationDefaultPrefix)]).Should(Equal("true")) + Expect(cIngress.Annotations[fmt.Sprintf("%s/canary-weight", nginxIngressAnnotationDefaultPrefix)]).Should(Equal("0")) + + // resume rollout canary + ResumeRolloutCanaryV1beta1(rollout.Name) + By("Resume rollout, and wait next step(2), routing 50% traffic to new version pods") + WaitRolloutCanaryStepPaused(rollout.Name, 2) + // check rollout status + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + Expect(rollout.Status.CanaryStatus.CanaryReplicas).Should(BeNumerically("==", 2)) + Expect(rollout.Status.CanaryStatus.CanaryReadyReplicas).Should(BeNumerically("==", 2)) + // check virtualservice spec + Expect(GetObject(vs.GetName(), vs)).NotTo(HaveOccurred()) + expectedSpec = `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"},"weight":50},{"destination":{"host":"echoserver-canary"},"weight":50}]}]}` + Expect(util.DumpJSON(vs.Object["spec"])).Should(Equal(expectedSpec)) + // check original spec annotation + Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal(expectedAnno)) + // check canary-ingress spec + Expect(GetObject(service.Name+"-canary", cIngress)).NotTo(HaveOccurred()) + Expect(cIngress.Annotations[fmt.Sprintf("%s/canary", nginxIngressAnnotationDefaultPrefix)]).Should(Equal("true")) + Expect(cIngress.Annotations[fmt.Sprintf("%s/canary-weight", nginxIngressAnnotationDefaultPrefix)]).Should(Equal(removePercentageSign(*rollout.Spec.Strategy.Canary.Steps[1].Traffic))) + + // resume rollout + ResumeRolloutCanaryV1beta1(rollout.Name) + WaitRolloutStatusPhase(rollout.Name, v1alpha1.RolloutPhaseHealthy) + By("rollout completed, and check") + // check service & virtualservice & deployment + // virtualservice + Expect(GetObject(vs.GetName(), vs)).NotTo(HaveOccurred()) + expectedSpec = `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]}` + Expect(util.DumpJSON(vs.Object["spec"])).Should(Equal(expectedSpec)) + Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal("")) + // service + Expect(GetObject(service.Name, service)).NotTo(HaveOccurred()) + Expect(service.Spec.Selector[apps.DefaultDeploymentUniqueLabelKey]).Should(Equal("")) + cService := &v1.Service{} + Expect(GetObject(fmt.Sprintf("%s-canary", service.Name), cService)).To(HaveOccurred()) + // deployment + Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) + Expect(workload.Spec.Paused).Should(BeFalse()) + Expect(workload.Status.UpdatedReplicas).Should(BeNumerically("==", *workload.Spec.Replicas)) + Expect(workload.Status.Replicas).Should(BeNumerically("==", *workload.Spec.Replicas)) + Expect(workload.Status.ReadyReplicas).Should(BeNumerically("==", *workload.Spec.Replicas)) + for _, env := range workload.Spec.Template.Spec.Containers[0].Env { + if env.Name == "NODE_NAME" { + Expect(env.Value).Should(Equal("version2")) + } + } + // check progressing succeed + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + cond := getRolloutConditionV1beta1(rollout.Status, v1beta1.RolloutConditionProgressing) + Expect(cond.Reason).Should(Equal(v1beta1.ProgressingReasonCompleted)) + Expect(string(cond.Status)).Should(Equal(string(metav1.ConditionFalse))) + cond = getRolloutConditionV1beta1(rollout.Status, v1beta1.RolloutConditionSucceeded) + Expect(string(cond.Status)).Should(Equal(string(metav1.ConditionTrue))) + Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) + WaitRolloutWorkloadGeneration(rollout.Name, workload.Generation) + }) + + It("V1->V2: Route traffic with header matches and weight for VirtualService and DestinationRule", func() { + By("Creating Rollout...") + rollout := &v1alpha1.Rollout{} + Expect(ReadYamlToObject("./test_data/customNetworkProvider/rollout_without_trafficrouting.yaml", rollout)).ToNot(HaveOccurred()) + CreateObject(rollout) + + By("Creating TrafficRouting...") + traffic := &v1alpha1.TrafficRouting{} + Expect(ReadYamlToObject("./test_data/customNetworkProvider/trafficrouting.yaml", traffic)).ToNot(HaveOccurred()) + CreateObject(traffic) + + By("Creating workload and waiting for all pods ready...") + // service + service := &v1.Service{} + Expect(ReadYamlToObject("./test_data/rollout/service.yaml", service)).ToNot(HaveOccurred()) + CreateObject(service) + // istio api + vs := &unstructured.Unstructured{} + Expect(ReadYamlToObject("./test_data/customNetworkProvider/virtualservice_without_destinationrule.yaml", vs)).ToNot(HaveOccurred()) + vs.SetAPIVersion("networking.istio.io/v1alpha3") + vs.SetKind("VirtualService") + CreateObject(vs) + dr := &unstructured.Unstructured{} + Expect(ReadYamlToObject("./test_data/customNetworkProvider/destinationrule.yaml", dr)).ToNot(HaveOccurred()) + dr.SetAPIVersion("networking.istio.io/v1alpha3") + dr.SetKind("DestinationRule") + CreateObject(dr) + // workload + workload := &apps.Deployment{} + Expect(ReadYamlToObject("./test_data/rollout/deployment.yaml", workload)).ToNot(HaveOccurred()) + workload.Spec.Replicas = utilpointer.Int32(4) + CreateObject(workload) + WaitDeploymentAllPodsReady(workload) + + // check rollout and trafficrouting status + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + Expect(rollout.Status.Phase).Should(Equal(v1alpha1.RolloutPhaseHealthy)) + Expect(GetObject(traffic.Name, traffic)).NotTo(HaveOccurred()) + Expect(traffic.Status.Phase).Should(Equal(v1alpha1.TrafficRoutingPhaseHealthy)) + By("check rollout and trafficrouting status & paused success") + + // v1 -> v2, start rollout action + newEnvs := mergeEnvVar(workload.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "NODE_NAME", Value: "version2"}) + workload.Spec.Template.Spec.Containers[0].Env = newEnvs + UpdateDeployment(workload) + By("Update deployment env NODE_NAME from(version1) -> to(version2), routing traffic with header agent:pc to new version pods") + time.Sleep(time.Second * 2) + + Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) + Expect(workload.Spec.Paused).Should(BeTrue()) + // wait step 1 complete + WaitRolloutCanaryStepPaused(rollout.Name, 1) + // check rollout and trafficrouting status + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + Expect(rollout.Status.CanaryStatus.CanaryReplicas).Should(BeNumerically("==", 1)) + Expect(rollout.Status.CanaryStatus.CanaryReadyReplicas).Should(BeNumerically("==", 1)) + Expect(GetObject(traffic.Name, traffic)).NotTo(HaveOccurred()) + Expect(traffic.Status.Phase).Should(Equal(v1alpha1.TrafficRoutingPhaseProgressing)) + // check virtualservice and destinationrule spec + Expect(GetObject(vs.GetName(), vs)).NotTo(HaveOccurred()) + 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"}}}` + Expect(util.DumpJSON(dr.Object["spec"])).Should(Equal(expectedDRSpec)) + // check original spec annotation + expectedVSAnno := `{"spec":{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]}}` + Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal(expectedVSAnno)) + expectedDRAnno := `{"spec":{"host":"svc-demo","subsets":[{"labels":{"version":"base"},"name":"echoserver"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}}` + Expect(dr.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal(expectedDRAnno)) + + // resume rollout + ResumeRolloutCanary(rollout.Name) + WaitRolloutStatusPhase(rollout.Name, v1alpha1.RolloutPhaseHealthy) + Expect(GetObject(traffic.Name, traffic)).NotTo(HaveOccurred()) + Expect(traffic.Status.Phase).Should(Equal(v1alpha1.TrafficRoutingPhaseHealthy)) + By("rollout completed, and check") + // check service & virtualservice & destinationrule & deployment + // virtualservice and destinationrule + Expect(GetObject(vs.GetName(), vs)).NotTo(HaveOccurred()) + expectedVSSpec = `{"gateways":["nginx-gateway"],"hosts":["*"],"http":[{"route":[{"destination":{"host":"echoserver"}}]}]}` + Expect(util.DumpJSON(vs.Object["spec"])).Should(Equal(expectedVSSpec)) + Expect(vs.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal("")) + + Expect(GetObject(dr.GetName(), dr)).NotTo(HaveOccurred()) + expectedDRSpec = `{"host":"svc-demo","subsets":[{"labels":{"version":"base"},"name":"echoserver"}],"trafficPolicy":{"loadBalancer":{"simple":"ROUND_ROBIN"}}}` + Expect(util.DumpJSON(dr.Object["spec"])).Should(Equal(expectedDRSpec)) + Expect(dr.GetAnnotations()[OriginalSpecAnnotation]).Should(Equal("")) + // service + Expect(GetObject(service.Name, service)).NotTo(HaveOccurred()) + Expect(service.Spec.Selector[apps.DefaultDeploymentUniqueLabelKey]).Should(Equal("")) + cService := &v1.Service{} + Expect(GetObject(fmt.Sprintf("%s-canary", service.Name), cService)).To(HaveOccurred()) + // deployment + Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) + Expect(workload.Spec.Paused).Should(BeFalse()) + Expect(workload.Status.UpdatedReplicas).Should(BeNumerically("==", *workload.Spec.Replicas)) + Expect(workload.Status.Replicas).Should(BeNumerically("==", *workload.Spec.Replicas)) + Expect(workload.Status.ReadyReplicas).Should(BeNumerically("==", *workload.Spec.Replicas)) + for _, env := range workload.Spec.Template.Spec.Containers[0].Env { + if env.Name == "NODE_NAME" { + Expect(env.Value).Should(Equal("version2")) + } + } + // check progressing succeed + Expect(GetObject(rollout.Name, rollout)).NotTo(HaveOccurred()) + cond := getRolloutCondition(rollout.Status, v1alpha1.RolloutConditionProgressing) + Expect(cond.Reason).Should(Equal(v1alpha1.ProgressingReasonCompleted)) + Expect(string(cond.Status)).Should(Equal(string(metav1.ConditionFalse))) + cond = getRolloutCondition(rollout.Status, v1alpha1.RolloutConditionSucceeded) + Expect(string(cond.Status)).Should(Equal(string(metav1.ConditionTrue))) + Expect(GetObject(workload.Name, workload)).NotTo(HaveOccurred()) + WaitRolloutWorkloadGeneration(rollout.Name, workload.Generation) + }) + }) + KruiseDescribe("DaemonSet canary rollout", func() { It("DaemonSet V1->V2: 1,100% Succeeded", func() { By("Creating Rollout...") diff --git a/test/e2e/test_data/customNetworkProvider/rollout_with_multi_trafficrouting.yaml b/test/e2e/test_data/customNetworkProvider/rollout_with_multi_trafficrouting.yaml new file mode 100644 index 0000000..5fa5c71 --- /dev/null +++ b/test/e2e/test_data/customNetworkProvider/rollout_with_multi_trafficrouting.yaml @@ -0,0 +1,37 @@ + apiVersion: rollouts.kruise.io/v1beta1 + kind: Rollout + metadata: + name: rollouts-demo + spec: + disabled: false + workloadRef: + apiVersion: apps/v1 + kind: Deployment + name: echoserver + strategy: + canary: + enableExtraWorkloadForCanary: true + steps: + - replicas: 1 + matches: + - headers: + - type: Exact + name: user-agent + value: pc + - queryParams: + - type: Exact + name: user-agent + value: pc + - path: + value: /pc + - replicas: "50%" + traffic: "50%" + trafficRoutings: + - service: echoserver + ingress: + classType: nginx + name: echoserver + customNetworkRefs: + - apiVersion: networking.istio.io/v1alpha3 + kind: VirtualService + name: vs-demo diff --git a/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml b/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml index 713ce5d..c705cff 100644 --- a/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml +++ b/test/e2e/test_data/customNetworkProvider/rollout_with_trafficrouting.yaml @@ -18,12 +18,6 @@ - type: Exact name: user-agent value: pc - - queryParams: - - type: Exact - name: user-agent - value: pc - - path: - value: /pc - replicas: "50%" traffic: "50%" trafficRoutings: diff --git a/test/e2e/test_data/rollout/deployment.yaml b/test/e2e/test_data/rollout/deployment.yaml index 68e1fea..cccfb93 100644 --- a/test/e2e/test_data/rollout/deployment.yaml +++ b/test/e2e/test_data/rollout/deployment.yaml @@ -21,7 +21,7 @@ spec: spec: containers: - name: echoserver - image: cilium/echoserver:latest + image: jmalloc/echo-server:latest # imagePullPolicy: IfNotPresent ports: - containerPort: 8080