From acdd2c46d572227afebee64eb62554162c664e72 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Wed, 16 Jan 2019 15:06:38 +0200 Subject: [PATCH] Refactor Canary status - add status phases (Initialized, Progressing, Succeeded, Failed) - rename status revision to LastAppliedSpec --- artifacts/flagger/crd.yaml | 2 +- charts/flagger/templates/crd.yaml | 2 +- pkg/apis/flagger/v1alpha3/types.go | 29 +++++++++++++++++++---------- pkg/controller/deployer.go | 16 ++++++++-------- pkg/controller/deployer_test.go | 12 ++++++------ pkg/controller/recorder.go | 4 ++-- pkg/controller/scheduler.go | 14 +++++++------- pkg/controller/scheduler_test.go | 6 +++--- 8 files changed, 47 insertions(+), 38 deletions(-) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 7983760b..ce7671ca 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -25,7 +25,7 @@ spec: additionalPrinterColumns: - name: Status type: string - JSONPath: .status.state + JSONPath: .status.phase - name: LastTransitionTime type: string JSONPath: .status.lastTransitionTime diff --git a/charts/flagger/templates/crd.yaml b/charts/flagger/templates/crd.yaml index 275be88c..6f58478d 100644 --- a/charts/flagger/templates/crd.yaml +++ b/charts/flagger/templates/crd.yaml @@ -26,7 +26,7 @@ spec: additionalPrinterColumns: - name: Status type: string - JSONPath: .status.state + JSONPath: .status.phase - name: LastTransitionTime type: string JSONPath: .status.lastTransitionTime diff --git a/pkg/apis/flagger/v1alpha3/types.go b/pkg/apis/flagger/v1alpha3/types.go index 944f9efa..32ea50ec 100755 --- a/pkg/apis/flagger/v1alpha3/types.go +++ b/pkg/apis/flagger/v1alpha3/types.go @@ -56,7 +56,7 @@ type CanarySpec struct { CanaryAnalysis CanaryAnalysis `json:"canaryAnalysis"` // the maximum time in seconds for a canary deployment to make progress - // before it is considered to be failed. Defaults to 60s. + // before it is considered to be failed. Defaults to ten minutes. ProgressDeadlineSeconds *int32 `json:"progressDeadlineSeconds,omitempty"` } @@ -70,21 +70,29 @@ type CanaryList struct { Items []Canary `json:"items"` } -// CanaryState used for status state op -type CanaryState string +// CanaryPhase is a label for the condition of a canary at the current time +type CanaryPhase string const ( - CanaryRunning CanaryState = "running" - CanaryFinished CanaryState = "finished" - CanaryFailed CanaryState = "failed" - CanaryInitialized CanaryState = "initialized" + // CanaryInitialized means the primary deployment, hpa and ClusterIP services + // have been created along with the Istio virtual service + CanaryInitialized CanaryPhase = "Initialized" + // CanaryProgressing means the canary analysis is underway + CanaryProgressing CanaryPhase = "Progressing" + // CanarySucceeded means the canary analysis has been successful + // and the canary deployment has been promoted + CanarySucceeded CanaryPhase = "Succeeded" + // CanaryFailed means the canary analysis failed + // and the canary deployment has been scaled to zero + CanaryFailed CanaryPhase = "Failed" ) // CanaryStatus is used for state persistence (read-only) type CanaryStatus struct { - State CanaryState `json:"state"` - CanaryRevision string `json:"canaryRevision"` - FailedChecks int `json:"failedChecks"` + Phase CanaryPhase `json:"phase"` + FailedChecks int `json:"failedChecks"` + // +optional + LastAppliedSpec string `json:"lastAppliedSpec,omitempty"` // +optional LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` } @@ -139,6 +147,7 @@ func (c *Canary) GetProgressDeadlineSeconds() int { return ProgressDeadlineSeconds } +// GetAnalysisInterval returns the canary analysis interval (default 60s) func (c *Canary) GetAnalysisInterval() time.Duration { if c.Spec.CanaryAnalysis.Interval == "" { return AnalysisInterval diff --git a/pkg/controller/deployer.go b/pkg/controller/deployer.go index db6e866d..94cdc179 100644 --- a/pkg/controller/deployer.go +++ b/pkg/controller/deployer.go @@ -133,12 +133,12 @@ func (c *CanaryDeployer) IsNewSpec(cd *flaggerv1.Canary) (bool, error) { return false, fmt.Errorf("deployment %s.%s query error %v", targetName, cd.Namespace, err) } - if cd.Status.CanaryRevision == "" { + if cd.Status.LastAppliedSpec == "" { return true, nil } newSpec := &canary.Spec.Template.Spec - oldSpecJson, err := base64.StdEncoding.DecodeString(cd.Status.CanaryRevision) + oldSpecJson, err := base64.StdEncoding.DecodeString(cd.Status.LastAppliedSpec) if err != nil { return false, fmt.Errorf("%s.%s decode error %v", cd.Name, cd.Namespace, err) } @@ -158,7 +158,7 @@ func (c *CanaryDeployer) IsNewSpec(cd *flaggerv1.Canary) (bool, error) { // ShouldAdvance determines if the canary analysis can proceed func (c *CanaryDeployer) ShouldAdvance(cd *flaggerv1.Canary) (bool, error) { - if cd.Status.CanaryRevision == "" || cd.Status.State == flaggerv1.CanaryRunning { + if cd.Status.LastAppliedSpec == "" || cd.Status.Phase == flaggerv1.CanaryProgressing { return true, nil } return c.IsNewSpec(cd) @@ -178,9 +178,9 @@ func (c *CanaryDeployer) SetFailedChecks(cd *flaggerv1.Canary, val int) error { } // SetState updates the canary status state -func (c *CanaryDeployer) SetState(cd *flaggerv1.Canary, state flaggerv1.CanaryState) error { +func (c *CanaryDeployer) SetState(cd *flaggerv1.Canary, state flaggerv1.CanaryPhase) error { cdCopy := cd.DeepCopy() - cdCopy.Status.State = state + cdCopy.Status.Phase = state cdCopy.Status.LastTransitionTime = metav1.Now() cd, err := c.flaggerClient.FlaggerV1alpha3().Canaries(cd.Namespace).UpdateStatus(cdCopy) @@ -206,9 +206,9 @@ func (c *CanaryDeployer) SyncStatus(cd *flaggerv1.Canary, status flaggerv1.Canar } cdCopy := cd.DeepCopy() - cdCopy.Status.State = status.State + cdCopy.Status.Phase = status.Phase cdCopy.Status.FailedChecks = status.FailedChecks - cdCopy.Status.CanaryRevision = base64.StdEncoding.EncodeToString(specJson) + cdCopy.Status.LastAppliedSpec = base64.StdEncoding.EncodeToString(specJson) cdCopy.Status.LastTransitionTime = metav1.Now() cd, err = c.flaggerClient.FlaggerV1alpha3().Canaries(cd.Namespace).UpdateStatus(cdCopy) @@ -247,7 +247,7 @@ func (c *CanaryDeployer) Sync(cd *flaggerv1.Canary) error { return fmt.Errorf("creating deployment %s.%s failed: %v", primaryName, cd.Namespace, err) } - if cd.Status.State == "" { + if cd.Status.Phase == "" { c.logger.Infof("Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) if err := c.Scale(cd, 0); err != nil { return err diff --git a/pkg/controller/deployer_test.go b/pkg/controller/deployer_test.go index 2f9ea9ea..83740761 100644 --- a/pkg/controller/deployer_test.go +++ b/pkg/controller/deployer_test.go @@ -387,7 +387,7 @@ func TestCanaryDeployer_SetState(t *testing.T) { t.Fatal(err.Error()) } - err = deployer.SetState(canary, v1alpha3.CanaryRunning) + err = deployer.SetState(canary, v1alpha3.CanaryProgressing) if err != nil { t.Fatal(err.Error()) } @@ -397,8 +397,8 @@ func TestCanaryDeployer_SetState(t *testing.T) { t.Fatal(err.Error()) } - if res.Status.State != v1alpha3.CanaryRunning { - t.Errorf("Got %v wanted %v", res.Status.State, v1alpha3.CanaryRunning) + if res.Status.Phase != v1alpha3.CanaryProgressing { + t.Errorf("Got %v wanted %v", res.Status.Phase, v1alpha3.CanaryProgressing) } } @@ -424,7 +424,7 @@ func TestCanaryDeployer_SyncStatus(t *testing.T) { } status := v1alpha3.CanaryStatus{ - State: v1alpha3.CanaryRunning, + Phase: v1alpha3.CanaryProgressing, FailedChecks: 2, } err = deployer.SyncStatus(canary, status) @@ -437,8 +437,8 @@ func TestCanaryDeployer_SyncStatus(t *testing.T) { t.Fatal(err.Error()) } - if res.Status.State != status.State { - t.Errorf("Got state %v wanted %v", res.Status.State, status.State) + if res.Status.Phase != status.Phase { + t.Errorf("Got state %v wanted %v", res.Status.Phase, status.Phase) } if res.Status.FailedChecks != status.FailedChecks { diff --git a/pkg/controller/recorder.go b/pkg/controller/recorder.go index 31b2444e..47e23a6e 100644 --- a/pkg/controller/recorder.go +++ b/pkg/controller/recorder.go @@ -72,8 +72,8 @@ func (cr *CanaryRecorder) SetTotal(namespace string, total int) { // SetStatus sets the last known canary analysis status func (cr *CanaryRecorder) SetStatus(cd *flaggerv1.Canary) { status := 1 - switch cd.Status.State { - case flaggerv1.CanaryRunning: + switch cd.Status.Phase { + case flaggerv1.CanaryProgressing: status = 0 case flaggerv1.CanaryFailed: status = 2 diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index e2e3672a..3c4fc766 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -137,7 +137,7 @@ func (c *Controller) advanceCanary(name string, namespace string) { } // check if the number of failed checks reached the threshold - if cd.Status.State == flaggerv1.CanaryRunning && + if cd.Status.Phase == flaggerv1.CanaryProgressing && (!retriable || cd.Status.FailedChecks >= cd.Spec.CanaryAnalysis.Threshold) { if cd.Status.FailedChecks >= cd.Spec.CanaryAnalysis.Threshold { @@ -173,7 +173,7 @@ func (c *Controller) advanceCanary(name string, namespace string) { } // mark canary as failed - if err := c.deployer.SyncStatus(cd, flaggerv1.CanaryStatus{State: flaggerv1.CanaryFailed}); err != nil { + if err := c.deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryFailed}); err != nil { c.logger.Errorf("%v", err) return } @@ -244,7 +244,7 @@ func (c *Controller) advanceCanary(name string, namespace string) { } // update status - if err := c.deployer.SetState(cd, flaggerv1.CanaryFinished); err != nil { + if err := c.deployer.SetState(cd, flaggerv1.CanarySucceeded); err != nil { c.recordEventWarningf(cd, "%v", err) return } @@ -256,12 +256,12 @@ func (c *Controller) advanceCanary(name string, namespace string) { func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary, deployer CanaryDeployer) bool { c.recorder.SetStatus(cd) - if cd.Status.State == flaggerv1.CanaryRunning { + if cd.Status.Phase == flaggerv1.CanaryProgressing { return true } - if cd.Status.State == "" { - if err := deployer.SyncStatus(cd, flaggerv1.CanaryStatus{State: flaggerv1.CanaryInitialized}); err != nil { + if cd.Status.Phase == "" { + if err := deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryInitialized}); err != nil { c.logger.Errorf("%v", err) return false } @@ -280,7 +280,7 @@ func (c *Controller) checkCanaryStatus(cd *flaggerv1.Canary, deployer CanaryDepl c.recordEventErrorf(cd, "%v", err) return false } - if err := deployer.SyncStatus(cd, flaggerv1.CanaryStatus{State: flaggerv1.CanaryRunning}); err != nil { + if err := deployer.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryProgressing}); err != nil { c.logger.Errorf("%v", err) return false } diff --git a/pkg/controller/scheduler_test.go b/pkg/controller/scheduler_test.go index e764f9b4..b796cf5f 100644 --- a/pkg/controller/scheduler_test.go +++ b/pkg/controller/scheduler_test.go @@ -194,7 +194,7 @@ func TestScheduler_Rollback(t *testing.T) { ctrl.advanceCanary("podinfo", "default") // update failed checks to max - err := deployer.SyncStatus(canary, v1alpha3.CanaryStatus{State: v1alpha3.CanaryRunning, FailedChecks: 11}) + err := deployer.SyncStatus(canary, v1alpha3.CanaryStatus{Phase: v1alpha3.CanaryProgressing, FailedChecks: 11}) if err != nil { t.Fatal(err.Error()) } @@ -207,7 +207,7 @@ func TestScheduler_Rollback(t *testing.T) { t.Fatal(err.Error()) } - if c.Status.State != v1alpha3.CanaryFailed { - t.Errorf("Got canary state %v wanted %v", c.Status.State, v1alpha3.CanaryFailed) + if c.Status.Phase != v1alpha3.CanaryFailed { + t.Errorf("Got canary state %v wanted %v", c.Status.Phase, v1alpha3.CanaryFailed) } }