Move from webhook.status to canary.status

Signed-off-by: Joseph Kwasniewski <[email protected]>
fluxcd · Oct 1, 2023 · c494f7e · c494f7e
1 parent 0df43ba
commit c494f7e
Show file tree

Hide file tree

Showing 14 changed files with 77 additions and 57 deletions.
diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml
@@ -1086,13 +1086,6 @@ spec:
                             type: object
                             additionalProperties:
                               type: string
-                          status:
-                            description: CanaryStatus defines the observed state of a canary.
-                            type: object
-                            properties:
-                              retries:
-                                description: Current number of retries the webhook has performed
-                                type: number
                           retries:
                             description: Number of retries before incrementing canary failure count
                             type: number
@@ -1142,6 +1135,14 @@ spec:
                   additionalProperties:
                     type: string
                   type: object
+                webhooks:
+                  description: Webhook status of this canary
+                  type: object
+                  additionalProperties:
+                    type: object
+                    properties:
+                      retries:
+                        type: number
                 lastAppliedSpec:
                   description: LastAppliedSpec of this canary
                   type: string

diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml
@@ -1086,13 +1086,6 @@ spec:
                             type: object
                             additionalProperties:
                               type: string
-                          status:
-                            description: CanaryStatus defines the observed state of a canary.
-                            type: object
-                            properties:
-                              retries:
-                                description: Current number of retries the webhook has performed
-                                type: number
                           retries:
                             description: Number of retries before incrementing canary failure count
                             type: number
@@ -1142,6 +1135,14 @@ spec:
                   additionalProperties:
                     type: string
                   type: object
+                webhooks:
+                  description: Webhook status of this canary
+                  type: object
+                  additionalProperties:
+                    type: object
+                    properties:
+                      retries:
+                        type: number
                 lastAppliedSpec:
                   description: LastAppliedSpec of this canary
                   type: string

diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml
@@ -1086,13 +1086,6 @@ spec:
                             type: object
                             additionalProperties:
                               type: string
-                          status:
-                            description: CanaryStatus defines the observed state of a canary.
-                            type: object
-                            properties:
-                              retries:
-                                description: Current number of retries the webhook has performed
-                                type: number
                           retries:
                             description: Number of retries before incrementing canary failure count
                             type: number
@@ -1142,6 +1135,14 @@ spec:
                   additionalProperties:
                     type: string
                   type: object
+                webhooks:
+                  description: Webhook status of this canary
+                  type: object
+                  additionalProperties:
+                    type: object
+                    properties:
+                      retries:
+                        type: number
                 lastAppliedSpec:
                   description: LastAppliedSpec of this canary
                   type: string

diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go
@@ -391,9 +391,6 @@ type CanaryWebhook struct {
 	// +optional
 	Metadata *map[string]string `json:"metadata,omitempty"`
 
-	// Canary Webhook Status
-	Status CanaryWebhookStatus `json:"status"`
-
 	// Retries for failing webhook before incrementing rollout failure threshold
 	// +optional
 	Retries int `json:"retries,omitempty"`

diff --git a/pkg/apis/flagger/v1beta1/status.go b/pkg/apis/flagger/v1beta1/status.go
@@ -87,9 +87,11 @@ type CanaryStatus struct {
 	LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
 	// +optional
 	Conditions []CanaryCondition `json:"conditions,omitempty"`
+	// +optional
+	Webhooks map[string]CanaryWebhookStatus `json:"webhooks,omitempty"`
 }
 
-// CanaryStatus is used for state persistence (read-only)
+// CanaryWebhookStatus is a status condition for a Canary Webhook
 type CanaryWebhookStatus struct {
 	Retries int `json:"retries"`
 }
diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go
diff --git a/pkg/canary/controller.go b/pkg/canary/controller.go
@@ -29,7 +29,7 @@ type Controller interface {
 	SetStatusWeight(canary *flaggerv1.Canary, val int) error
 	SetStatusIterations(canary *flaggerv1.Canary, val int) error
 	SetStatusPhase(canary *flaggerv1.Canary, phase flaggerv1.CanaryPhase) error
-	SetWebhookStatusRetries(canary *flaggerv1.Canary, webhook int, val int) error
+	SetStatusWebhookRetries(canary *flaggerv1.Canary, webhook string, val int) error
 	Initialize(canary *flaggerv1.Canary) error
 	Promote(canary *flaggerv1.Canary) error
 	HasTargetChanged(canary *flaggerv1.Canary) (bool, error)

diff --git a/pkg/canary/daemonset_status.go b/pkg/canary/daemonset_status.go
@@ -72,7 +72,7 @@ func (c *DaemonSetController) SetStatusPhase(cd *flaggerv1.Canary, phase flagger
 	return setStatusPhase(c.flaggerClient, cd, phase)
 }
 
-// SetWebhookStatusRetries updates the webhook retries counter
-func (c *DaemonSetController) SetWebhookStatusRetries(cd *flaggerv1.Canary, webhook int, val int) error {
-	return SetWebhookStatusRetries(c.flaggerClient, cd, webhook, val)
+// SetStatusWebhookRetries updates the webhook retries counter
+func (c *DaemonSetController) SetStatusWebhookRetries(cd *flaggerv1.Canary, webhook string, val int) error {
+	return SetStatusWebhookRetries(c.flaggerClient, cd, webhook, val)
 }
diff --git a/pkg/canary/deployment_status.go b/pkg/canary/deployment_status.go
@@ -62,7 +62,7 @@ func (c *DeploymentController) SetStatusPhase(cd *flaggerv1.Canary, phase flagge
 	return setStatusPhase(c.flaggerClient, cd, phase)
 }
 
-// SetWebhookStatusRetries updates the webhook retries counter
-func (c *DeploymentController) SetWebhookStatusRetries(cd *flaggerv1.Canary, webhook int, val int) error {
-	return SetWebhookStatusRetries(c.flaggerClient, cd, webhook, val)
+// SetStatusWebhookRetries updates the webhook retries counter
+func (c *DeploymentController) SetStatusWebhookRetries(cd *flaggerv1.Canary, webhook string, val int) error {
+	return SetStatusWebhookRetries(c.flaggerClient, cd, webhook, val)
 }
diff --git a/pkg/canary/service_controller.go b/pkg/canary/service_controller.go
@@ -60,9 +60,9 @@ func (c *ServiceController) SetStatusPhase(cd *flaggerv1.Canary, phase flaggerv1
 	return setStatusPhase(c.flaggerClient, cd, phase)
 }
 
-// SetWebhookStatusRetries updates the webhook retries counter
-func (c *ServiceController) SetWebhookStatusRetries(cd *flaggerv1.Canary, webhook int, val int) error {
-	return SetWebhookStatusRetries(c.flaggerClient, cd, webhook, val)
+// SetStatusWebhookRetries updates the webhook retries counter
+func (c *ServiceController) SetStatusWebhookRetries(cd *flaggerv1.Canary, webhook string, val int) error {
+	return SetStatusWebhookRetries(c.flaggerClient, cd, webhook, val)
 }
 
 // GetMetadata returns the pod label selector, label value and svc ports

diff --git a/pkg/canary/status.go b/pkg/canary/status.go
@@ -47,6 +47,7 @@ func syncCanaryStatus(flaggerClient clientset.Interface, cd *flaggerv1.Canary, s
 		cdCopy.Status.CanaryWeight = status.CanaryWeight
 		cdCopy.Status.FailedChecks = status.FailedChecks
 		cdCopy.Status.Iterations = status.Iterations
+		cdCopy.Status.Webhooks = status.Webhooks
 		cdCopy.Status.LastAppliedSpec = hash
 		if status.Phase == flaggerv1.CanaryPhaseInitialized {
 			cdCopy.Status.LastPromotedSpec = hash
@@ -185,7 +186,7 @@ func setStatusPhase(flaggerClient clientset.Interface, cd *flaggerv1.Canary, pha
 	return nil
 }
 
-func SetWebhookStatusRetries(flaggerClient clientset.Interface, cd *flaggerv1.Canary, webhook int, val int) error {
+func SetStatusWebhookRetries(flaggerClient clientset.Interface, cd *flaggerv1.Canary, webhook string, val int) error {
 	firstTry := true
 	name, ns := cd.GetName(), cd.GetNamespace()
 	err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) {
@@ -197,7 +198,11 @@ func SetWebhookStatusRetries(flaggerClient clientset.Interface, cd *flaggerv1.Ca
 		}
 
 		cdCopy := cd.DeepCopy()
-		cdCopy.GetAnalysis().Webhooks[webhook].Status.Retries = val
+		// initialize the map if it's nil
+		if cdCopy.Status.Webhooks == nil {
+			cdCopy.Status.Webhooks = make(map[string]flaggerv1.CanaryWebhookStatus)
+		}
+		cdCopy.Status.Webhooks[webhook] = flaggerv1.CanaryWebhookStatus{Retries: val}
 		cdCopy.Status.LastTransitionTime = metav1.Now()
 
 		err = updateStatusWithUpgrade(flaggerClient, cdCopy)

diff --git a/pkg/controller/scheduler.go b/pkg/controller/scheduler.go
@@ -735,25 +735,32 @@ func (c *Controller) runBlueGreen(canary *flaggerv1.Canary, canaryController can
 
 func (c *Controller) runAnalysisWebhooks(canary *flaggerv1.Canary, canaryController canary.Controller) string {
 	// run external checks
-	for id, webhook := range canary.GetAnalysis().Webhooks {
+	var retry bool
+	var failure bool
+	for _, webhook := range canary.GetAnalysis().Webhooks {
 		if webhook.Type == "" || webhook.Type == flaggerv1.RolloutHook {
 			err := CallWebhook(canary.Name, canary.Namespace, flaggerv1.CanaryPhaseProgressing, webhook)
 			if err != nil {
-				if webhook.Retries > 0 && webhook.Status.Retries < webhook.Retries {
-					if err := canaryController.SetWebhookStatusRetries(canary, id, webhook.Status.Retries+1); err != nil {
+				if webhook.Retries > 0 && canary.Status.Webhooks[webhook.Name].Retries < webhook.Retries {
+					if err := canaryController.SetStatusWebhookRetries(canary, webhook.Name, canary.Status.Webhooks[webhook.Name].Retries+1); err != nil {
 						c.recordEventWarningf(canary, "%v", err)
 					}
-					c.recordEventWarningf(canary, "Halt %s.%s advancement external check %s failed %v retrying %d more times",
-						canary.Name, canary.Namespace, webhook.Name, err, webhook.Retries-webhook.Status.Retries-1)
-					return "retry"
+					c.recordEventWarningf(canary, "Halt %s.%s advancement external check %s failed %v retries remaining %d",
+						canary.Name, canary.Namespace, webhook.Name, err, webhook.Retries-canary.Status.Webhooks[webhook.Name].Retries-1)
+					retry = true
+				} else {
+					c.recordEventWarningf(canary, "Halt %s.%s advancement external check %s failed %v",
+						canary.Name, canary.Namespace, webhook.Name, err)
+					failure = true
 				}
-				c.recordEventWarningf(canary, "Halt %s.%s advancement external check %s failed %v",
-					canary.Name, canary.Namespace, webhook.Name, err)
-				return "failure"
 			}
 		}
 	}
-
+	if failure {
+		return "failure"
+	} else if retry {
+		return "retry"
+	}
 	return "success"
 }
 

diff --git a/pkg/controller/scheduler_deployment_test.go b/pkg/controller/scheduler_deployment_test.go
@@ -702,16 +702,16 @@ func TestScheduler_DeploymentWebhookRetry(t *testing.T) {
 	// verify webhook retry counter increases
 	mocks.ctrl.advanceCanary("podinfo", "default")
 	// Failing URL 1, Success URL 0
-	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", 0, 1))
-	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", 1, 0))
+	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", "failing-url", 1))
+	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", "success-url", 0))
 	// Failing URL 2, Success URL 0
 	mocks.ctrl.advanceCanary("podinfo", "default")
-	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", 0, 2))
-	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", 1, 0))
+	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", "failing-url", 2))
+	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", "success-url", 0))
 	// Failing URL 3, Success URL 0
 	mocks.ctrl.advanceCanary("podinfo", "default")
-	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", 0, 3))
-	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", 1, 0))
+	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", "failing-url", 3))
+	require.NoError(t, assertWebhookRetries(mocks.flaggerClient, "podinfo", "success-url", 0))
 
 	// update failed checks to max
 	mocks.deployer.SyncStatus(mocks.canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseProgressing, FailedChecks: 10})

diff --git a/pkg/controller/scheduler_test.go b/pkg/controller/scheduler_test.go
@@ -60,14 +60,14 @@ func assertPhase(flaggerClient clientset.Interface, canary string, phase flagger
 	return nil
 }
 
-func assertWebhookRetries(flaggerClient clientset.Interface, canary string, webhook int, retries int) error {
+func assertWebhookRetries(flaggerClient clientset.Interface, canary string, webhook string, retries int) error {
 	c, err := flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), canary, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
 
-	if c.GetAnalysis().Webhooks[webhook].Status.Retries != retries {
-		return fmt.Errorf("got webhook status retries %d wanted %d", c.GetAnalysis().Webhooks[webhook].Status.Retries, retries)
+	if c.Status.Webhooks[webhook].Retries != retries {
+		return fmt.Errorf("got webhook status retries %d wanted %d %v", c.Status.Webhooks[webhook].Retries, retries, c.Status.Webhooks)
 	}
 
 	return nil