Merge pull request #1412 from fluxcd/fix-postrenderers-digest

Fix postRenderers not causing new upgrade when applied during ongoing upgrade

Author: matheuscscp

internal/reconcile/atomic_release.go

@@ -38,9 +38,7 @@ import (
 	v2 "github.com/fluxcd/helm-controller/api/v2"
 	"github.com/fluxcd/helm-controller/internal/action"
 	"github.com/fluxcd/helm-controller/internal/diff"
-	"github.com/fluxcd/helm-controller/internal/digest"
 	interrors "github.com/fluxcd/helm-controller/internal/errors"
-	"github.com/fluxcd/helm-controller/internal/postrender"
 )
 
 // OwnedConditions is a list of Condition types owned by the HelmRelease object.
@@ -219,23 +217,10 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
 				conditions.Delete(req.Object, meta.ReconcilingCondition)
 
 				// Always summarize; this ensures we restore transient errors
-				// written to Ready.
+				// written to Ready, and updates the observed post-renderers
+				// and common-metadata digests.
 				summarize(req)
 
-				// remove stale post-renderers digest on successful reconciliation.
-				if conditions.IsReady(req.Object) {
-					req.Object.Status.ObservedPostRenderersDigest = ""
-					if req.Object.Spec.PostRenderers != nil {
-						// Update the post-renderers digest if the post-renderers exist.
-						req.Object.Status.ObservedPostRenderersDigest = postrender.Digest(digest.Canonical, req.Object.Spec.PostRenderers).String()
-					}
-					req.Object.Status.ObservedCommonMetadataDigest = ""
-					if req.Object.Spec.CommonMetadata != nil {
-						// Update the common-metadata digest if common-metadata exist.
-						req.Object.Status.ObservedCommonMetadataDigest = postrender.CommonMetadataDigest(digest.Canonical, req.Object.Spec.CommonMetadata).String()
-					}
-				}
-
 				return nil
 			}
 

internal/reconcile/atomic_release_test.go

@@ -1376,14 +1376,15 @@ func TestAtomicRelease_Reconcile_PostRenderers_Scenarios(t *testing.T) {
 			},
 			status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
 				return v2.HelmReleaseStatus{
+					ObservedGeneration: 2, // Matches obj.Generation to skip the digest check.
 					History: v2.Snapshots{
 						release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
 					},
 					Conditions: []metav1.Condition{
 						{
 							Type:               meta.ReadyCondition,
 							Status:             metav1.ConditionTrue,
-							ObservedGeneration: 2, // This is used to set processed config generation.
+							ObservedGeneration: 2,
 						},
 					},
 					ObservedPostRenderersDigest: postrender.Digest(digest.Canonical, postRenderers).String(),
@@ -2357,14 +2358,15 @@ func TestAtomicRelease_Reconcile_CommonMetadata_Scenarios(t *testing.T) {
 			},
 			status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
 				return v2.HelmReleaseStatus{
+					ObservedGeneration: 2, // Matches obj.Generation to skip the digest check.
 					History: v2.Snapshots{
 						release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
 					},
 					Conditions: []metav1.Condition{
 						{
 							Type:               meta.ReadyCondition,
 							Status:             metav1.ConditionTrue,
-							ObservedGeneration: 2, // This is used to set processed config generation.
+							ObservedGeneration: 2,
 						},
 					},
 					ObservedPostRenderersDigest: postrender.CommonMetadataDigest(digest.Canonical, commonMetadata).String(),

internal/reconcile/release.go

@@ -36,7 +36,9 @@ import (
 
 	v2 "github.com/fluxcd/helm-controller/api/v2"
 	"github.com/fluxcd/helm-controller/internal/action"
+	"github.com/fluxcd/helm-controller/internal/digest"
 	"github.com/fluxcd/helm-controller/internal/inventory"
+	"github.com/fluxcd/helm-controller/internal/postrender"
 	"github.com/fluxcd/helm-controller/internal/release"
 	"github.com/fluxcd/helm-controller/internal/storage"
 )
@@ -206,7 +208,8 @@ func observeInventory(obj *v2.HelmRelease, chart *helmchart.Chart, getter generi
 //
 // If Ready=True, any Stalled condition is removed.
 //
-// The ObservedPostRenderersDigest is updated if the post-renderers exist.
+// The ObservedPostRenderersDigest and ObservedCommonMetadataDigest are
+// updated to reflect the current spec.
 func summarize(req *Request) {
 	var sumConds = []string{v2.RemediatedCondition, v2.ReleasedCondition}
 	if req.Object.GetTest().Enable && !req.Object.GetTest().IgnoreFailures {
@@ -256,6 +259,17 @@ func summarize(req *Request) {
 		Message:            conds[0].Message,
 		ObservedGeneration: req.Object.Generation,
 	})
+
+	// Update the observed post-renderers and common-metadata digests to
+	// reflect the current spec.
+	req.Object.Status.ObservedPostRenderersDigest = ""
+	if req.Object.Spec.PostRenderers != nil {
+		req.Object.Status.ObservedPostRenderersDigest = postrender.Digest(digest.Canonical, req.Object.Spec.PostRenderers).String()
+	}
+	req.Object.Status.ObservedCommonMetadataDigest = ""
+	if req.Object.Spec.CommonMetadata != nil {
+		req.Object.Status.ObservedCommonMetadataDigest = postrender.CommonMetadataDigest(digest.Canonical, req.Object.Spec.CommonMetadata).String()
+	}
 }
 
 // eventMessageWithLog returns an event message composed out of the given

internal/reconcile/state.go

@@ -21,8 +21,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/fluxcd/pkg/apis/meta"
-	"github.com/fluxcd/pkg/runtime/conditions"
 	"github.com/fluxcd/pkg/ssa/jsondiff"
 	"helm.sh/helm/v4/pkg/kube"
 	helmreleasecommon "helm.sh/helm/v4/pkg/release/common"
@@ -147,14 +145,17 @@ func DetermineReleaseState(ctx context.Context, cfg *action.ConfigFactory, req *
 		}
 
 		// Verify if postrender digest or common metadata digest has changed
-		// if config has not been processed. For the processed or partially processed generation,
-		// the updated observation will only be reflected at the end of a successful
-		// reconciliation. Comparing here would result the reconciliation to
-		// get stuck in this check due to a mismatch forever.  The value can't
-		// change without a new generation. Hence, compare the observed digest
-		// for new generations only.
-		ready := conditions.Get(req.Object, meta.ReadyCondition)
-		if ready != nil && ready.ObservedGeneration != req.Object.Generation {
+		// for new generations only. The observed digests are updated after
+		// each successful release action, so comparing here will not cause
+		// an infinite loop within the same reconciliation.
+		//
+		// We use the top-level status.observedGeneration rather than the
+		// Ready condition's ObservedGeneration, because the latter can be
+		// inadvertently advanced by the patch helper's conflict resolution
+		// (patchStatusConditions re-fetches the object from the API server,
+		// and conditions.Set always sets ObservedGeneration to the latest
+		// metadata.generation).
+		if req.Object.Status.ObservedGeneration != req.Object.Generation {
 			var postrenderersDigest string
 			if req.Object.Spec.PostRenderers != nil {
 				postrenderersDigest = postrender.Digest(digest.Canonical, req.Object.Spec.PostRenderers).String()

internal/reconcile/state_test.go

@@ -508,17 +508,11 @@ func Test_DetermineReleaseState(t *testing.T) {
 			},
 			status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
 				return v2.HelmReleaseStatus{
+					ObservedGeneration: 2,
 					History: v2.Snapshots{
 						release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
 					},
 					ObservedPostRenderersDigest: postrender.Digest(digest.Canonical, postRenderers).String(),
-					Conditions: []metav1.Condition{
-						{
-							Type:               meta.ReadyCondition,
-							Status:             metav1.ConditionTrue,
-							ObservedGeneration: 2,
-						},
-					},
 				}
 			},
 			chart:  testutil.BuildChart(),
@@ -578,17 +572,11 @@ func Test_DetermineReleaseState(t *testing.T) {
 			},
 			status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
 				return v2.HelmReleaseStatus{
+					ObservedGeneration: 2,
 					History: v2.Snapshots{
 						release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
 					},
 					ObservedCommonMetadataDigest: postrender.CommonMetadataDigest(digest.Canonical, commonMetadata).String(),
-					Conditions: []metav1.Condition{
-						{
-							Type:               meta.ReadyCondition,
-							Status:             metav1.ConditionTrue,
-							ObservedGeneration: 2,
-						},
-					},
 				}
 			},
 			chart:  testutil.BuildChart(),