From 495a5b24f45b67caabe46012303c1c33bacc0d44 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Tue, 11 Apr 2023 17:48:44 +0530 Subject: [PATCH] run confirm-rollout checks only before scaling up Run the `confirm-rollout` webhook check right before scaling up the deployment only, instead of running it on every loop. Signed-off-by: Sanskar Jaiswal --- pkg/controller/scheduler.go | 52 ++++++++++++++++++++----------- pkg/controller/scheduler_hooks.go | 16 ++-------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go index ba45aca4..2aa6c9af 100644 --- a/pkg/controller/scheduler.go +++ b/pkg/controller/scheduler.go @@ -248,6 +248,12 @@ func (c *Controller) advanceCanary(name string, namespace string) { } } + // set canary phase to initialized and sync the status + if err = c.setPhaseInitialized(cd, canaryController); err != nil { + c.recordEventWarningf(cd, "%v", err) + return + } + // check for changes shouldAdvance, err := c.shouldAdvance(cd, canaryController) if err != nil { @@ -260,11 +266,6 @@ func (c *Controller) advanceCanary(name string, namespace string) { return } - // check gates - if isApproved := c.runConfirmRolloutHooks(cd, canaryController); !isApproved { - return - } - maxWeight := c.maxWeight(cd) // check primary status @@ -791,9 +792,7 @@ func (c *Controller) shouldSkipAnalysis(canary *flaggerv1.Canary, canaryControll } func (c *Controller) shouldAdvance(canary *flaggerv1.Canary, canaryController canary.Controller) (bool, error) { - if canary.Status.LastAppliedSpec == "" || - canary.Status.Phase == flaggerv1.CanaryPhaseInitializing || - canary.Status.Phase == flaggerv1.CanaryPhaseProgressing || + if canary.Status.Phase == flaggerv1.CanaryPhaseProgressing || canary.Status.Phase == flaggerv1.CanaryPhaseWaiting || canary.Status.Phase == flaggerv1.CanaryPhaseWaitingPromotion || canary.Status.Phase == flaggerv1.CanaryPhasePromoting || @@ -842,19 +841,12 @@ func (c *Controller) checkCanaryStatus(canary *flaggerv1.Canary, canaryControlle return false } - if canary.Status.Phase == "" || canary.Status.Phase == flaggerv1.CanaryPhaseInitializing { - if err := canaryController.SyncStatus(canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseInitialized}); err != nil { - c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)).Errorf("%v", err) + if shouldAdvance { + // check confirm-rollout gate + if isApproved := c.runConfirmRolloutHooks(canary, canaryController); !isApproved { return false } - c.recorder.SetStatus(canary, flaggerv1.CanaryPhaseInitialized) - c.recordEventInfof(canary, "Initialization done! %s.%s", canary.Name, canary.Namespace) - c.alert(canary, fmt.Sprintf("New %s detected, initialization completed.", canary.Spec.TargetRef.Kind), - true, flaggerv1.SeverityInfo) - return false - } - if shouldAdvance { canaryPhaseProgressing := canary.DeepCopy() canaryPhaseProgressing.Status.Phase = flaggerv1.CanaryPhaseProgressing c.recordEventInfof(canaryPhaseProgressing, "New revision detected! Scaling up %s.%s", canaryPhaseProgressing.Spec.TargetRef.Name, canaryPhaseProgressing.Namespace) @@ -941,6 +933,30 @@ func (c *Controller) rollback(canary *flaggerv1.Canary, canaryController canary. c.runPostRolloutHooks(canary, flaggerv1.CanaryPhaseFailed) } +func (c *Controller) setPhaseInitialized(cd *flaggerv1.Canary, canaryController canary.Controller) error { + if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing { + cd.Status.Phase = flaggerv1.CanaryPhaseInitialized + if err := canaryController.SyncStatus(cd, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseInitialized}); err != nil { + return fmt.Errorf("failed to sync canary %s.%s status: %w", cd.Name, cd.Namespace, err) + } + + canary, err := c.flaggerClient.FlaggerV1beta1().Canaries(cd.Namespace).Get(context.TODO(), cd.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to get canary %s.%s: %w", cd.Name, cd.Namespace, err) + } + // We need to sync the LastAppliedSpec and TrackedConfigs of the `cd` Canary object as it + // is used later to determine whether target revision has changed in `shouldAdvance()`. + cd.Status.LastAppliedSpec = canary.Status.LastAppliedSpec + cd.Status.TrackedConfigs = canary.Status.TrackedConfigs + + c.recorder.SetStatus(cd, flaggerv1.CanaryPhaseInitialized) + c.recordEventInfof(cd, "Initialization done! %s.%s", cd.Name, cd.Namespace) + c.alert(cd, fmt.Sprintf("New %s detected, initialization completed.", cd.Spec.TargetRef.Kind), + true, flaggerv1.SeverityInfo) + } + return nil +} + func (c *Controller) setPhaseInitializing(cd *flaggerv1.Canary) error { phase := flaggerv1.CanaryPhaseInitializing firstTry := true diff --git a/pkg/controller/scheduler_hooks.go b/pkg/controller/scheduler_hooks.go index 7d2349be..b45ebbd0 100644 --- a/pkg/controller/scheduler_hooks.go +++ b/pkg/controller/scheduler_hooks.go @@ -44,7 +44,7 @@ func (c *Controller) runConfirmTrafficIncreaseHooks(canary *flaggerv1.Canary) bo func (c *Controller) runConfirmRolloutHooks(canary *flaggerv1.Canary, canaryController canary.Controller) bool { for _, webhook := range canary.GetAnalysis().Webhooks { if webhook.Type == flaggerv1.ConfirmRolloutHook { - err := CallWebhook(canary.Name, canary.Namespace, flaggerv1.CanaryPhaseProgressing, webhook) + err := CallWebhook(canary.Name, canary.Namespace, canary.Status.Phase, webhook) if err != nil { if canary.Status.Phase != flaggerv1.CanaryPhaseWaiting { if err := canaryController.SetStatusPhase(canary, flaggerv1.CanaryPhaseWaiting); err != nil { @@ -57,20 +57,8 @@ func (c *Controller) runConfirmRolloutHooks(canary *flaggerv1.Canary, canaryCont } } return false - } else { - if canary.Status.Phase == flaggerv1.CanaryPhaseWaiting { - if err := canaryController.SetStatusPhase(canary, flaggerv1.CanaryPhaseProgressing); err != nil { - c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)).Errorf("%v", err) - return false - } - if err := canaryController.ScaleFromZero(canary); err != nil { - c.recordEventErrorf(canary, "%v", err) - return false - } - c.recordEventInfof(canary, "Confirm-rollout check %s passed", webhook.Name) - return false - } } + c.recordEventInfof(canary, "Confirm-rollout check %s passed", webhook.Name) } } return true