diff --git a/go.mod b/go.mod index 1649ca6b1..eeacba361 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/aws/aws-sdk-go v1.29.29 github.com/davecgh/go-spew v1.1.1 github.com/google/go-cmp v0.4.0 + github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.5.1 github.com/stretchr/testify v1.5.1 go.uber.org/zap v1.14.1 diff --git a/pkg/canary/daemonset_ready.go b/pkg/canary/daemonset_ready.go index 1c5a392d8..a8790ba9c 100644 --- a/pkg/canary/daemonset_ready.go +++ b/pkg/canary/daemonset_ready.go @@ -44,21 +44,31 @@ func (c *DaemonSetController) IsCanaryReady(cd *flaggerv1.Canary) (bool, error) } // isDaemonSetReady determines if a daemonset is ready by checking the number of old version daemons +// reference: https://github.com/kubernetes/kubernetes/blob/5232ad4a00ec93942d0b2c6359ee6cd1201b46bc/pkg/kubectl/rollout_status.go#L110 func (c *DaemonSetController) isDaemonSetReady(cd *flaggerv1.Canary, daemonSet *appsv1.DaemonSet) (bool, error) { - if diff := daemonSet.Status.DesiredNumberScheduled - daemonSet.Status.UpdatedNumberScheduled; diff > 0 || daemonSet.Status.NumberUnavailable > 0 { + if daemonSet.Generation <= daemonSet.Status.ObservedGeneration { + // calculate conditions + newCond := daemonSet.Status.UpdatedNumberScheduled < daemonSet.Status.DesiredNumberScheduled + availableCond := daemonSet.Status.NumberAvailable < daemonSet.Status.DesiredNumberScheduled + if !newCond && !availableCond { + return true, nil + } + + // check if deadline exceeded from := cd.Status.LastTransitionTime delta := time.Duration(cd.GetProgressDeadlineSeconds()) * time.Second - dl := from.Add(delta) - if dl.Before(time.Now()) { + if from.Add(delta).Before(time.Now()) { return false, fmt.Errorf("exceeded its progressDeadlineSeconds: %d", cd.GetProgressDeadlineSeconds()) - } else { - return true, fmt.Errorf( - "waiting for rollout to finish: desiredNumberScheduled=%d, updatedNumberScheduled=%d, numberUnavailable=%d", - daemonSet.Status.DesiredNumberScheduled, - daemonSet.Status.UpdatedNumberScheduled, - daemonSet.Status.NumberUnavailable, - ) + } + + // retryable + if newCond { + return true, fmt.Errorf("waiting for rollout to finish: %d out of %d new pods have been updated", + daemonSet.Status.UpdatedNumberScheduled, daemonSet.Status.DesiredNumberScheduled) + } else if availableCond { + return true, fmt.Errorf("waiting for rollout to finish: %d of %d updated pods are available", + daemonSet.Status.NumberAvailable, daemonSet.Status.DesiredNumberScheduled) } } - return true, nil + return true, fmt.Errorf("waiting for rollout to finish: observed daemonset generation less then desired generation") } diff --git a/pkg/canary/daemonset_ready_test.go b/pkg/canary/daemonset_ready_test.go index 76a810cf8..343e73b10 100644 --- a/pkg/canary/daemonset_ready_test.go +++ b/pkg/canary/daemonset_ready_test.go @@ -1,6 +1,7 @@ package canary import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -24,39 +25,57 @@ func TestDaemonSetController_IsReady(t *testing.T) { } func TestDaemonSetController_isDaemonSetReady(t *testing.T) { - ds := &appsv1.DaemonSet{ - Status: appsv1.DaemonSetStatus{ - DesiredNumberScheduled: 1, - UpdatedNumberScheduled: 1, - }, - } - + mocks := newDaemonSetFixture() cd := &flaggerv1.Canary{} - cd.Spec.ProgressDeadlineSeconds = int32p(1e5) - cd.Status.LastTransitionTime = metav1.Now() - // ready - mocks := newDaemonSetFixture() - _, err := mocks.controller.isDaemonSetReady(cd, ds) + // observed generation is less than desired generation + ds := &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{}} + ds.Status.ObservedGeneration-- + retyable, err := mocks.controller.isDaemonSetReady(cd, ds) + require.Error(t, err) + require.True(t, retyable) + + // succeeded + ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ + UpdatedNumberScheduled: 1, + DesiredNumberScheduled: 1, + NumberAvailable: 1, + }} + retyable, err = mocks.controller.isDaemonSetReady(cd, ds) require.NoError(t, err) + require.True(t, retyable) - // not ready but retriable - ds.Status.NumberUnavailable++ - retrieable, err := mocks.controller.isDaemonSetReady(cd, ds) + // deadline exceeded + ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ + UpdatedNumberScheduled: 0, + DesiredNumberScheduled: 1, + }} + cd.Status.LastTransitionTime = metav1.Now() + cd.Spec.ProgressDeadlineSeconds = int32p(-1e6) + retyable, err = mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.True(t, retrieable) - ds.Status.NumberUnavailable-- + require.False(t, retyable) - ds.Status.DesiredNumberScheduled++ - retrieable, err = mocks.controller.isDaemonSetReady(cd, ds) + // only newCond not satisfied + ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ + UpdatedNumberScheduled: 0, + DesiredNumberScheduled: 1, + NumberAvailable: 1, + }} + cd.Spec.ProgressDeadlineSeconds = int32p(1e6) + retyable, err = mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.True(t, retrieable) + require.True(t, retyable) + require.True(t, strings.Contains(err.Error(), "new pods")) - // not ready and not retriable - cd.Status.LastTransitionTime = metav1.Now() - cd.Spec.ProgressDeadlineSeconds = int32p(-1e5) - retrieable, err = mocks.controller.isDaemonSetReady(cd, ds) + // only availableCond not satisfied + ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ + UpdatedNumberScheduled: 1, + DesiredNumberScheduled: 1, + NumberAvailable: 0, + }} + retyable, err = mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.False(t, retrieable) - + require.True(t, retyable) + require.True(t, strings.Contains(err.Error(), "available")) }