Merge pull request #1414 from fluxcd/confirm-rollout

Run `confirm-rollout` checks only before scaling up deployment
fluxcd · May 8, 2023 · 15a6f74 · 15a6f74
2 parents 956daea + 495a5b2
commit 15a6f74
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 32 deletions.
diff --git 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
@@ -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