From b25e12d45d6f14c2487ca5a9f459627bf392267b Mon Sep 17 00:00:00 2001 From: miguelvalerio Date: Thu, 6 Jul 2023 10:26:52 +0100 Subject: [PATCH] fix initial deployment downtime Signed-off-by: miguelvalerio --- pkg/canary/daemonset_controller.go | 10 ++-------- pkg/canary/deployment_controller.go | 9 +-------- pkg/controller/scheduler.go | 10 ++++++++++ pkg/controller/scheduler_daemonset_test.go | 7 ++++++- pkg/controller/scheduler_deployment_test.go | 7 ++++++- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go index 69ada2d50..026c243e4 100644 --- a/pkg/canary/daemonset_controller.go +++ b/pkg/canary/daemonset_controller.go @@ -91,8 +91,7 @@ func (c *DaemonSetController) ScaleFromZero(cd *flaggerv1.Canary) error { return nil } -// Initialize creates the primary DaemonSet, scales down the canary DaemonSet, -// and returns the pod selector label and container ports +// Initialize creates the primary DaemonSet if it does not exist. func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary) (err error) { err = c.createPrimaryDaemonSet(cd, c.includeLabelPrefix) if err != nil { @@ -105,13 +104,8 @@ func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary) (err error) { return fmt.Errorf("%w", err) } } - - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("Scaling down DaemonSet %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) - if err := c.ScaleToZero(cd); err != nil { - return fmt.Errorf("ScaleToZero failed: %w", err) - } } + return nil } diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go index f35072c5a..ad95426a9 100644 --- a/pkg/canary/deployment_controller.go +++ b/pkg/canary/deployment_controller.go @@ -45,8 +45,7 @@ type DeploymentController struct { includeLabelPrefix []string } -// Initialize creates the primary deployment, hpa, -// scales to zero the canary deployment and returns the pod selector label and container ports +// Initialize creates the primary deployment if it does not exist. func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) { if err := c.createPrimaryDeployment(cd, c.includeLabelPrefix); err != nil { return fmt.Errorf("createPrimaryDeployment failed: %w", err) @@ -58,12 +57,6 @@ func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) { return fmt.Errorf("%w", err) } } - - c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). - Infof("Scaling down Deployment %s.%s", cd.Spec.TargetRef.Name, cd.Namespace) - if err := c.ScaleToZero(cd); err != nil { - return fmt.Errorf("scaling down canary deployment %s.%s failed: %w", cd.Spec.TargetRef.Name, cd.Namespace, err) - } } return nil diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index 9dd8c0e06..d470d993d 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -248,6 +248,16 @@ func (c *Controller) advanceCanary(name string, namespace string) { return } + // scale down the canary target to 0 replicas after the service is pointing to the primary target + if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing { + c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)). + Infof("Scaling down %s %s.%s", cd.Spec.TargetRef.Kind, cd.Spec.TargetRef.Name, cd.Namespace) + if err := canaryController.ScaleToZero(cd); err != nil { + c.recordEventWarningf(cd, "scaling down canary %s %s.%s failed: %v", cd.Spec.TargetRef.Kind, cd.Spec.TargetRef.Name, cd.Namespace, err) + return + } + } + // take over an existing virtual service or ingress // runs after the primary is ready to ensure zero downtime if !strings.HasPrefix(provider, flaggerv1.AppMeshProvider) { diff --git a/pkg/controller/scheduler_daemonset_test.go b/pkg/controller/scheduler_daemonset_test.go index 1551a7bf0..1340e1e64 100644 --- a/pkg/controller/scheduler_daemonset_test.go +++ b/pkg/controller/scheduler_daemonset_test.go @@ -46,9 +46,14 @@ func TestScheduler_DaemonSetNewRevision(t *testing.T) { mocks := newDaemonSetFixture(nil) mocks.ctrl.advanceCanary("podinfo", "default") + // check if ScaleToZero was performed + ds, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + assert.Contains(t, ds.Spec.Template.Spec.NodeSelector, "flagger.app/scale-to-zero") + // update dae2 := newDaemonSetTestDaemonSetV2() - _, err := mocks.kubeClient.AppsV1().DaemonSets("default").Update(context.TODO(), dae2, metav1.UpdateOptions{}) + _, err = mocks.kubeClient.AppsV1().DaemonSets("default").Update(context.TODO(), dae2, metav1.UpdateOptions{}) require.NoError(t, err) // detect changes diff --git a/pkg/controller/scheduler_deployment_test.go b/pkg/controller/scheduler_deployment_test.go index 3095821f3..51161ec44 100644 --- a/pkg/controller/scheduler_deployment_test.go +++ b/pkg/controller/scheduler_deployment_test.go @@ -54,9 +54,14 @@ func TestScheduler_DeploymentNewRevision(t *testing.T) { // initialization done mocks.ctrl.advanceCanary("podinfo", "default") + // check if ScaleToZero was performed + dp, err := mocks.kubeClient.AppsV1().Deployments("default").Get(context.TODO(), "podinfo", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, int32(0), *dp.Spec.Replicas) + // update dep2 := newDeploymentTestDeploymentV2() - _, err := mocks.kubeClient.AppsV1().Deployments("default").Update(context.TODO(), dep2, metav1.UpdateOptions{}) + _, err = mocks.kubeClient.AppsV1().Deployments("default").Update(context.TODO(), dep2, metav1.UpdateOptions{}) require.NoError(t, err) // detect changes