Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework HelmRelease reconciliation logic #738

Merged
merged 76 commits into from
Nov 24, 2023
Merged
Changes from 1 commit
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
e82d389
helm/storage: add observator and implementation
hiddeco May 2, 2022
fe661df
Move HelmChart handling to separate reconciler
hiddeco May 2, 2022
0140eee
Factor various bits out of reconciler
hiddeco May 5, 2022
c99b00d
Move predicates into package and add tests
hiddeco May 6, 2022
730ccec
Move post renderers into separate package
hiddeco May 6, 2022
14e08f7
api: introduce v2beta2 API
hiddeco Jul 1, 2022
89a6f49
Run individual Helm actions using HelmRelease
hiddeco Jul 1, 2022
dfebba2
Add `ObservedRelease` and other release utils
hiddeco Jul 1, 2022
d9055f8
Add reconcile logic for individual Helm actions
hiddeco Jul 1, 2022
220e789
Allow detection of next reconcile action
hiddeco Jul 1, 2022
5843cc2
action: allow passing of config options
hiddeco Jul 6, 2022
6db62ed
Adding test filters
jtyr Jul 28, 2022
88a21fe
Moving stuff from runner; removing changes in v2beta1
jtyr Jul 28, 2022
e139354
Fixing typo
jtyr Jul 28, 2022
8cefed1
Adding tests
jtyr Jul 28, 2022
9e1eedc
api: various changes to support new logic
hiddeco Sep 15, 2022
0b8692f
api: add service account name validation rule
hiddeco Sep 20, 2022
9812286
action: add `Len` method to `LogBuffer`
hiddeco Sep 15, 2022
026fd45
action: add name param to rollback and uninstall
hiddeco Sep 15, 2022
4793414
action: allow composed release name >=53 char
hiddeco Sep 20, 2022
b975b3f
reconcile: add atomic release reconciler
hiddeco Sep 16, 2022
ea81c8e
action: include TS in LogBuffer
hiddeco Oct 5, 2022
64cc09c
reconcile: test emitted events
hiddeco Sep 27, 2022
410ce3a
reconcile: include "token" in event metadata
hiddeco Jun 14, 2023
64b2d54
Address review comments
hiddeco Jul 10, 2023
76f62ff
api: backport uninstall del propagation to v2beta2
hiddeco Jul 11, 2023
deb0b14
api: make v2beta2 storage version
hiddeco Jun 16, 2023
bb4e9b7
Update YAMLs to `helm.toolkit.fluxcd.io/v2beta2`
hiddeco Jun 16, 2023
eee91b0
Introduce new `yaml` package with `Encode` func
hiddeco Jul 17, 2023
d802ba6
controllers: roughly rewire HelmRelease reconciler
hiddeco Jul 24, 2023
bbefbc4
reconcile: use failure count in Stalled condition
hiddeco Jul 24, 2023
866f076
reconcile: share PatchHelper with controller
hiddeco Jul 24, 2023
68c273b
controller: handle delete before adding finalizer
hiddeco Jul 24, 2023
e32c1a0
reconcile: trim space from Helm error messages
hiddeco Jul 24, 2023
dab2578
acl: introduce package to enable global config
hiddeco Jul 27, 2023
5e3ad5d
reconcile: add `HelmChartTemplate` sub-reconciler
hiddeco Jul 27, 2023
1dac82a
reconcile: handle manually uninstalled release
hiddeco Jul 27, 2023
fbd73ac
controller: start w/ adding tests for HelmRelease
hiddeco Jul 28, 2023
b2ba3d9
controller: improve deletion logic and add tests
hiddeco Aug 21, 2023
9df9b17
api: various naming improvements
hiddeco Aug 21, 2023
7dfce0c
api: introduce `APIVersion` in `Snapshot`
hiddeco Aug 21, 2023
882da27
api: move `Current` and `Previous` into `History`
hiddeco Aug 25, 2023
94064da
controller: add reconcile release tests
hiddeco Sep 15, 2023
5510175
reconcile: tweak event messages
hiddeco Sep 22, 2023
bc036c0
reconcile: improve insights of progress in logs
hiddeco Oct 31, 2023
a6ae4c3
reconcile: improve log levels of actions
hiddeco Oct 31, 2023
272329d
action: add `:` separator between ts and msg logs
hiddeco Oct 31, 2023
19be1b2
api: change format of `Snapshot#FullReleaseName`
hiddeco Oct 31, 2023
ac9c2c3
reconcile: ensure object patch on context cancel
hiddeco Oct 31, 2023
f156c35
reconcile: allow cfg of manager in atomic action
hiddeco Oct 31, 2023
191bebf
reconcile: simplify `NextAction` logic
hiddeco Oct 31, 2023
d0c4c14
reconcile: improve uninstall w/o purging history
hiddeco Nov 1, 2023
096956f
controller: properly record object metrics
hiddeco Nov 3, 2023
7c52fd2
action: simplify chart diff logic
hiddeco Nov 3, 2023
2df90eb
reconcile: improve observability between actions
hiddeco Nov 3, 2023
80d0878
controller: ignore `NotFound` API error on delete
hiddeco Nov 3, 2023
10277c7
api: add `LastAttemptedReleaseAction` to status
hiddeco Nov 7, 2023
2e0e225
reconcile: improve state determination
hiddeco Nov 7, 2023
16da3ec
reconcile: allow unlock without current
hiddeco Nov 8, 2023
517d42f
build: incorporate condition changes in e2e
hiddeco Oct 31, 2023
c5a017c
api: record observed releases in `Status.History`
hiddeco Nov 17, 2023
533589c
api: change `MaxHistory` default to `5`
hiddeco Nov 17, 2023
eacd975
reconcile: remove reconciler type from messages
hiddeco Nov 17, 2023
7048501
controller: requeue on fixed interval on chart 404
hiddeco Nov 17, 2023
28a7800
reconcile: mark `Ready=Unknown` when awaiting test
hiddeco Nov 17, 2023
6f05228
reconcile: remove logs from test failure event
hiddeco Nov 17, 2023
9bb8f02
api: continue to record `LastAppliedRevision`
hiddeco Nov 21, 2023
eab8a50
api: prepare `v2beta1` status for `v2beta2`
hiddeco Nov 21, 2023
580c72c
controller: adopt release based on v2beta1 state
hiddeco Nov 21, 2023
20c00fd
action: provide a reason on release target changes
hiddeco Nov 22, 2023
5d1f34a
controller: patch after setting `Reconciling=True`
hiddeco Nov 22, 2023
7aad010
controller: immediate requeue unfinished release
hiddeco Nov 22, 2023
6ffdadd
action: omit logging on CRD apply no-op
hiddeco Nov 22, 2023
0535ae1
predicates: notice source changing to `Ready=True`
hiddeco Nov 22, 2023
3ce6e8d
reconcile: improve wording `Stalled` condition
hiddeco Nov 22, 2023
4a8d2ff
action: provide reason for failures count reset
hiddeco Nov 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
reconcile: share PatchHelper with controller
This ensures they both have the same observation on the last
modifications made to the object. Preventing possible scenarios where
a condition would not be removed because it wasn't set at the start of
the reconcile run, then added, and then removed. Causing it to go
unnoticed during the diff calculation.

Signed-off-by: Hidde Beydals <[email protected]>
hiddeco committed Nov 20, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 866f076d1f09eb88a4714a5ae3394bc72b24cfa9
6 changes: 3 additions & 3 deletions internal/controller/helmrelease_controller.go
Original file line number Diff line number Diff line change
@@ -202,10 +202,10 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}

return r.reconcileRelease(ctx, obj)
return r.reconcileRelease(ctx, patchHelper, obj)
}

func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, obj *v2.HelmRelease) (ctrl.Result, error) {
func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelper *patch.SerialPatcher, obj *v2.HelmRelease) (ctrl.Result, error) {
log := ctrl.LoggerFrom(ctx)

// Mark the resource as under reconciliation.
@@ -310,7 +310,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, obj *v2.He
}

// Off we go!
if err = intreconcile.NewAtomicRelease(r.Client, cfg, r.EventRecorder).Reconcile(ctx, &intreconcile.Request{
if err = intreconcile.NewAtomicRelease(patchHelper, cfg, r.EventRecorder).Reconcile(ctx, &intreconcile.Request{
Object: obj,
Chart: loadedChart,
Values: values,
17 changes: 7 additions & 10 deletions internal/reconcile/atomic_release.go
Original file line number Diff line number Diff line change
@@ -23,14 +23,12 @@ import (
"strings"
"time"

"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/fluxcd/pkg/runtime/logger"
"github.com/fluxcd/pkg/runtime/patch"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"

v2 "github.com/fluxcd/helm-controller/api/v2beta2"
"github.com/fluxcd/helm-controller/internal/action"
@@ -78,17 +76,17 @@ var OwnedConditions = []string{
// For more information on the individual ActionReconcilers, refer to their
// documentation.
type AtomicRelease struct {
kubeClient client.Client
patchHelper *patch.SerialPatcher
configFactory *action.ConfigFactory
eventRecorder record.EventRecorder
strategy releaseStrategy
}

// NewAtomicRelease returns a new AtomicRelease reconciler configured with the
// provided values.
func NewAtomicRelease(client client.Client, cfg *action.ConfigFactory, recorder record.EventRecorder) *AtomicRelease {
func NewAtomicRelease(patchHelper *patch.SerialPatcher, cfg *action.ConfigFactory, recorder record.EventRecorder) *AtomicRelease {
return &AtomicRelease{
kubeClient: client,
patchHelper: patchHelper,
eventRecorder: recorder,
configFactory: cfg,
strategy: &cleanReleaseStrategy{},
@@ -128,7 +126,6 @@ func (cleanReleaseStrategy) MustStop(current ReconcilerType, _ ReconcilerTypeSet

func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
log := ctrl.LoggerFrom(ctx).V(logger.DebugLevel)
patchHelper := patch.NewSerialPatcher(req.Object, r.kubeClient)

var (
previous ReconcilerTypeSet
@@ -178,7 +175,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
conditions.MarkTrue(req.Object, meta.ReconcilingCondition, "Progressing", "Running '%s' %s action with timeout of %s",
next.Name(), next.Type(), timeoutForAction(next, req.Object).String())
// Patch the object to reflect the new condition.
if err = patchHelper.Patch(ctx, req.Object, patch.WithOwnedConditions{Conditions: OwnedConditions}, patch.WithFieldOwner("helm-controller")); err != nil {
if err = r.patchHelper.Patch(ctx, req.Object, patch.WithOwnedConditions{Conditions: OwnedConditions}, patch.WithFieldOwner("helm-controller")); err != nil {
return err
}

@@ -201,7 +198,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
previous = append(previous, next.Type())

// Patch the release to reflect progress.
if err = patchHelper.Patch(ctx, req.Object, patch.WithOwnedConditions{Conditions: OwnedConditions}, patch.WithFieldOwner("helm-controller")); err != nil {
if err = r.patchHelper.Patch(ctx, req.Object, patch.WithOwnedConditions{Conditions: OwnedConditions}, patch.WithFieldOwner("helm-controller")); err != nil {
return err
}
}
4 changes: 3 additions & 1 deletion internal/reconcile/atomic_release_test.go
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ package reconcile

import (
"context"
"github.com/fluxcd/pkg/runtime/patch"
"testing"
"time"

@@ -155,14 +156,15 @@ func TestAtomicRelease_Reconcile(t *testing.T) {
WithObjects(obj).
WithStatusSubresource(&v2.HelmRelease{}).
Build()
patchHelper := patch.NewSerialPatcher(obj, client)
recorder := new(record.FakeRecorder)

req := &Request{
Object: obj,
Chart: testutil.BuildChart(testutil.ChartWithTestHook()),
Values: nil,
}
g.Expect(NewAtomicRelease(client, cfg, recorder).Reconcile(context.TODO(), req)).ToNot(HaveOccurred())
g.Expect(NewAtomicRelease(patchHelper, cfg, recorder).Reconcile(context.TODO(), req)).ToNot(HaveOccurred())

g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
{