Add canary finalizers

artifacts/flagger/crd.yaml

@@ -502,6 +502,9 @@ spec:
             skipAnalysis:
               description: Skip analysis and promote canary
               type: boolean
+            revertOnDeletion:
+              description: Revert mutated resources to original spec on deletion
+              type: boolean
             analysis:
               description: Canary analysis for this canary
               type: object
@@ -650,6 +653,8 @@ spec:
                 - Finalising
                 - Succeeded
                 - Failed
+                - Terminating
+                - Terminated
             canaryWeight:
               description: Traffic weight percentage routed to canary
               type: number

charts/flagger/crds/crd.yaml

@@ -502,6 +502,9 @@ spec:
             skipAnalysis:
               description: Skip analysis and promote canary
               type: boolean
+            revertOnDeletion:
+              description: Revert mutated resources to original spec on deletion
+              type: boolean
             analysis:
               description: Canary analysis for this canary
               type: object
@@ -650,6 +653,8 @@ spec:
                 - Finalising
                 - Succeeded
                 - Failed
+                - Terminating
+                - Terminated
             canaryWeight:
               description: Traffic weight percentage routed to canary
               type: number

docs/gitbook/usage/how-it-works.md

@@ -240,6 +240,32 @@ kubectl wait canary/podinfo --for=condition=promoted --timeout=5m
 kubectl get canary/podinfo | grep Succeeded
 ```
 
+### Canary finalizers
+
+The default behavior of Flagger on canary deletion is to leave resources that aren't owned by the controller 
+in their current state.  This simplifies the deletion action and avoids possible deadlocks during resource 
+finalization.  In the event the canary was introduced with existing resource(s) (i.e. service, virtual service, etc.),
+they would be mutated during the initialization phase and no longer reflect their initial state.  If the desired
+functionality upon deletion is to revert the resources to their initial state, the `revertOnDeletion` attribute
+can be enabled.  
+
+```yaml
+spec:
+  revertOnDeletion: true
+```
+
+When a deletion action is submitted to the cluster, Flagger will attempt to revert the following resources:
+
+* [Canary target](#canary-target) replicas will be updated to the primary replica count
+* [Canary service](#canary-service) selector will be reverted
+* Mesh/Ingress traffic routed to the target   
+
+The recommended approach to disable canary analysis would be utilization of the `skipAnalysis`
+attribute, which limits the need for resource reconciliation.  Utilizing the `revertOnDeletion` attribute should be 
+enabled when you no longer plan to rely on Flagger for deployment management.
+
+**Note** When this feature is enabled expect a delay in the delete action due to the reconciliation.  
+
 ### Canary analysis
 
 The canary analysis defines:

go.sum

@@ -322,4 +322,4 @@ modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs
 modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
 sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI=
 sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs=
-sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
+sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=

kustomize/base/flagger/crd.yaml

@@ -502,6 +502,9 @@ spec:
             skipAnalysis:
               description: Skip analysis and promote canary
               type: boolean
+            revertOnDeletion:
+              description: Revert mutated resources to original spec on deletion
+              type: boolean
             analysis:
               description: Canary analysis for this canary
               type: object
@@ -650,6 +653,8 @@ spec:
                 - Finalising
                 - Succeeded
                 - Failed
+                - Terminating
+                - Terminated
             canaryWeight:
               description: Traffic weight percentage routed to canary
               type: number

pkg/apis/flagger/v1beta1/canary.go

@@ -94,6 +94,10 @@ type CanarySpec struct {
 	// SkipAnalysis promotes the canary without analysing it
 	// +optional
 	SkipAnalysis bool `json:"skipAnalysis,omitempty"`
+
+	// revert canary mutation on deletion of canary resource
+	// +optional
+	RevertOnDeletion bool `json:"revertOnDeletion,omitempty"`
 }
 
 // CanaryService defines how ClusterIP services, service mesh or ingress routing objects are generated
@@ -298,7 +302,7 @@ type CanaryWebhook struct {
 	URL string `json:"url"`
 
 	// Request timeout for this webhook
-	Timeout string `json:"timeout"`
+	Timeout string `json:"timeout,omitempty"`
 
 	// Metadata (key-value pairs) for this webhook
 	// +optional

pkg/apis/flagger/v1beta1/status.go

@@ -57,6 +57,12 @@ const (
 	// CanaryPhaseFailed means the canary analysis failed
 	// and the canary deployment has been scaled to zero
 	CanaryPhaseFailed CanaryPhase = "Failed"
+	// CanaryPhaseTerminating means the canary has been marked
+	// for deletion and in the finalizing state
+	CanaryPhaseTerminating CanaryPhase = "Terminating"
+	// CanaryPhaseTerminated means the canary has been finalized
+	// and successfully deleted
+	CanaryPhaseTerminated CanaryPhase = "Terminated"
 )
 
 // CanaryStatus is used for state persistence (read-only)

pkg/canary/controller.go

@@ -19,4 +19,5 @@ type Controller interface {
 	HaveDependenciesChanged(canary *flaggerv1.Canary) (bool, error)
 	ScaleToZero(canary *flaggerv1.Canary) error
 	ScaleFromZero(canary *flaggerv1.Canary) error
+	Finalize(canary *flaggerv1.Canary) error
 }

pkg/canary/daemonset_controller.go

@@ -296,3 +296,12 @@ func (c *DaemonSetController) getSelectorLabel(daemonSet *appsv1.DaemonSet) (str
 func (c *DaemonSetController) HaveDependenciesChanged(cd *flaggerv1.Canary) (bool, error) {
 	return c.configTracker.HasConfigChanged(cd)
 }
+
+//Finalize scale the reference instance from zero
+func (c *DaemonSetController) Finalize(cd *flaggerv1.Canary) error {
+
+	if err := c.ScaleFromZero(cd); err != nil {
+		return err
+	}
+	return nil
+}

pkg/canary/daemonset_controller_test.go

@@ -194,3 +194,19 @@ func TestDaemonSetController_Scale(t *testing.T) {
 		}
 	})
 }
+
+func TestDaemonSetController_Finalize(t *testing.T) {
+	mocks := newDaemonSetFixture()
+	err := mocks.controller.Initialize(mocks.canary, true)
+	require.NoError(t, err)
+
+	err = mocks.controller.Finalize(mocks.canary)
+	require.NoError(t, err)
+
+	dep, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get("podinfo", metav1.GetOptions{})
+	require.NoError(t, err)
+
+	_, ok := dep.Spec.Template.Spec.NodeSelector["flagger.app/scale-to-zero"]
+
+	assert.False(t, ok)
+}

pkg/canary/deployment_controller.go

@@ -179,6 +179,27 @@ func (c *DeploymentController) ScaleFromZero(cd *flaggerv1.Canary) error {
 	return nil
 }
 
+// Scale sets the canary deployment replicas
+func (c *DeploymentController) Scale(cd *flaggerv1.Canary, replicas int32) error {
+	targetName := cd.Spec.TargetRef.Name
+	dep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(targetName, metav1.GetOptions{})
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return fmt.Errorf("deployment %s.%s not found", targetName, cd.Namespace)
+		}
+		return fmt.Errorf("deployment %s.%s query error %v", targetName, cd.Namespace, err)
+	}
+
+	depCopy := dep.DeepCopy()
+	depCopy.Spec.Replicas = int32p(replicas)
+
+	_, err = c.kubeClient.AppsV1().Deployments(dep.Namespace).Update(depCopy)
+	if err != nil {
+		return fmt.Errorf("scaling %s.%s to %v failed: %v", depCopy.GetName(), depCopy.Namespace, replicas, err)
+	}
+	return nil
+}
+
 // GetMetadata returns the pod label selector and svc ports
 func (c *DeploymentController) GetMetadata(cd *flaggerv1.Canary) (string, map[string]int32, error) {
 	targetName := cd.Spec.TargetRef.Name
@@ -376,3 +397,39 @@ func (c *DeploymentController) getSelectorLabel(deployment *appsv1.Deployment) (
 func (c *DeploymentController) HaveDependenciesChanged(cd *flaggerv1.Canary) (bool, error) {
 	return c.configTracker.HasConfigChanged(cd)
 }
+
+// Finalize will set the replica count from the primary to the reference instance.  This method is used
+// during a delete to attempt to revert the deployment back to the original state.  Error is returned if unable
+// update the reference deployment replicas to the primary replicas
+func (c *DeploymentController) Finalize(cd *flaggerv1.Canary) error {
+
+	primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
+
+	//Get ref deployment
+	refDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(cd.Spec.TargetRef.Name, metav1.GetOptions{})
+	if err != nil {
+		return err
+	}
+
+	//2. Get primary if possible, if not scale from zero
+	primaryDep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(primaryName, metav1.GetOptions{})
+	if err != nil {
+		if errors.IsNotFound(err) {
+			if err := c.ScaleFromZero(cd); err != nil {
+				return err
+			}
+			return nil
+		}
+		return err
+	}
+
+	//3. If both ref and primary present update the replicas of the ref to match the primary
+	if refDep.Spec.Replicas != primaryDep.Spec.Replicas {
+		//3. Set the replicas value on the original reference deployment
+		if err := c.Scale(cd, int32Default(primaryDep.Spec.Replicas)); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

pkg/canary/deployment_controller_test.go

@@ -188,3 +188,48 @@ func TestDeploymentController_HasTargetChanged(t *testing.T) {
 	require.NoError(t, err)
 	assert.True(t, isNew)
 }
+
+func TestDeploymentController_Finalize(t *testing.T) {
+
+	mocks := newDeploymentFixture()
+
+	tables := []struct {
+		mocks            deploymentControllerFixture
+		callInitialize   bool
+		shouldError      bool
+		expectedReplicas int32
+		canary           *flaggerv1.Canary
+	}{
+		//Primary not found returns error
+		{mocks, false, false, 1, mocks.canary},
+		//Happy path
+		{mocks, true, false, 1, mocks.canary},
+	}
+
+	for _, table := range tables {
+		if table.callInitialize {
+			err := mocks.controller.Initialize(table.canary, true)
+			if err != nil {
+				t.Fatal(err.Error())
+			}
+		}
+
+		err := mocks.controller.Finalize(table.canary)
+
+		if table.shouldError && err == nil {
+			t.Error("Expected error while calling Finalize, but none was returned")
+		} else if !table.shouldError && err != nil {
+			t.Errorf("Expected no error would be returned while calling Finalize, but returned %s", err)
+		}
+
+		if table.expectedReplicas > 0 {
+			c, err := mocks.kubeClient.AppsV1().Deployments(mocks.canary.Namespace).Get(mocks.canary.Name, metav1.GetOptions{})
+			if err != nil {
+				t.Fatal(err.Error())
+			}
+			if int32Default(c.Spec.Replicas) != table.expectedReplicas {
+				t.Errorf("Expected replicas %d recieved replicas %d", table.expectedReplicas, c.Spec.Replicas)
+			}
+		}
+	}
+}

pkg/canary/service_controller.go

@@ -221,3 +221,7 @@ func (c *ServiceController) IsPrimaryReady(_ *flaggerv1.Canary) error {
 func (c *ServiceController) IsCanaryReady(_ *flaggerv1.Canary) (bool, error) {
 	return true, nil
 }
+
+func (c *ServiceController) Finalize(cd *flaggerv1.Canary) error {
+	return nil
+}

pkg/controller/controller.go

@@ -128,6 +128,21 @@ func NewController(
 				}
 
 				ctrl.enqueue(new)
+			} else if !newCanary.DeletionTimestamp.IsZero() && hasFinalizer(&newCanary, finalizer) ||
+				!hasFinalizer(&newCanary, finalizer) && newCanary.Spec.RevertOnDeletion {
+				//If this was marked for deletion and has finalizers enqueue for finalizing or
+				//If this canary doesn't have finalizers and RevertOnDeletion is true updated speck enqueue
+				ctrl.enqueue(new)
+			}
+
+			//If canary no longer desires reverting, finalizers should be removed
+			if oldCanary.Spec.RevertOnDeletion && !newCanary.Spec.RevertOnDeletion {
+				ctrl.logger.Infof("%s.%s opting out, deleting finalizers", newCanary.Name, newCanary.Namespace)
+				err := ctrl.removeFinalizer(&newCanary, finalizer)
+				if err != nil {
+					ctrl.logger.Warnf("Failed to remove finalizers for %s.%s", oldCanary.Name, oldCanary.Namespace)
+					return
+				}
 			}
 		},
 		DeleteFunc: func(old interface{}) {
@@ -217,6 +232,33 @@ func (c *Controller) syncHandler(key string) error {
 		return nil
 	}
 
+	//Finalize if canary has been marked for deletion and revert is desired
+	if cd.Spec.RevertOnDeletion && cd.ObjectMeta.DeletionTimestamp != nil {
+
+		//If finalizers have been previously removed proceed
+		if !hasFinalizer(cd, finalizer) {
+			c.logger.Infof("Canary %s.%s has been finalized", cd.Name, cd.Namespace)
+			return nil
+		}
+
+		if cd.Status.Phase != flaggerv1.CanaryPhaseTerminated {
+			if err := c.finalize(cd); err != nil {
+				return fmt.Errorf("unable to finalize to canary %s.%s error %s", cd.Name, cd.Namespace, err)
+			}
+		}
+
+		//Remove finalizer from Canary
+		if err := c.removeFinalizer(cd, finalizer); err != nil {
+			return fmt.Errorf("unable to remove finalizer for canary %s.%s", cd.Name, cd.Namespace)
+		}
+
+		//record event
+		c.recordEventInfof(cd, "Terminated canary %s.%s", cd.Name, cd.Namespace)
+
+		c.logger.Infof("Canary %s.%s has been successfully processed and marked for deletion", cd.Name, cd.Namespace)
+		return nil
+	}
+
 	// set status condition for new canaries
 	if cd.Status.Conditions == nil {
 		if ok, conditions := canary.MakeStatusConditions(cd, flaggerv1.CanaryPhaseInitializing); ok {
@@ -233,6 +275,14 @@ func (c *Controller) syncHandler(key string) error {
 	}
 
 	c.canaries.Store(fmt.Sprintf("%s.%s", cd.Name, cd.Namespace), cd)
+
+	//If opt in for revertOnDeletion add finaliers if not present
+	if cd.Spec.RevertOnDeletion && !hasFinalizer(cd, finalizer) {
+		if err := c.addFinalizer(cd, finalizer); err != nil {
+			return fmt.Errorf("unable to add finalizer to canary %s.%s", cd.Name, cd.Namespace)
+		}
+
+	}
 	c.logger.Infof("Synced %s", key)
 
 	return nil