Skip to content

Commit 5b03840

Browse files
aryan9600aryan9600
authored
Merge pull request #1603 from fluxcd/fix-deploy-progress
scheduler: fail canary according to progress deadline
2 parents 1a27295 + 757d901 commit 5b03840

13 files changed

+63
-53
lines changed

‎pkg/canary/config_tracker_test.go

+1-1
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

+2-2
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

+6-6
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

+8-8
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

+6-6
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

+2-2
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

+3-3
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

+5-5
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

+3-2
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

+6-6
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

‎pkg/canary/deployment_ready_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func TestDeploymentController_IsReady(t *testing.T) {
3232
mocks := newDeploymentFixture(dc)
3333
mocks.controller.Initialize(mocks.canary)
3434

35-
err := mocks.controller.IsPrimaryReady(mocks.canary)
35+
_, err := mocks.controller.IsPrimaryReady(mocks.canary)
3636
require.Error(t, err)
3737

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

‎pkg/canary/service_controller.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -66,25 +66,25 @@ func (c *ServiceController) GetMetadata(_ *flaggerv1.Canary) (string, string, ma
6666
}
6767

6868
// Initialize creates or updates the primary and canary services to prepare for the canary release process targeted on the K8s service
69-
func (c *ServiceController) Initialize(cd *flaggerv1.Canary) (err error) {
69+
func (c *ServiceController) Initialize(cd *flaggerv1.Canary) (bool, error) {
7070
targetName := cd.Spec.TargetRef.Name
7171
primaryName := fmt.Sprintf("%s-primary", targetName)
7272
canaryName := fmt.Sprintf("%s-canary", targetName)
7373

7474
svc, err := c.kubeClient.CoreV1().Services(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{})
7575
if err != nil {
76-
return fmt.Errorf("service %s.%s get query error: %w", primaryName, cd.Namespace, err)
76+
return true, fmt.Errorf("service %s.%s get query error: %w", primaryName, cd.Namespace, err)
7777
}
7878

7979
if err = c.reconcileCanaryService(cd, canaryName, svc); err != nil {
80-
return fmt.Errorf("reconcileCanaryService failed: %w", err)
80+
return true, fmt.Errorf("reconcileCanaryService failed: %w", err)
8181
}
8282

8383
if err = c.reconcilePrimaryService(cd, primaryName, svc); err != nil {
84-
return fmt.Errorf("reconcilePrimaryService failed: %w", err)
84+
return true, fmt.Errorf("reconcilePrimaryService failed: %w", err)
8585
}
8686

87-
return nil
87+
return true, nil
8888
}
8989

9090
func (c *ServiceController) reconcileCanaryService(canary *flaggerv1.Canary, name string, src *corev1.Service) error {
@@ -249,8 +249,8 @@ func (c *ServiceController) HaveDependenciesChanged(_ *flaggerv1.Canary) (bool,
249249
return false, nil
250250
}
251251

252-
func (c *ServiceController) IsPrimaryReady(_ *flaggerv1.Canary) error {
253-
return nil
252+
func (c *ServiceController) IsPrimaryReady(_ *flaggerv1.Canary) (bool, error) {
253+
return true, nil
254254
}
255255

256256
func (c *ServiceController) IsCanaryReady(_ *flaggerv1.Canary) (bool, error) {

‎pkg/controller/scheduler.go

+13-4
Original file line numberDiff line numberDiff line change
@@ -221,9 +221,12 @@ func (c *Controller) advanceCanary(name string, namespace string) {
221221
}
222222

223223
// create primary workload
224-
err = canaryController.Initialize(cd)
224+
retriable, err := canaryController.Initialize(cd)
225225
if err != nil {
226226
c.recordEventWarningf(cd, "%v", err)
227+
if !retriable {
228+
c.rollback(cd, canaryController, meshRouter, scalerReconciler)
229+
}
227230
return
228231
}
229232

@@ -289,8 +292,12 @@ func (c *Controller) advanceCanary(name string, namespace string) {
289292

290293
// check primary status
291294
if !cd.SkipAnalysis() {
292-
if err := canaryController.IsPrimaryReady(cd); err != nil {
295+
retriable, err := canaryController.IsPrimaryReady(cd)
296+
if err != nil {
293297
c.recordEventWarningf(cd, "%v", err)
298+
if !retriable {
299+
c.rollback(cd, canaryController, meshRouter, scalerReconciler)
300+
}
294301
return
295302
}
296303
}
@@ -336,10 +343,12 @@ func (c *Controller) advanceCanary(name string, namespace string) {
336343
}
337344

338345
// check canary status
339-
var retriable = true
340346
retriable, err = canaryController.IsCanaryReady(cd)
341-
if err != nil && retriable {
347+
if err != nil {
342348
c.recordEventWarningf(cd, "%v", err)
349+
if !retriable {
350+
c.rollback(cd, canaryController, meshRouter, scalerReconciler)
351+
}
343352
return
344353
}
345354

0 commit comments

Comments
 (0)