From 69875cb3dce458cab66f8f0666383e215482564b Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Mon, 29 Jul 2019 13:43:30 +0300 Subject: [PATCH 1/3] Add finalising status phase to CRD --- artifacts/flagger/crd.yaml | 1 + charts/flagger/templates/crd.yaml | 1 + kustomize/base/flagger/crd.yaml | 1 + pkg/apis/flagger/v1alpha3/status.go | 2 ++ 4 files changed, 5 insertions(+) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index ed54fbfe..548ef6d2 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -176,6 +176,7 @@ spec: - Initialized - Waiting - Progressing + - Finalising - Succeeded - Failed canaryWeight: diff --git a/charts/flagger/templates/crd.yaml b/charts/flagger/templates/crd.yaml index e7cd7641..a60bf529 100644 --- a/charts/flagger/templates/crd.yaml +++ b/charts/flagger/templates/crd.yaml @@ -177,6 +177,7 @@ spec: - Initialized - Waiting - Progressing + - Finalising - Succeeded - Failed canaryWeight: diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index ed54fbfe..548ef6d2 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -176,6 +176,7 @@ spec: - Initialized - Waiting - Progressing + - Finalising - Succeeded - Failed canaryWeight: diff --git a/pkg/apis/flagger/v1alpha3/status.go b/pkg/apis/flagger/v1alpha3/status.go index b0ca1fd9..afe7cf28 100644 --- a/pkg/apis/flagger/v1alpha3/status.go +++ b/pkg/apis/flagger/v1alpha3/status.go @@ -47,6 +47,8 @@ const ( CanaryPhaseWaiting CanaryPhase = "Waiting" // CanaryPhaseProgressing means the canary analysis is underway CanaryPhaseProgressing CanaryPhase = "Progressing" + // CanaryPhaseProgressing means the canary analysis is finished and traffic has been routed back to primary + CanaryPhaseFinalising CanaryPhase = "Finalising" // CanaryPhaseSucceeded means the canary analysis has been successful // and the canary deployment has been promoted CanaryPhaseSucceeded CanaryPhase = "Succeeded" From b2ca0c4c16fd4c361863b835c0d54e203b39e233 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Mon, 29 Jul 2019 13:52:11 +0300 Subject: [PATCH 2/3] Implement finalising state Set the canary status to finalising after routing the traffic back to the primary. Run one final loop before scaling the canary to zero so that the canary has a chance to process all inflight requests. --- pkg/canary/status.go | 3 ++ pkg/controller/scheduler.go | 69 ++++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/pkg/canary/status.go b/pkg/canary/status.go index a744d437..e68d911e 100644 --- a/pkg/canary/status.go +++ b/pkg/canary/status.go @@ -211,6 +211,9 @@ func (c *Deployer) MakeStatusConditions(canaryStatus flaggerv1.CanaryStatus, case flaggerv1.CanaryPhaseProgressing: status = corev1.ConditionUnknown message = "New revision detected, starting canary analysis." + case flaggerv1.CanaryPhaseFinalising: + status = corev1.ConditionUnknown + message = "Canary analysis completed, routing all traffic to primary." case flaggerv1.CanaryPhaseSucceeded: status = corev1.ConditionTrue message = "Canary analysis completed successfully, promotion finished." diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index cf9a595b..9454524f 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -214,6 +214,26 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh return } + // scale canary to zero if analysis has succeeded + if cd.Status.Phase == flaggerv1.CanaryPhaseFinalising { + if err := c.deployer.Scale(cd, 0); err != nil { + c.recordEventWarningf(cd, "%v", err) + return + } + + // set status to succeeded + if err := c.deployer.SetStatusPhase(cd, flaggerv1.CanaryPhaseSucceeded); err != nil { + c.recordEventWarningf(cd, "%v", err) + return + } + c.recorder.SetStatus(cd, flaggerv1.CanaryPhaseSucceeded) + c.runPostRolloutHooks(cd, flaggerv1.CanaryPhaseSucceeded) + c.recordEventInfof(cd, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) + c.sendNotification(cd, "Canary analysis completed successfully, promotion finished.", + false, false) + return + } + // check if the number of failed checks reached the threshold if cd.Status.Phase == flaggerv1.CanaryPhaseProgressing && (!retriable || cd.Status.FailedChecks >= cd.Spec.CanaryAnalysis.Threshold) { @@ -319,31 +339,23 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh return } - // shutdown canary + // route all traffic to primary if cd.Spec.CanaryAnalysis.Iterations < cd.Status.Iterations { - // route all traffic to the primary - if err := meshRouter.SetRoutes(cd, 100, 0); err != nil { - c.recordEventWarningf(cd, "%v", err) - return - } - c.recorder.SetWeight(cd, 100, 0) - c.recordEventInfof(cd, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) - - // canary scale to zero - if err := c.deployer.Scale(cd, 0); err != nil { + primaryWeight = 100 + canaryWeight = 0 + if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } + c.recorder.SetWeight(cd, primaryWeight, canaryWeight) // update status phase - if err := c.deployer.SetStatusPhase(cd, flaggerv1.CanaryPhaseSucceeded); err != nil { + if err := c.deployer.SetStatusPhase(cd, flaggerv1.CanaryPhaseFinalising); err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetStatus(cd, flaggerv1.CanaryPhaseSucceeded) - c.runPostRolloutHooks(cd, flaggerv1.CanaryPhaseSucceeded) - c.sendNotification(cd, "Canary analysis completed successfully, promotion finished.", - false, false) + + c.recordEventInfof(cd, "Routing all traffic to primary") return } @@ -385,32 +397,23 @@ func (c *Controller) advanceCanary(name string, namespace string, skipLivenessCh } } } else { - // route all traffic back to primary + // route all traffic to primary primaryWeight = 100 canaryWeight = 0 if err := meshRouter.SetRoutes(cd, primaryWeight, canaryWeight); err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetWeight(cd, primaryWeight, canaryWeight) - c.recordEventInfof(cd, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) - - // shutdown canary - if err := c.deployer.Scale(cd, 0); err != nil { - c.recordEventWarningf(cd, "%v", err) - return - } // update status phase - if err := c.deployer.SetStatusPhase(cd, flaggerv1.CanaryPhaseSucceeded); err != nil { + if err := c.deployer.SetStatusPhase(cd, flaggerv1.CanaryPhaseFinalising); err != nil { c.recordEventWarningf(cd, "%v", err) return } - c.recorder.SetStatus(cd, flaggerv1.CanaryPhaseSucceeded) - c.runPostRolloutHooks(cd, flaggerv1.CanaryPhaseSucceeded) - c.sendNotification(cd, "Canary analysis completed successfully, promotion finished.", - false, false) + + c.recordEventInfof(cd, "Routing all traffic to primary") + return } } @@ -462,7 +465,8 @@ func (c *Controller) shouldAdvance(cd *flaggerv1.Canary) (bool, error) { if cd.Status.LastAppliedSpec == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing || cd.Status.Phase == flaggerv1.CanaryPhaseProgressing || - cd.Status.Phase == flaggerv1.CanaryPhaseWaiting { + cd.Status.Phase == flaggerv1.CanaryPhaseWaiting || + cd.Status.Phase == flaggerv1.CanaryPhaseFinalising { return true, nil } @@ -485,7 +489,8 @@ func (c *Controller) shouldAdvance(cd *flaggerv1.Canary) (bool, error) { func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary, shouldAdvance bool) bool { c.recorder.SetStatus(cd, cd.Status.Phase) - if cd.Status.Phase == flaggerv1.CanaryPhaseProgressing { + if cd.Status.Phase == flaggerv1.CanaryPhaseProgressing || + cd.Status.Phase == flaggerv1.CanaryPhaseFinalising { return true } From c463b6b231f2b8db48b8d25e3755755d93c35588 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Mon, 29 Jul 2019 14:02:16 +0300 Subject: [PATCH 3/3] Add finalising state tests --- pkg/controller/scheduler_test.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/pkg/controller/scheduler_test.go b/pkg/controller/scheduler_test.go index bd88e8e4..c2838cc9 100644 --- a/pkg/controller/scheduler_test.go +++ b/pkg/controller/scheduler_test.go @@ -250,11 +250,20 @@ func TestScheduler_Promotion(t *testing.T) { t.Errorf("Got primary secret %s wanted %s", secretPrimary.Data["apiKey"], secret2.Data["apiKey"]) } + // check finalising status c, err := mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) if err != nil { t.Fatal(err.Error()) } + // scale canary to zero + mocks.ctrl.advanceCanary("podinfo", "default", true) + + c, err = mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + if c.Status.Phase != v1alpha3.CanaryPhaseSucceeded { t.Errorf("Got canary state %v wanted %v", c.Status.Phase, v1alpha3.CanaryPhaseSucceeded) } @@ -302,9 +311,22 @@ func TestScheduler_ABTesting(t *testing.T) { t.Fatal(err.Error()) } - // promote + // advance mocks.ctrl.advanceCanary("podinfo", "default", true) + // finalising + mocks.ctrl.advanceCanary("podinfo", "default", true) + + // check finalising status + c, err := mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + if c.Status.Phase != v1alpha3.CanaryPhaseFinalising { + t.Errorf("Got canary state %v wanted %v", c.Status.Phase, v1alpha3.CanaryPhaseFinalising) + } + // check if the container image tag was updated primaryDep, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{}) if err != nil { @@ -321,7 +343,7 @@ func TestScheduler_ABTesting(t *testing.T) { mocks.ctrl.advanceCanary("podinfo", "default", true) // check rollout status - c, err := mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) + c, err = mocks.flaggerClient.FlaggerV1alpha3().Canaries("default").Get("podinfo", metav1.GetOptions{}) if err != nil { t.Fatal(err.Error()) }