diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index 9accd818..1c07c7fc 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -253,12 +253,6 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary) error primaryDep, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(primaryName, metav1.GetOptions{}) if errors.IsNotFound(err) { - if cd.GetProgressDeadlineSeconds() > 0 { - // (@mathetake): should we? - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("progressDeadlineSeconds is ignored for DaemonSet") - } - // create primary secrets and config maps configRefs, err := c.configTracker.GetTargetConfigs(cd) if err != nil { diff --git a/pkg/canary/daemonset_fixture_test.go b/pkg/canary/daemonset_fixture_test.go index e706556f..d0919af0 100644 --- a/pkg/canary/daemonset_fixture_test.go +++ b/pkg/canary/daemonset_fixture_test.go @@ -1,7 +1,6 @@ package canary import ( - "github.com/weaveworks/flagger/pkg/logger" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -13,6 +12,7 @@ import ( flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" clientset "github.com/weaveworks/flagger/pkg/client/clientset/versioned" fakeFlagger "github.com/weaveworks/flagger/pkg/client/clientset/versioned/fake" + "github.com/weaveworks/flagger/pkg/logger" ) type daemonSetControllerFixture struct { diff --git a/pkg/canary/daemonset_ready.go b/pkg/canary/daemonset_ready.go index 2cee65c2..4120082a 100644 --- a/pkg/canary/daemonset_ready.go +++ b/pkg/canary/daemonset_ready.go @@ -2,6 +2,7 @@ package canary import ( "fmt" + "time" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -22,7 +23,7 @@ func (c *DaemonSetController) IsPrimaryReady(cd *flaggerv1.Canary) (bool, error) return true, fmt.Errorf("deployment %s.%s query error %v", primaryName, cd.Namespace, err) } - retriable, err := c.isDaemonSetReady(primary) + retriable, err := c.isDaemonSetReady(cd, primary) if err != nil { return retriable, fmt.Errorf("halt advancement %s.%s %s", primaryName, cd.Namespace, err.Error()) } @@ -41,7 +42,7 @@ func (c *DaemonSetController) IsCanaryReady(cd *flaggerv1.Canary) (bool, error) return true, fmt.Errorf("daemonset %s.%s query error %v", targetName, cd.Namespace, err) } - retriable, err := c.isDaemonSetReady(canary) + retriable, err := c.isDaemonSetReady(cd, canary) if err != nil { return retriable, fmt.Errorf("halt advancement %s.%s %s", targetName, cd.Namespace, err.Error()) } @@ -49,13 +50,21 @@ func (c *DaemonSetController) IsCanaryReady(cd *flaggerv1.Canary) (bool, error) } // isDaemonSetReady determines if a daemonset is ready by checking the number of old version daemons -func (c *DaemonSetController) isDaemonSetReady(daemonSet *appsv1.DaemonSet) (bool, error) { - if daemonSet.Generation <= daemonSet.Status.ObservedGeneration { - if diff := daemonSet.Status.DesiredNumberScheduled - daemonSet.Status.UpdatedNumberScheduled; diff > 0 { - return true, fmt.Errorf("waiting for rollout to finish: %d old daemons not replaced yet", diff) +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 { + from := cd.Status.LastTransitionTime + delta := time.Duration(cd.GetProgressDeadlineSeconds()) * time.Second + dl := from.Add(delta) + if dl.Before(time.Now()) { + return false, fmt.Errorf("daemonset %s exceeded its progress deadline", cd.GetName()) + } 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, + ) } - } else { - return true, fmt.Errorf("waiting for rollout to finish: observed daemonset generation less then desired generation") } return true, nil } diff --git a/pkg/canary/daemonset_ready_test.go b/pkg/canary/daemonset_ready_test.go index 00c98598..16b8f824 100644 --- a/pkg/canary/daemonset_ready_test.go +++ b/pkg/canary/daemonset_ready_test.go @@ -4,7 +4,9 @@ import ( "testing" appsv1 "k8s.io/api/apps/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" ) func TestDaemonSetController_IsReady(t *testing.T) { @@ -26,32 +28,52 @@ func TestDaemonSetController_IsReady(t *testing.T) { } func TestDaemonSetController_isDaemonSetReady(t *testing.T) { - mocks := newDaemonSetFixture() - _, err := mocks.controller.isDaemonSetReady(&appsv1.DaemonSet{ - ObjectMeta: v1.ObjectMeta{ - Generation: 10, - }, + ds := &appsv1.DaemonSet{ Status: appsv1.DaemonSetStatus{ - ObservedGeneration: 10, DesiredNumberScheduled: 1, UpdatedNumberScheduled: 1, }, - }) + } + + cd := &flaggerv1.Canary{} + cd.Spec.ProgressDeadlineSeconds = int32p(1e5) + cd.Status.LastTransitionTime = metav1.Now() + + // ready + mocks := newDaemonSetFixture() + _, err := mocks.controller.isDaemonSetReady(cd, ds) if err != nil { t.Fatal(err.Error()) } - _, err = mocks.controller.isDaemonSetReady(&appsv1.DaemonSet{ - ObjectMeta: v1.ObjectMeta{ - Generation: 9, - }, - Status: appsv1.DaemonSetStatus{ - ObservedGeneration: 10, - DesiredNumberScheduled: 2, - UpdatedNumberScheduled: 1, - }, - }) + // not ready but retriable + ds.Status.NumberUnavailable++ + retrieable, err := mocks.controller.isDaemonSetReady(cd, ds) if err == nil { t.Fatal("expected error") } + if !retrieable { + t.Fatal("expected retriable") + } + ds.Status.NumberUnavailable-- + + ds.Status.DesiredNumberScheduled++ + retrieable, err = mocks.controller.isDaemonSetReady(cd, ds) + if err == nil { + t.Fatal("expected error") + } + if !retrieable { + t.Fatal("expected retriable") + } + + // not ready and not retriable + cd.Status.LastTransitionTime = metav1.Now() + cd.Spec.ProgressDeadlineSeconds = int32p(-1e5) + retrieable, err = mocks.controller.isDaemonSetReady(cd, ds) + if err == nil { + t.Fatal("expected error") + } + if retrieable { + t.Fatal("expected not retriable") + } } diff --git a/pkg/canary/daemonset_status.go b/pkg/canary/daemonset_status.go index 6d4b338c..49329866 100644 --- a/pkg/canary/daemonset_status.go +++ b/pkg/canary/daemonset_status.go @@ -4,9 +4,10 @@ import ( "fmt" ex "github.com/pkg/errors" - flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" ) // SyncStatus encodes the canary pod spec and updates the canary status diff --git a/pkg/canary/daemonset_status_test.go b/pkg/canary/daemonset_status_test.go index d4af4e9a..9c4425f3 100644 --- a/pkg/canary/daemonset_status_test.go +++ b/pkg/canary/daemonset_status_test.go @@ -3,8 +3,9 @@ package canary import ( "testing" - flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" ) func TestDaemonSetController_SyncStatus(t *testing.T) { diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go index 20f20834..62e2a2b9 100644 --- a/pkg/canary/deployment_fixture_test.go +++ b/pkg/canary/deployment_fixture_test.go @@ -1,7 +1,6 @@ package canary import ( - "github.com/weaveworks/flagger/pkg/logger" "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" hpav2 "k8s.io/api/autoscaling/v2beta1" @@ -14,6 +13,7 @@ import ( flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" clientset "github.com/weaveworks/flagger/pkg/client/clientset/versioned" fakeFlagger "github.com/weaveworks/flagger/pkg/client/clientset/versioned/fake" + "github.com/weaveworks/flagger/pkg/logger" ) type deploymentControllerFixture struct { diff --git a/pkg/controller/scheduler_daemonset_fixture_test.go b/pkg/controller/scheduler_daemonset_fixture_test.go index 66c2fd15..8e0b2478 100644 --- a/pkg/controller/scheduler_daemonset_fixture_test.go +++ b/pkg/controller/scheduler_daemonset_fixture_test.go @@ -4,8 +4,6 @@ import ( "sync" "time" - "github.com/weaveworks/flagger/pkg/metrics/observers" - "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -25,6 +23,7 @@ import ( informers "github.com/weaveworks/flagger/pkg/client/informers/externalversions" "github.com/weaveworks/flagger/pkg/logger" "github.com/weaveworks/flagger/pkg/metrics" + "github.com/weaveworks/flagger/pkg/metrics/observers" "github.com/weaveworks/flagger/pkg/router" ) diff --git a/pkg/controller/scheduler_deployment_fixture_test.go b/pkg/controller/scheduler_deployment_fixture_test.go index 666a4ab8..a3f2db50 100644 --- a/pkg/controller/scheduler_deployment_fixture_test.go +++ b/pkg/controller/scheduler_deployment_fixture_test.go @@ -4,8 +4,6 @@ import ( "sync" "time" - "github.com/weaveworks/flagger/pkg/metrics/observers" - "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" hpav2 "k8s.io/api/autoscaling/v2beta1" @@ -26,6 +24,7 @@ import ( informers "github.com/weaveworks/flagger/pkg/client/informers/externalversions" "github.com/weaveworks/flagger/pkg/logger" "github.com/weaveworks/flagger/pkg/metrics" + "github.com/weaveworks/flagger/pkg/metrics/observers" "github.com/weaveworks/flagger/pkg/router" ) diff --git a/pkg/controller/webhook_test.go b/pkg/controller/webhook_test.go index 43143441..96750148 100644 --- a/pkg/controller/webhook_test.go +++ b/pkg/controller/webhook_test.go @@ -3,12 +3,13 @@ package controller import ( "encoding/json" "fmt" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1" "net/http" "net/http/httptest" "testing" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" )