Prevent spurious canary rollouts using a server-side change fence

artifacts/flagger/crd.yaml

@@ -1276,6 +1276,9 @@ spec:
                 lastPromotedSpec:
                   description: LastPromotedSpec of this canary
                   type: string
+                lastTrackedRevision:
+                  description: LastTrackedRevision is the change fence of the canary target
+                  type: string
                 lastTransitionTime:
                   description: LastTransitionTime of this canary
                   format: date-time

charts/flagger/crds/crd.yaml

@@ -1276,6 +1276,9 @@ spec:
                 lastPromotedSpec:
                   description: LastPromotedSpec of this canary
                   type: string
+                lastTrackedRevision:
+                  description: LastTrackedRevision is the change fence of the canary target
+                  type: string
                 lastTransitionTime:
                   description: LastTransitionTime of this canary
                   format: date-time

kustomize/base/flagger/crd.yaml

@@ -1276,6 +1276,9 @@ spec:
                 lastPromotedSpec:
                   description: LastPromotedSpec of this canary
                   type: string
+                lastTrackedRevision:
+                  description: LastTrackedRevision is the change fence of the canary target
+                  type: string
                 lastTransitionTime:
                   description: LastTransitionTime of this canary
                   format: date-time

pkg/apis/flagger/v1beta1/status.go

@@ -85,6 +85,14 @@ type CanaryStatus struct {
 	LastAppliedSpec string `json:"lastAppliedSpec,omitempty"`
 	// +optional
 	LastPromotedSpec string `json:"lastPromotedSpec,omitempty"`
+	// LastTrackedRevision is the server-side change fence for the target workload,
+	// recorded as <uid>/<counter>[/<deployment-revision>]. The counter is
+	// metadata.generation for Deployments/DaemonSets and metadata.resourceVersion
+	// for Services. It identifies the exact object state that LastAppliedSpec
+	// was computed from, allowing hash drift caused by Flagger or cluster
+	// upgrades to be absorbed without triggering a canary analysis.
+	// +optional
+	LastTrackedRevision string `json:"lastTrackedRevision,omitempty"`
 	// +optional
 	LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
 	// +optional

pkg/canary/daemonset_controller.go

@@ -37,6 +37,26 @@ var (
 	daemonSetScaleDownNodeSelector = map[string]string{"flagger.app/scale-to-zero": "true"}
 )
 
+// daemonSetSnapshot returns the content hash of the daemonset pod template —
+// normalized so that Flagger's own scale-to-zero node selector does not
+// register as a spec change — together with its change fence.
+func daemonSetSnapshot(dae *appsv1.DaemonSet) (targetSnapshot, error) {
+	template := dae.Spec.Template.DeepCopy()
+	for key := range daemonSetScaleDownNodeSelector {
+		delete(template.Spec.NodeSelector, key)
+	}
+
+	hash, err := ComputeSpecHash(template)
+	if err != nil {
+		return targetSnapshot{}, fmt.Errorf("daemonset %s.%s: %w", dae.Name, dae.Namespace, err)
+	}
+
+	return targetSnapshot{
+		hash:  hash,
+		fence: encodeFence(dae.UID, fmt.Sprintf("%d", dae.Generation), ""),
+	}, nil
+}
+
 // DaemonSetController is managing the operations for Kubernetes DaemonSet kind
 type DaemonSetController struct {
 	kubeClient         kubernetes.Interface
@@ -65,13 +85,33 @@ func (c *DaemonSetController) ScaleToZero(cd *flaggerv1.Canary) error {
 		daeCopy.Spec.Template.Spec.NodeSelector[k] = v
 	}
 
-	_, err = c.kubeClient.AppsV1().DaemonSets(dae.Namespace).Update(context.TODO(), daeCopy, metav1.UpdateOptions{})
+	scaled, err := c.kubeClient.AppsV1().DaemonSets(dae.Namespace).Update(context.TODO(), daeCopy, metav1.UpdateOptions{})
 	if err != nil {
 		return fmt.Errorf("updating daemonset %s.%s failed: %w", daeCopy.GetName(), daeCopy.Namespace, err)
 	}
+	c.refreshTracking(cd, scaled)
 	return nil
 }
 
+// refreshTracking keeps the canary status fence current after Flagger's own
+// scaling writes, which advance the daemonset generation while leaving the
+// normalized pod template unchanged. The snapshot is taken from the object
+// returned by the write (not a re-read, which could observe a concurrent user
+// change); refreshTrackedRevision only moves the fence when the recorded
+// lastAppliedSpec still matches the template hash, so a concurrent template
+// change is never marked as seen. Failures are non-fatal: a stale fence
+// degrades to hash comparison, never to absorption.
+func (c *DaemonSetController) refreshTracking(cd *flaggerv1.Canary, dae *appsv1.DaemonSet) {
+	snap, err := daemonSetSnapshot(dae)
+	if err == nil {
+		err = refreshTrackedRevision(c.flaggerClient, cd, snap)
+	}
+	if err != nil {
+		c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).
+			Warnf("failed to refresh tracked revision: %v", err)
+	}
+}
+
 func (c *DaemonSetController) ScaleFromZero(cd *flaggerv1.Canary) error {
 	targetName := cd.Spec.TargetRef.Name
 	dep, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{})
@@ -84,6 +124,9 @@ func (c *DaemonSetController) ScaleFromZero(cd *flaggerv1.Canary) error {
 		delete(depCopy.Spec.Template.Spec.NodeSelector, k)
 	}
 
+	// no fence refresh here: this path is followed by a SyncStatus write in
+	// the scheduler, and a transiently stale fence self-heals through the
+	// fence-refresh rule on the next change detection
 	_, err = c.kubeClient.AppsV1().DaemonSets(dep.Namespace).Update(context.TODO(), depCopy, metav1.UpdateOptions{})
 	if err != nil {
 		return fmt.Errorf("scaling up daemonset %s.%s failed: %w", depCopy.GetName(), depCopy.Namespace, err)
@@ -192,17 +235,27 @@ func (c *DaemonSetController) HasTargetChanged(cd *flaggerv1.Canary) (bool, erro
 		return false, fmt.Errorf("daemonset %s.%s get query error: %w", targetName, cd.Namespace, err)
 	}
 
-	// ignore `daemonSetScaleDownNodeSelector` node selector
-	for key := range daemonSetScaleDownNodeSelector {
-		delete(canary.Spec.Template.Spec.NodeSelector, key)
+	snap, err := daemonSetSnapshot(canary)
+	if err != nil {
+		return false, err
 	}
 
-	// since nil and capacity zero map would have different hash, we have to initialize here
-	if canary.Spec.Template.Spec.NodeSelector == nil {
-		canary.Spec.Template.Spec.NodeSelector = map[string]string{}
-	}
+	return hasSpecChanged(c.logger, c.flaggerClient, cd, snap, func() (bool, error) {
+		return c.canaryImagesDiffer(cd, canary)
+	})
+}
 
-	return hasSpecChanged(cd, canary.Spec.Template)
+// canaryImagesDiffer compares the canary container images against the primary
+// ones; used as a safety heuristic when migrating the spec tracking from a
+// previous Flagger version, to catch rollouts applied while Flagger was not
+// running.
+func (c *DaemonSetController) canaryImagesDiffer(cd *flaggerv1.Canary, canary *appsv1.DaemonSet) (bool, error) {
+	primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
+	primary, err := c.kubeClient.AppsV1().DaemonSets(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{})
+	if err != nil {
+		return false, fmt.Errorf("daemonset %s.%s get query error: %w", primaryName, cd.Namespace, err)
+	}
+	return podImagesDiffer(canary.Spec.Template.Spec, primary.Spec.Template.Spec), nil
 }
 
 // GetMetadata returns the pod label selector and svc ports

pkg/canary/daemonset_status.go

@@ -32,22 +32,17 @@ func (c *DaemonSetController) SyncStatus(cd *flaggerv1.Canary, status flaggerv1.
 		return fmt.Errorf("daemonset %s.%s get query error: %w", cd.Spec.TargetRef.Name, cd.Namespace, err)
 	}
 
-	// ignore `daemonSetScaleDownNodeSelector` node selector
-	for key := range daemonSetScaleDownNodeSelector {
-		delete(dae.Spec.Template.Spec.NodeSelector, key)
-	}
-
-	// since nil and capacity zero map would have different hash, we have to initialize here
-	if dae.Spec.Template.Spec.NodeSelector == nil {
-		dae.Spec.Template.Spec.NodeSelector = map[string]string{}
-	}
-
 	configs, err := c.configTracker.GetConfigRefs(cd)
 	if err != nil {
 		return fmt.Errorf("GetConfigRefs failed: %w", err)
 	}
 
-	return syncCanaryStatus(c.flaggerClient, cd, status, dae.Spec.Template, func(cdCopy *flaggerv1.Canary) {
+	snap, err := daemonSetSnapshot(dae)
+	if err != nil {
+		return err
+	}
+
+	return syncCanaryStatus(c.flaggerClient, cd, status, snap, func(cdCopy *flaggerv1.Canary) {
 		cdCopy.Status.TrackedConfigs = configs
 	})
 }

pkg/canary/deployment_controller.go

@@ -44,6 +44,33 @@ type DeploymentController struct {
 	includeLabelPrefix []string
 }
 
+// deploymentRevisionAnnotation is set by the Kubernetes deployment controller
+// and incremented only when the pod template changes (a rollout).
+const deploymentRevisionAnnotation = "deployment.kubernetes.io/revision"
+
+// deploymentSnapshot returns the content hash of the deployment pod template
+// together with its change fence. The revision component is recorded only
+// while it provably corresponds to the current template: the revision
+// annotation is written asynchronously by the deployment controller, so it is
+// trusted only when rollouts are not paused and the controller has observed
+// the current generation.
+func deploymentSnapshot(dep *appsv1.Deployment) (targetSnapshot, error) {
+	hash, err := ComputeSpecHash(&dep.Spec.Template)
+	if err != nil {
+		return targetSnapshot{}, fmt.Errorf("deployment %s.%s: %w", dep.Name, dep.Namespace, err)
+	}
+
+	revision := ""
+	if !dep.Spec.Paused && dep.Status.ObservedGeneration == dep.Generation {
+		revision = dep.Annotations[deploymentRevisionAnnotation]
+	}
+
+	return targetSnapshot{
+		hash:  hash,
+		fence: encodeFence(dep.UID, fmt.Sprintf("%d", dep.Generation), revision),
+	}, nil
+}
+
 // Initialize creates the primary deployment if it does not exist.
 func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (bool, error) {
 	if err := c.createPrimaryDeployment(cd, c.includeLabelPrefix); err != nil {
@@ -144,7 +171,27 @@ func (c *DeploymentController) HasTargetChanged(cd *flaggerv1.Canary) (bool, err
 		return false, fmt.Errorf("deployment %s.%s get query error: %w", targetName, cd.Namespace, err)
 	}
 
-	return hasSpecChanged(cd, canary.Spec.Template)
+	snap, err := deploymentSnapshot(canary)
+	if err != nil {
+		return false, err
+	}
+
+	return hasSpecChanged(c.logger, c.flaggerClient, cd, snap, func() (bool, error) {
+		return c.canaryImagesDiffer(cd, canary)
+	})
+}
+
+// canaryImagesDiffer compares the canary container images against the primary
+// ones; used as a safety heuristic when migrating the spec tracking from a
+// previous Flagger version, to catch rollouts applied while Flagger was not
+// running.
+func (c *DeploymentController) canaryImagesDiffer(cd *flaggerv1.Canary, canary *appsv1.Deployment) (bool, error) {
+	primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
+	primary, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), primaryName, metav1.GetOptions{})
+	if err != nil {
+		return false, fmt.Errorf("deployment %s.%s get query error: %w", primaryName, cd.Namespace, err)
+	}
+	return podImagesDiffer(canary.Spec.Template.Spec, primary.Spec.Template.Spec), nil
 }
 
 // ScaleToZero Scale sets the canary deployment replicas
@@ -156,13 +203,33 @@ func (c *DeploymentController) ScaleToZero(cd *flaggerv1.Canary) error {
 	}
 
 	patch := []byte(fmt.Sprintf(`{"spec":{"replicas": %d}}`, 0))
-	_, err = c.kubeClient.AppsV1().Deployments(dep.Namespace).Patch(context.TODO(), dep.GetName(), types.MergePatchType, patch, metav1.PatchOptions{})
+	scaled, err := c.kubeClient.AppsV1().Deployments(dep.Namespace).Patch(context.TODO(), dep.GetName(), types.MergePatchType, patch, metav1.PatchOptions{})
 	if err != nil {
 		return fmt.Errorf("deployment %s.%s patch query error: %w", targetName, cd.Namespace, err)
 	}
+	c.refreshTracking(cd, scaled)
 	return nil
 }
 
+// refreshTracking keeps the canary status fence current after Flagger's own
+// scaling writes, which advance the deployment generation without changing
+// the pod template. The snapshot is taken from the object returned by the
+// write (not a re-read, which could observe a concurrent user change);
+// refreshTrackedRevision only moves the fence when the recorded
+// lastAppliedSpec still matches the template hash, so a concurrent template
+// change is never marked as seen. Failures are non-fatal: a stale fence
+// degrades to hash comparison, never to absorption.
+func (c *DeploymentController) refreshTracking(cd *flaggerv1.Canary, dep *appsv1.Deployment) {
+	snap, err := deploymentSnapshot(dep)
+	if err == nil {
+		err = refreshTrackedRevision(c.flaggerClient, cd, snap)
+	}
+	if err != nil {
+		c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).
+			Warnf("failed to refresh tracked revision: %v", err)
+	}
+}
+
 func (c *DeploymentController) ScaleFromZero(cd *flaggerv1.Canary) error {
 	targetName := cd.Spec.TargetRef.Name
 	dep, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(context.TODO(), targetName, metav1.GetOptions{})
@@ -203,6 +270,9 @@ func (c *DeploymentController) ScaleFromZero(cd *flaggerv1.Canary) error {
 	}
 
 	patch := []byte(fmt.Sprintf(`{"spec":{"replicas": %d}}`, *replicas))
+	// no fence refresh here: this path is followed by a SyncStatus write in
+	// the scheduler, and a transiently stale fence self-heals through the
+	// fence-refresh rule on the next change detection
 	_, err = c.kubeClient.AppsV1().Deployments(dep.Namespace).Patch(context.TODO(), dep.GetName(), types.MergePatchType, patch, metav1.PatchOptions{})
 	if err != nil {
 		return fmt.Errorf("scaling up %s.%s to %d failed: %v", dep.GetName(), dep.Namespace, *replicas, err)

pkg/canary/deployment_status.go

@@ -37,7 +37,12 @@ func (c *DeploymentController) SyncStatus(cd *flaggerv1.Canary, status flaggerv1
 		return fmt.Errorf("GetConfigRefs failed: %w", err)
 	}
 
-	return syncCanaryStatus(c.flaggerClient, cd, status, dep.Spec.Template, func(cdCopy *flaggerv1.Canary) {
+	snap, err := deploymentSnapshot(dep)
+	if err != nil {
+		return err
+	}
+
+	return syncCanaryStatus(c.flaggerClient, cd, status, snap, func(cdCopy *flaggerv1.Canary) {
 		cdCopy.Status.TrackedConfigs = configs
 	})
 }

pkg/canary/factory.go

@@ -77,6 +77,7 @@ func (factory *Factory) Controller(obj v1beta1.LocalObjectReference) Controller
 		includeLabelPrefix: factory.includeLabelPrefix,
 	}
 	knativeCtrl := &KnativeController{
+		logger:        factory.logger,
 		flaggerClient: factory.flaggerClient,
 		knativeClient: factory.knativeClient,
 	}

pkg/canary/knative_controller.go

@@ -58,13 +58,26 @@ func (kc *KnativeController) GetMetadata(canary *flaggerv1.Canary) (string, stri
 	return "", "", make(map[string]int32), nil
 }
 
+// knativeSnapshot returns the content hash of the latest created revision
+// name together with its change fence. The hash input lives in the service
+// status, which changes without bumping metadata.generation, so the fence
+// counter is the revision name itself: the fence then matches exactly when
+// the hash input is unchanged, making absorption safe for algorithm changes
+// only.
+func knativeSnapshot(service *serving.Service) targetSnapshot {
+	return targetSnapshot{
+		hash:  ComputeStringHash(service.Status.LatestCreatedRevisionName),
+		fence: encodeFence(service.UID, service.Status.LatestCreatedRevisionName, ""),
+	}
+}
+
 // SyncStatus encodes list of revisions and updates the canary status
 func (kc *KnativeController) SyncStatus(cd *flaggerv1.Canary, status flaggerv1.CanaryStatus) error {
 	service, err := kc.knativeClient.ServingV1().Services(cd.Namespace).Get(context.TODO(), cd.Spec.TargetRef.Name, metav1.GetOptions{})
 	if err != nil {
 		return fmt.Errorf("Knative Service %s.%s get query error: %w", cd.Spec.TargetRef.Name, cd.Namespace, err)
 	}
-	return syncCanaryStatus(kc.flaggerClient, cd, status, service.Status.LatestCreatedRevisionName, func(copy *flaggerv1.Canary) {})
+	return syncCanaryStatus(kc.flaggerClient, cd, status, knativeSnapshot(service), func(copy *flaggerv1.Canary) {})
 }
 
 // SetStatusFailedChecks updates the canary failed checks counter
@@ -141,7 +154,12 @@ func (kc *KnativeController) HasTargetChanged(cd *flaggerv1.Canary) (bool, error
 	if err != nil {
 		return true, fmt.Errorf("Knative Service %s.%s get query error: %w", cd.Spec.TargetRef.Name, cd.Namespace, err)
 	}
-	return hasSpecChanged(cd, service.Status.LatestCreatedRevisionName)
+	// the migration heuristic compares the latest revision with the promoted
+	// one recorded on the service, catching revisions created while Flagger
+	// was not running
+	return hasSpecChanged(kc.logger, kc.flaggerClient, cd, knativeSnapshot(service), func() (bool, error) {
+		return service.Status.LatestCreatedRevisionName != service.Annotations["flagger.app/primary-revision"], nil
+	})
 }
 
 func (kc *KnativeController) HaveDependenciesChanged(canary *flaggerv1.Canary) (bool, error) {