Skip to content

Commit 757d901

Browse files
aryan9600aryan9600
committed
scheduler: fail canary according to progress deadline
Modify `canary.IsPrimaryReady()` and `canary.Initialize()` to return a boolean indicating if the error is retriable. Modify the scheduler to rollback the analysis and mark the Canary object as failed if the above two functions or `canary.IsCanaryRead()` returns false along with an error. Signed-off-by: Sanskar Jaiswal <[email protected]>
1 parent 1a27295 commit 757d901

13 files changed

Lines changed: 63 additions & 53 deletions

‎pkg/canary/config_tracker_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func TestConfigTracker_ConfigMaps(t *testing.T) {
124124
configMap := newDaemonSetControllerTestConfigMap()
125125
configMapProjected := newDaemonSetControllerTestConfigProjected()
126126

127-
err := mocks.controller.Initialize(mocks.canary)
127+
_, err := mocks.controller.Initialize(mocks.canary)
128128
require.NoError(t, err)
129129

130130
daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})

‎pkg/canary/controller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ import (
2121
)
2222

2323
type Controller interface {
24-
IsPrimaryReady(canary *flaggerv1.Canary) error
24+
IsPrimaryReady(canary *flaggerv1.Canary) (bool, error)
2525
IsCanaryReady(canary *flaggerv1.Canary) (bool, error)
2626
GetMetadata(canary *flaggerv1.Canary) (string, string, map[string]int32, error)
2727
SyncStatus(canary *flaggerv1.Canary, status flaggerv1.CanaryStatus) error
2828
SetStatusFailedChecks(canary *flaggerv1.Canary, val int) error
2929
SetStatusWeight(canary *flaggerv1.Canary, val int) error
3030
SetStatusIterations(canary *flaggerv1.Canary, val int) error
3131
SetStatusPhase(canary *flaggerv1.Canary, phase flaggerv1.CanaryPhase) error
32-
Initialize(canary *flaggerv1.Canary) error
32+
Initialize(canary *flaggerv1.Canary) (bool, error)
3333
Promote(canary *flaggerv1.Canary) error
3434
HasTargetChanged(canary *flaggerv1.Canary) (bool, error)
3535
HaveDependenciesChanged(canary *flaggerv1.Canary) (bool, error)

‎pkg/canary/daemonset_controller.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,21 @@ func (c *DaemonSetController) ScaleFromZero(cd *flaggerv1.Canary) error {
9292
}
9393

9494
// Initialize creates the primary DaemonSet if it does not exist.
95-
func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary) (err error) {
96-
err = c.createPrimaryDaemonSet(cd, c.includeLabelPrefix)
95+
func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary) (bool, error) {
96+
err := c.createPrimaryDaemonSet(cd, c.includeLabelPrefix)
9797
if err != nil {
98-
return fmt.Errorf("createPrimaryDaemonSet failed: %w", err)
98+
return true, fmt.Errorf("createPrimaryDaemonSet failed: %w", err)
9999
}
100100

101101
if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing {
102102
if !cd.SkipAnalysis() {
103-
if err := c.IsPrimaryReady(cd); err != nil {
104-
return fmt.Errorf("%w", err)
103+
if retriable, err := c.IsPrimaryReady(cd); err != nil {
104+
return retriable, fmt.Errorf("%w", err)
105105
}
106106
}
107107
}
108108

109-
return nil
109+
return true, nil
110110
}
111111

112112
// Promote copies the pod spec, secrets and config maps from canary to primary

‎pkg/canary/daemonset_controller_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
func TestDaemonSetController_Sync_ConsistentNaming(t *testing.T) {
3535
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
3636
mocks := newDaemonSetFixture(dc)
37-
err := mocks.controller.Initialize(mocks.canary)
37+
_, err := mocks.controller.Initialize(mocks.canary)
3838
require.NoError(t, err)
3939

4040
daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), fmt.Sprintf("%s-primary", dc.name), metav1.GetOptions{})
@@ -56,7 +56,7 @@ func TestDaemonSetController_Sync_ConsistentNaming(t *testing.T) {
5656
func TestDaemonSetController_Sync_InconsistentNaming(t *testing.T) {
5757
dc := daemonsetConfigs{name: "podinfo-service", label: "name", labelValue: "podinfo"}
5858
mocks := newDaemonSetFixture(dc)
59-
err := mocks.controller.Initialize(mocks.canary)
59+
_, err := mocks.controller.Initialize(mocks.canary)
6060
require.NoError(t, err)
6161

6262
daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), fmt.Sprintf("%s-primary", dc.name), metav1.GetOptions{})
@@ -75,7 +75,7 @@ func TestDaemonSetController_Sync_InconsistentNaming(t *testing.T) {
7575
func TestDaemonSetController_Promote(t *testing.T) {
7676
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
7777
mocks := newDaemonSetFixture(dc)
78-
err := mocks.controller.Initialize(mocks.canary)
78+
_, err := mocks.controller.Initialize(mocks.canary)
7979
require.NoError(t, err)
8080

8181
dae2 := newDaemonSetControllerTestPodInfoV2()
@@ -116,7 +116,7 @@ func TestDaemonSetController_NoConfigTracking(t *testing.T) {
116116
mocks := newDaemonSetFixture(dc)
117117
mocks.controller.configTracker = &NopTracker{}
118118

119-
err := mocks.controller.Initialize(mocks.canary)
119+
_, err := mocks.controller.Initialize(mocks.canary)
120120
require.NoError(t, err)
121121

122122
daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
@@ -132,7 +132,7 @@ func TestDaemonSetController_NoConfigTracking(t *testing.T) {
132132
func TestDaemonSetController_HasTargetChanged(t *testing.T) {
133133
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
134134
mocks := newDaemonSetFixture(dc)
135-
err := mocks.controller.Initialize(mocks.canary)
135+
_, err := mocks.controller.Initialize(mocks.canary)
136136
require.NoError(t, err)
137137

138138
// save last applied hash
@@ -221,7 +221,7 @@ func TestDaemonSetController_Scale(t *testing.T) {
221221
t.Run("ScaleToZero", func(t *testing.T) {
222222
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
223223
mocks := newDaemonSetFixture(dc)
224-
err := mocks.controller.Initialize(mocks.canary)
224+
_, err := mocks.controller.Initialize(mocks.canary)
225225
require.NoError(t, err)
226226

227227
err = mocks.controller.ScaleToZero(mocks.canary)
@@ -238,7 +238,7 @@ func TestDaemonSetController_Scale(t *testing.T) {
238238
t.Run("ScaleFromZeo", func(t *testing.T) {
239239
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
240240
mocks := newDaemonSetFixture(dc)
241-
err := mocks.controller.Initialize(mocks.canary)
241+
_, err := mocks.controller.Initialize(mocks.canary)
242242
require.NoError(t, err)
243243

244244
err = mocks.controller.ScaleFromZero(mocks.canary)
@@ -257,7 +257,7 @@ func TestDaemonSetController_Scale(t *testing.T) {
257257
func TestDaemonSetController_Finalize(t *testing.T) {
258258
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
259259
mocks := newDaemonSetFixture(dc)
260-
err := mocks.controller.Initialize(mocks.canary)
260+
_, err := mocks.controller.Initialize(mocks.canary)
261261
require.NoError(t, err)
262262

263263
err = mocks.controller.Finalize(mocks.canary)

‎pkg/canary/daemonset_ready.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,21 @@ import (
2929

3030
// IsPrimaryReady checks the primary daemonset status and returns an error if
3131
// the daemonset is in the middle of a rolling update
32-
func (c *DaemonSetController) IsPrimaryReady(cd *flaggerv1.Canary) error {
32+
func (c *DaemonSetController) IsPrimaryReady(cd *flaggerv1.Canary) (bool, error) {
3333
primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
3434
primary, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{})
3535
if err != nil {
36-
return fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err)
36+
return true, fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err)
3737
}
3838

39-
_, err = c.isDaemonSetReady(cd, primary, cd.GetAnalysisPrimaryReadyThreshold())
39+
retriable, err := c.isDaemonSetReady(cd, primary, cd.GetAnalysisPrimaryReadyThreshold())
4040
if err != nil {
41-
return fmt.Errorf("primary daemonset %s.%s not ready: %w", primaryName, cd.Namespace, err)
41+
return retriable, fmt.Errorf("primary daemonset %s.%s not ready: %w", primaryName, cd.Namespace, err)
4242
}
43-
return nil
43+
return true, nil
4444
}
4545

46-
// IsCanaryReady checks the primary daemonset and returns an error if
46+
// IsCanaryReady checks the canary daemonset and returns an error if
4747
// the daemonset is in the middle of a rolling update
4848
func (c *DaemonSetController) IsCanaryReady(cd *flaggerv1.Canary) (bool, error) {
4949
targetName := cd.Spec.TargetRef.Name

‎pkg/canary/daemonset_ready_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ import (
3030
func TestDaemonSetController_IsReady(t *testing.T) {
3131
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
3232
mocks := newDaemonSetFixture(dc)
33-
err := mocks.controller.Initialize(mocks.canary)
33+
_, err := mocks.controller.Initialize(mocks.canary)
3434
require.NoError(t, err)
3535

36-
err = mocks.controller.IsPrimaryReady(mocks.canary)
36+
_, err = mocks.controller.IsPrimaryReady(mocks.canary)
3737
require.NoError(t, err)
3838

3939
_, err = mocks.controller.IsCanaryReady(mocks.canary)

‎pkg/canary/daemonset_status_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
func TestDaemonSetController_SyncStatus(t *testing.T) {
3131
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
3232
mocks := newDaemonSetFixture(dc)
33-
err := mocks.controller.Initialize(mocks.canary)
33+
_, err := mocks.controller.Initialize(mocks.canary)
3434
require.NoError(t, err)
3535

3636
status := flaggerv1.CanaryStatus{
@@ -55,7 +55,7 @@ func TestDaemonSetController_SyncStatus(t *testing.T) {
5555
func TestDaemonSetController_SetFailedChecks(t *testing.T) {
5656
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
5757
mocks := newDaemonSetFixture(dc)
58-
err := mocks.controller.Initialize(mocks.canary)
58+
_, err := mocks.controller.Initialize(mocks.canary)
5959
require.NoError(t, err)
6060

6161
err = mocks.controller.SetStatusFailedChecks(mocks.canary, 1)
@@ -69,7 +69,7 @@ func TestDaemonSetController_SetFailedChecks(t *testing.T) {
6969
func TestDaemonSetController_SetState(t *testing.T) {
7070
dc := daemonsetConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
7171
mocks := newDaemonSetFixture(dc)
72-
err := mocks.controller.Initialize(mocks.canary)
72+
_, err := mocks.controller.Initialize(mocks.canary)
7373
require.NoError(t, err)
7474

7575
err = mocks.controller.SetStatusPhase(mocks.canary, flaggerv1.CanaryPhaseProgressing)

‎pkg/canary/deployment_controller.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,20 @@ type DeploymentController struct {
4444
}
4545

4646
// Initialize creates the primary deployment if it does not exist.
47-
func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) {
47+
func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (bool, error) {
4848
if err := c.createPrimaryDeployment(cd, c.includeLabelPrefix); err != nil {
49-
return fmt.Errorf("createPrimaryDeployment failed: %w", err)
49+
return true, fmt.Errorf("createPrimaryDeployment failed: %w", err)
5050
}
5151

5252
if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing {
5353
if !cd.SkipAnalysis() {
54-
if err := c.IsPrimaryReady(cd); err != nil {
55-
return fmt.Errorf("%w", err)
54+
if retriable, err := c.IsPrimaryReady(cd); err != nil {
55+
return retriable, fmt.Errorf("%w", err)
5656
}
5757
}
5858
}
5959

60-
return nil
60+
return true, nil
6161
}
6262

6363
// Promote copies the pod spec, secrets and config maps from canary to primary

‎pkg/canary/deployment_fixture_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type deploymentConfigs struct {
5555
}
5656

5757
func (d deploymentControllerFixture) initializeCanary(t *testing.T) {
58-
err := d.controller.Initialize(d.canary)
58+
_, err := d.controller.Initialize(d.canary)
5959
require.Error(t, err) // not ready yet
6060

6161
primaryName := fmt.Sprintf("%s-primary", d.canary.Spec.TargetRef.Name)
@@ -73,7 +73,8 @@ func (d deploymentControllerFixture) initializeCanary(t *testing.T) {
7373
_, err = d.controller.kubeClient.AppsV1().Deployments(d.canary.Namespace).Update(context.TODO(), p, metav1.UpdateOptions{})
7474
require.NoError(t, err)
7575

76-
require.NoError(t, d.controller.Initialize(d.canary))
76+
_, err = d.controller.Initialize(d.canary)
77+
require.NoError(t, err)
7778
}
7879

7980
func newDeploymentFixture(dc deploymentConfigs) deploymentControllerFixture {

‎pkg/canary/deployment_ready.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,23 +30,23 @@ import (
3030
// IsPrimaryReady checks the primary deployment status and returns an error if
3131
// the deployment is in the middle of a rolling update or if the pods are unhealthy
3232
// it will return a non retryable error if the rolling update is stuck
33-
func (c *DeploymentController) IsPrimaryReady(cd *flaggerv1.Canary) error {
33+
func (c *DeploymentController) IsPrimaryReady(cd *flaggerv1.Canary) (bool, error) {
3434
primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
3535
primary, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{})
3636
if err != nil {
37-
return fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err)
37+
return true, fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err)
3838
}
3939

40-
_, err = c.isDeploymentReady(primary, cd.GetProgressDeadlineSeconds(), cd.GetAnalysisPrimaryReadyThreshold())
40+
retriable, err := c.isDeploymentReady(primary, cd.GetProgressDeadlineSeconds(), cd.GetAnalysisPrimaryReadyThreshold())
4141
if err != nil {
42-
return fmt.Errorf("%s.%s not ready: %w", primaryName, cd.Namespace, err)
42+
return retriable, fmt.Errorf("%s.%s not ready: %w", primaryName, cd.Namespace, err)
4343
}
4444

4545
if primary.Spec.Replicas == int32p(0) {
46-
return fmt.Errorf("halt %s.%s advancement: primary deployment is scaled to zero",
46+
return false, fmt.Errorf("halt %s.%s advancement: primary deployment is scaled to zero",
4747
cd.Name, cd.Namespace)
4848
}
49-
return nil
49+
return true, nil
5050
}
5151

5252
// IsCanaryReady checks the canary deployment status and returns an error if

0 commit comments

Comments
 (0)