Skip to content

Commit

Permalink
Merge pull request #972 from fluxcd/improve-postrenderer-tracking
Browse files Browse the repository at this point in the history 
PostRenderersDigest observation improvements
  • Loading branch information
@stefanprodan
stefanprodan authored May 9, 2024
2 parents 461d45d + 63f7a76 commit f88b3be
Show file tree
Hide file tree
Showing 6 changed files with 318 additions and 126 deletions.
11 changes: 11 additions & 0 deletions internal/reconcile/atomic_release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ 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.
Expand Down Expand Up @@ -210,6 +212,15 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
// written to Ready.
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()
}
}

return nil
}

Expand Down
225 changes: 225 additions & 0 deletions internal/reconcile/atomic_release_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,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/kube"
"github.com/fluxcd/helm-controller/internal/postrender"
"github.com/fluxcd/helm-controller/internal/release"
"github.com/fluxcd/helm-controller/internal/testutil"
)
Expand Down Expand Up @@ -1067,6 +1069,229 @@ func TestAtomicRelease_Reconcile_Scenarios(t *testing.T) {
}
}

func TestAtomicRelease_Reconcile_PostRenderers_Scenarios(t *testing.T) {
tests := []struct {
name string
releases func(namespace string) []*helmrelease.Release
spec func(spec *v2.HelmReleaseSpec)
values map[string]interface{}
status func(releases []*helmrelease.Release) v2.HelmReleaseStatus
wantDigest string
wantReleaseAction v2.ReleaseAction
}{
{
name: "addition of post renderers",
releases: func(namespace string) []*helmrelease.Release {
return []*helmrelease.Release{
testutil.BuildRelease(&helmrelease.MockReleaseOptions{
Name: mockReleaseName,
Namespace: namespace,
Version: 1,
Status: helmrelease.StatusDeployed,
Chart: testutil.BuildChart(),
}, testutil.ReleaseWithConfig(nil)),
}
},
spec: func(spec *v2.HelmReleaseSpec) {
spec.PostRenderers = postRenderers
},
status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
return v2.HelmReleaseStatus{
History: v2.Snapshots{
release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
},
Conditions: []metav1.Condition{
{
Type: meta.ReadyCondition,
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
},
},
}
},
wantDigest: postrender.Digest(digest.Canonical, postRenderers).String(),
wantReleaseAction: v2.ReleaseActionUpgrade,
},
{
name: "config change and addition of post renderers",
releases: func(namespace string) []*helmrelease.Release {
return []*helmrelease.Release{
testutil.BuildRelease(&helmrelease.MockReleaseOptions{
Name: mockReleaseName,
Namespace: namespace,
Version: 1,
Status: helmrelease.StatusDeployed,
Chart: testutil.BuildChart(),
}, testutil.ReleaseWithConfig(nil)),
}
},
spec: func(spec *v2.HelmReleaseSpec) {
spec.PostRenderers = postRenderers
},
status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
return v2.HelmReleaseStatus{
History: v2.Snapshots{
release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
},
Conditions: []metav1.Condition{
{
Type: meta.ReadyCondition,
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
},
},
}
},
values: map[string]interface{}{"foo": "baz"},
wantDigest: postrender.Digest(digest.Canonical, postRenderers).String(),
wantReleaseAction: v2.ReleaseActionUpgrade,
},
{
name: "existing and new post renderers value",
releases: func(namespace string) []*helmrelease.Release {
return []*helmrelease.Release{
testutil.BuildRelease(&helmrelease.MockReleaseOptions{
Name: mockReleaseName,
Namespace: namespace,
Version: 1,
Status: helmrelease.StatusDeployed,
Chart: testutil.BuildChart(),
}, testutil.ReleaseWithConfig(nil)),
}
},
spec: func(spec *v2.HelmReleaseSpec) {
spec.PostRenderers = postRenderers2
},
status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
return v2.HelmReleaseStatus{
History: v2.Snapshots{
release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
},
Conditions: []metav1.Condition{
{
Type: meta.ReadyCondition,
Status: metav1.ConditionTrue,
ObservedGeneration: 1,
},
},
ObservedPostRenderersDigest: postrender.Digest(digest.Canonical, postRenderers).String(),
}
},
wantDigest: postrender.Digest(digest.Canonical, postRenderers2).String(),
wantReleaseAction: v2.ReleaseActionUpgrade,
},
{
name: "post renderers mismatch remains in sync for processed config",
releases: func(namespace string) []*helmrelease.Release {
return []*helmrelease.Release{
testutil.BuildRelease(&helmrelease.MockReleaseOptions{
Name: mockReleaseName,
Namespace: namespace,
Version: 1,
Status: helmrelease.StatusDeployed,
Chart: testutil.BuildChart(),
}, testutil.ReleaseWithConfig(nil)),
}
},
spec: func(spec *v2.HelmReleaseSpec) {
spec.PostRenderers = postRenderers2
},
status: func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
return v2.HelmReleaseStatus{
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.
},
},
ObservedPostRenderersDigest: postrender.Digest(digest.Canonical, postRenderers).String(),
}
},
wantDigest: postrender.Digest(digest.Canonical, postRenderers2).String(),
wantReleaseAction: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace)
g.Expect(err).NotTo(HaveOccurred())
t.Cleanup(func() {
_ = testEnv.Delete(context.TODO(), namedNS)
})
releaseNamespace := namedNS.Name

releases := tt.releases(releaseNamespace)

obj := &v2.HelmRelease{
ObjectMeta: metav1.ObjectMeta{
Name: mockReleaseName,
Namespace: releaseNamespace,
// Set a higher generation value to allow setting
// observations in previous generations.
Generation: 2,
},
Spec: v2.HelmReleaseSpec{
ReleaseName: mockReleaseName,
TargetNamespace: releaseNamespace,
StorageNamespace: releaseNamespace,
Timeout: &metav1.Duration{Duration: 100 * time.Millisecond},
},
}

if tt.spec != nil {
tt.spec(&obj.Spec)
}
if tt.status != nil {
obj.Status = tt.status(releases)
}

getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace())
g.Expect(err).ToNot(HaveOccurred())

cfg, err := action.NewConfigFactory(getter,
action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()),
)
g.Expect(err).ToNot(HaveOccurred())

store := helmstorage.Init(cfg.Driver)
for _, r := range releases {
g.Expect(store.Create(r)).To(Succeed())
}

// We use a fake client here to allow us to work with a minimal release
// object mock. As the fake client does not perform any validation.
// However, for the Helm storage driver to work, we need a real client
// which is therefore initialized separately above.
client := fake.NewClientBuilder().
WithScheme(testEnv.Scheme()).
WithObjects(obj).
WithStatusSubresource(&v2.HelmRelease{}).
Build()
patchHelper := patch.NewSerialPatcher(obj, client)
recorder := new(record.FakeRecorder)

req := &Request{
Object: obj,
Chart: testutil.BuildChart(),
Values: tt.values,
}

err = NewAtomicRelease(patchHelper, cfg, recorder, testFieldManager).Reconcile(context.TODO(), req)
g.Expect(err).ToNot(HaveOccurred())

g.Expect(obj.Status.ObservedPostRenderersDigest).To(Equal(tt.wantDigest))
g.Expect(obj.Status.LastAttemptedReleaseAction).To(Equal(tt.wantReleaseAction))
})
}
}

func TestAtomicRelease_actionForState(t *testing.T) {
tests := []struct {
name string
Expand Down
11 changes: 0 additions & 11 deletions internal/reconcile/release.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ 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/postrender"
"github.com/fluxcd/helm-controller/internal/release"
"github.com/fluxcd/helm-controller/internal/storage"
)
Expand Down Expand Up @@ -190,15 +188,6 @@ func summarize(req *Request) {
Message: conds[0].Message,
ObservedGeneration: req.Object.Generation,
})

// remove stale post-renderers digest
if conditions.Get(req.Object, meta.ReadyCondition).Status == metav1.ConditionTrue {
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()
}
}
}

// eventMessageWithLog returns an event message composed out of the given
Expand Down
Loading

0 comments on commit f88b3be

Please sign in to comment.