From 58fc4df1e8b2c73b2699f21993e95cef473ce414 Mon Sep 17 00:00:00 2001 From: TarasLykhenko <18290033+TarasLykhenko@users.noreply.github.com> Date: Thu, 12 Nov 2026 14:48:33 +0000 Subject: [PATCH] helm-operator: record observedGeneration in the custom resource status The Helm operator now sets status.observedGeneration to the custom resource's metadata.generation on every status write, so clients can tell whether the conditions and deployedRelease describe the current spec or an earlier one. The generation is set before any condition changes, so failure paths record it too, and a spec change that needs no release upgrade still produces a status write. Signed-off-by: TarasLykhenko <18290033+TarasLykhenko@users.noreply.github.com> --- .../fragments/helm-observed-generation.yaml | 8 + internal/helm/controller/reconcile.go | 2 + internal/helm/controller/reconcile_test.go | 176 ++++++++++++++++++ internal/helm/internal/types/types.go | 5 +- 4 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 changelog/fragments/helm-observed-generation.yaml diff --git a/changelog/fragments/helm-observed-generation.yaml b/changelog/fragments/helm-observed-generation.yaml new file mode 100644 index 00000000000..8966e3db6b9 --- /dev/null +++ b/changelog/fragments/helm-observed-generation.yaml @@ -0,0 +1,8 @@ +entries: + - description: > + For Helm-based operators, the custom resource status now records + `observedGeneration`, the `metadata.generation` that its conditions and + `deployedRelease` describe, so clients can tell whether the status + reflects the current spec. + kind: addition + breaking: false diff --git a/internal/helm/controller/reconcile.go b/internal/helm/controller/reconcile.go index ec9a77ef90e..cf26f654203 100644 --- a/internal/helm/controller/reconcile.go +++ b/internal/helm/controller/reconcile.go @@ -106,6 +106,8 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile status := types.StatusFor(o) originalStatus := types.StatusFor(o.DeepCopy()) + // Set first so every status write records it, even when the release is unchanged. + status.ObservedGeneration = o.GetGeneration() log = log.WithValues("release", manager.ReleaseName()) reconcileResult := reconcile.Result{RequeueAfter: r.ReconcilePeriod} diff --git a/internal/helm/controller/reconcile_test.go b/internal/helm/controller/reconcile_test.go index 4700a2e5924..eddd5d83556 100644 --- a/internal/helm/controller/reconcile_test.go +++ b/internal/helm/controller/reconcile_test.go @@ -15,11 +15,25 @@ package controller import ( + "context" + "errors" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + rpb "helm.sh/helm/v3/pkg/release" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + helmtypes "github.com/operator-framework/operator-sdk/internal/helm/internal/types" + "github.com/operator-framework/operator-sdk/internal/helm/release" ) func TestDetermineReconcilePeriod(t *testing.T) { @@ -251,3 +265,165 @@ func Test_readBoolAnnotationWithDefault(t *testing.T) { }) } } + +var testGVK = schema.GroupVersionKind{Group: "example.com", Version: "v1", Kind: "App"} + +func TestReconcileSetsObservedGeneration(t *testing.T) { + deployed := &rpb.Release{Name: "app", Version: 2, Manifest: "kind: ConfigMap\n"} + + tests := []struct { + name string + status map[string]any + manager *fakeManager + wantErr bool + wantCondition helmtypes.HelmAppConditionType + }{ + { + name: "Should record the generation on install", + manager: &fakeManager{installed: &rpb.Release{Name: "app", Version: 1, Manifest: "kind: ConfigMap\n"}}, + wantCondition: helmtypes.ConditionDeployed, + }, + { + name: "Should record the generation on upgrade", + status: deployedStatus(t, 1, deployed), + manager: &fakeManager{isInstalled: true, upgradeRequired: true, upgraded: deployed}, + wantCondition: helmtypes.ConditionDeployed, + }, + { + // The release is unchanged, so only the generation differs from + // the stored status. + name: "Should record the generation when the release is unchanged", + status: deployedStatus(t, 1, deployed), + manager: &fakeManager{isInstalled: true, reconciled: deployed}, + wantCondition: helmtypes.ConditionDeployed, + }, + { + name: "Should record the generation when sync fails", + status: deployedStatus(t, 1, deployed), + manager: &fakeManager{isInstalled: true, syncErr: errors.New("sync failed")}, + wantErr: true, + wantCondition: helmtypes.ConditionIrreconcilable, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + obj := newApp(2, tc.status) + c := fake.NewClientBuilder().WithObjects(obj).WithStatusSubresource(obj). + WithInterceptorFuncs(interceptor.Funcs{SubResourceUpdate: typedStatusToMap}).Build() + r := HelmOperatorReconciler{ + Client: c, + EventRecorder: record.NewFakeRecorder(10), + GVK: testGVK, + ManagerFactory: fakeManagerFactory{tc.manager}, + } + + _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "ns", Name: "app"}}) + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + got := &unstructured.Unstructured{} + got.SetGroupVersionKind(testGVK) + require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(obj), got)) + status := helmtypes.StatusFor(got) + assert.Equal(t, got.GetGeneration(), status.ObservedGeneration) + assert.True(t, hasCondition(status, tc.wantCondition), "missing condition %s", tc.wantCondition) + }) + } +} + +// typedStatusToMap converts the typed status that updateResourceStatus stores +// in the unstructured object into a map, as JSON serialization does for a real +// API server. The fake client cannot deep copy the typed value. +func typedStatusToMap(ctx context.Context, c client.Client, subResource string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + if u, ok := obj.(*unstructured.Unstructured); ok { + if s, ok := u.Object["status"].(*helmtypes.HelmAppStatus); ok { + m, err := s.ToMap() + if err != nil { + return err + } + u.Object["status"] = m + } + } + return c.SubResource(subResource).Update(ctx, obj, opts...) +} + +func newApp(generation int64, status map[string]any) *unstructured.Unstructured { + obj := &unstructured.Unstructured{Object: map[string]any{"spec": map[string]any{"replicas": int64(1)}}} + obj.SetGroupVersionKind(testGVK) + obj.SetNamespace("ns") + obj.SetName("app") + obj.SetGeneration(generation) + obj.SetFinalizers([]string{uninstallFinalizer}) + if status != nil { + obj.Object["status"] = status + } + return obj +} + +func deployedStatus(t *testing.T, generation int64, rel *rpb.Release) map[string]any { + t.Helper() + status := &helmtypes.HelmAppStatus{ + Conditions: []helmtypes.HelmAppCondition{ + {Type: helmtypes.ConditionInitialized, Status: helmtypes.StatusTrue}, + {Type: helmtypes.ConditionDeployed, Status: helmtypes.StatusTrue, Reason: helmtypes.ReasonUpgradeSuccessful}, + }, + DeployedRelease: &helmtypes.HelmAppRelease{Name: rel.Name, Manifest: rel.Manifest}, + ObservedGeneration: generation, + } + m, err := status.ToMap() + require.NoError(t, err) + return m +} + +func hasCondition(status *helmtypes.HelmAppStatus, ct helmtypes.HelmAppConditionType) bool { + for _, c := range status.Conditions { + if c.Type == ct && c.Status == helmtypes.StatusTrue { + return true + } + } + return false +} + +type fakeManagerFactory struct { + manager *fakeManager +} + +func (f fakeManagerFactory) NewManager(*unstructured.Unstructured, map[string]string, string) (release.Manager, error) { + return f.manager, nil +} + +type fakeManager struct { + isInstalled bool + upgradeRequired bool + syncErr error + installed *rpb.Release + upgraded *rpb.Release + reconciled *rpb.Release +} + +func (m *fakeManager) ReleaseName() string { return "app" } +func (m *fakeManager) IsInstalled() bool { return m.isInstalled } +func (m *fakeManager) IsUpgradeRequired() bool { return m.upgradeRequired } +func (m *fakeManager) Sync() error { return m.syncErr } +func (m *fakeManager) RollBack(...release.RollBackOption) error { return nil } +func (m *fakeManager) CleanupRelease(string) (bool, error) { return true, nil } + +func (m *fakeManager) InstallRelease(...release.InstallOption) (*rpb.Release, error) { + return m.installed, nil +} + +func (m *fakeManager) UpgradeRelease(...release.UpgradeOption) (*rpb.Release, *rpb.Release, error) { + return m.upgraded, m.upgraded, nil +} + +func (m *fakeManager) ReconcileRelease(context.Context) (*rpb.Release, error) { + return m.reconciled, nil +} + +func (m *fakeManager) UninstallRelease(...release.UninstallOption) (*rpb.Release, error) { + return nil, nil +} diff --git a/internal/helm/internal/types/types.go b/internal/helm/internal/types/types.go index a1ac6d63ba6..4cf6a585200 100644 --- a/internal/helm/internal/types/types.go +++ b/internal/helm/internal/types/types.go @@ -75,8 +75,9 @@ const ( ) type HelmAppStatus struct { - Conditions []HelmAppCondition `json:"conditions"` - DeployedRelease *HelmAppRelease `json:"deployedRelease,omitempty"` + Conditions []HelmAppCondition `json:"conditions"` + DeployedRelease *HelmAppRelease `json:"deployedRelease,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } func (s *HelmAppStatus) ToMap() (map[string]any, error) {