From c7c0c76bd3270d4d90dc14425efe99f9ab3ccdeb Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Fri, 15 Apr 2022 13:33:06 +0530 Subject: [PATCH] fix canary rollback behaviour Prevents the canary from getting triggered, when a canary deploy is updated to match the primary deploy after an analysis fails. Signed-off-by: Sanskar Jaiswal --- artifacts/flagger/crd.yaml | 3 + charts/flagger/crds/crd.yaml | 3 + kustomize/base/flagger/crd.yaml | 3 + pkg/canary/status.go | 3 + pkg/controller/scheduler.go | 8 ++ test/nginx/install.sh | 2 +- test/nginx/run.sh | 2 + test/nginx/test-lifecycle.sh | 233 ++++++++++++++++++++++++++++++++ 8 files changed, 256 insertions(+), 1 deletion(-) create mode 100755 test/nginx/test-lifecycle.sh diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 361dd2cb..19b24319 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -1049,6 +1049,9 @@ spec: lastAppliedSpec: description: LastAppliedSpec of this canary type: string + lastPromotedSpec: + description: LastPromotedSpec of this canary + type: string lastTransitionTime: description: LastTransitionTime of this canary format: date-time diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index 361dd2cb..19b24319 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -1049,6 +1049,9 @@ spec: lastAppliedSpec: description: LastAppliedSpec of this canary type: string + lastPromotedSpec: + description: LastPromotedSpec of this canary + type: string lastTransitionTime: description: LastTransitionTime of this canary format: date-time diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index 361dd2cb..6d31548a 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -1046,6 +1046,9 @@ spec: iterations: description: Iteration count of the current canary analysis type: number + lastPromotedSpec: + description: LastPromotedSpec of this canary + type: string lastAppliedSpec: description: LastAppliedSpec of this canary type: string diff --git a/pkg/canary/status.go b/pkg/canary/status.go index 178ed6ea..a8917e8c 100644 --- a/pkg/canary/status.go +++ b/pkg/canary/status.go @@ -48,6 +48,9 @@ func syncCanaryStatus(flaggerClient clientset.Interface, cd *flaggerv1.Canary, s cdCopy.Status.FailedChecks = status.FailedChecks cdCopy.Status.Iterations = status.Iterations cdCopy.Status.LastAppliedSpec = hash + if status.Phase == flaggerv1.CanaryPhaseInitialized { + cdCopy.Status.LastPromotedSpec = hash + } cdCopy.Status.LastTransitionTime = metav1.Now() setAll(cdCopy) diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index 6bc81ef4..a131ddcf 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -758,6 +758,14 @@ func (c *Controller) shouldAdvance(canary *flaggerv1.Canary, canaryController ca return true, nil } + // Make sure to sync lastAppliedSpec even if the canary is in a failed state. + if canary.Status.Phase == flaggerv1.CanaryPhaseFailed { + if err := canaryController.SyncStatus(canary, canary.Status); err != nil { + c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)).Errorf("%v", err) + return false, err + } + } + newTarget, err := canaryController.HasTargetChanged(canary) if err != nil { return false, err diff --git a/test/nginx/install.sh b/test/nginx/install.sh index 20f0776f..b8681e6e 100755 --- a/test/nginx/install.sh +++ b/test/nginx/install.sh @@ -28,7 +28,7 @@ helm upgrade -i flagger ${REPO_ROOT}/charts/flagger \ --set prometheus.install=true \ --set meshProvider=nginx -# kubectl -n ingress-nginx set image deployment/flagger flagger=test/flagger:latest +kubectl -n ingress-nginx set image deployment/flagger flagger=test/flagger:latest kubectl -n ingress-nginx rollout status deployment/flagger kubectl -n ingress-nginx rollout status deployment/flagger-prometheus diff --git a/test/nginx/run.sh b/test/nginx/run.sh index 67153fa9..16905bb7 100755 --- a/test/nginx/run.sh +++ b/test/nginx/run.sh @@ -9,3 +9,5 @@ DIR="$(cd "$(dirname "$0")" && pwd)" "$REPO_ROOT"/test/workloads/init.sh "$DIR"/test-canary.sh +"$REPO_ROOT"/test/workloads/init.sh +"$DIR"/test-lifecycle.sh diff --git a/test/nginx/test-lifecycle.sh b/test/nginx/test-lifecycle.sh new file mode 100755 index 00000000..3a7ee159 --- /dev/null +++ b/test/nginx/test-lifecycle.sh @@ -0,0 +1,233 @@ +#!/usr/bin/env bash + +# This script runs e2e tests for Canary initialization, analysis and promotion +# Prerequisites: Kubernetes Kind, Helm and NGINX ingress controller + +set -o errexit + +REPO_ROOT=$(git rev-parse --show-toplevel) + +cat <>> Waiting for primary to be ready' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Initialized' && ok=true || ok=false + sleep 5 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '✔ Canary initialization test passed' + +echo '>>> Triggering canary deployment' +kubectl -n test set image deployment/podinfo podinfod=ghcr.io/stefanprodan/podinfo:6.0.1 + +echo '>>> Waiting for canary rollback' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Failed' && ok=true || ok=false + sleep 10 + kubectl -n ingress-nginx logs deployment/flagger --tail 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '✔ Canary rollback test passed' + +pod_hash=$(kubectl get pods -l app=podinfo-primary -n test -o=jsonpath='{.items[0].metadata.labels.pod-template-hash}') + +echo '>>> Reverting canary deployment to match primary' +kubectl -n test set image deployment/podinfo podinfod=ghcr.io/stefanprodan/podinfo:6.0.0 + +sleep 15 + +new_pod_hash=$(kubectl get pods -l app=podinfo-primary -n test -o=jsonpath='{.items[0].metadata.labels.pod-template-hash}') +failed=false +kubectl -n test get canary/podinfo | grep 'Failed' && failed=true || ok=false + +if [ "$new_pod_hash" = "$pod_hash" -a "$failed" = true ]; then + echo '✔ Canary not triggered upon reverting canary image to match primary ' +else + echo '⨯ Canary got triggered upon reverting canary image to match primary' + exit 1 +fi + +echo '>>> Triggering canary deployment again' +kubectl -n test set image deployment/podinfo podinfod=ghcr.io/stefanprodan/podinfo:6.0.1 + +echo '>>> Waiting for canary to start progress' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Progressing' && ok=true || ok=false + sleep 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + kubectl -n test get httpproxy podinfo -oyaml + echo "No more retries left" + exit 1 + fi +done + +echo '>>> Waiting for canary rollback' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Failed' && ok=true || ok=false + sleep 10 + kubectl -n ingress-nginx logs deployment/flagger --tail 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +cat <>> Retrying failed canary run' +kubectl -n test patch deploy/podinfo -p '[{"op": "add", "path":"/spec/template/metadata/annotations", "value": {"thisis": "theway"}}]' --type=json + +echo '>>> Waiting for canary promotion' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test describe deployment/podinfo-primary | grep '6.0.1' && ok=true || ok=false + sleep 10 + kubectl -n ingress-nginx logs deployment/flagger --tail 1 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '>>> Waiting for canary finalization' +retries=50 +count=0 +ok=false +until ${ok}; do + kubectl -n test get canary/podinfo | grep 'Succeeded' && ok=true || ok=false + sleep 5 + count=$(($count + 1)) + if [[ ${count} -eq ${retries} ]]; then + kubectl -n ingress-nginx logs deployment/flagger + echo "No more retries left" + exit 1 + fi +done + +echo '✔ Canary promotion test passed'