fluxcd
/
helm-controller
mirror of https://github.com/fluxcd/helm-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

controller: unready dep should not bump obs gen 

This ensures that any unfulfilled dependencies for which we requeue do
not prematurely bump the observed generation by introducing typed
errors.

These typed errors ensure that the logic to bump the observed generation
can continue to be the same, while ignoring them just in time before
returning the final error.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
This commit is contained in:
Hidde Beydals 2023-11-30 12:59:32 +01:00
parent bc7fb25d27
commit 48cad68386
No known key found for this signature in database
GPG Key ID: 979F380FC2341744
4 changed files with 87 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
internal/controller/helmrelease_controller.go
View File

@ -59,6 +59,7 @@ import (
"github.com/fluxcd/helm-controller/internal/action"
"github.com/fluxcd/helm-controller/internal/chartutil"
"github.com/fluxcd/helm-controller/internal/digest"
interrors "github.com/fluxcd/helm-controller/internal/errors"
"github.com/fluxcd/helm-controller/internal/features"
"github.com/fluxcd/helm-controller/internal/kube"
"github.com/fluxcd/helm-controller/internal/loader"
@ -97,6 +98,11 @@ type HelmReleaseReconcilerOptions struct {
RateLimiter ratelimiter.RateLimiter
}
var (
errWaitForDependency = errors.New("must wait for dependency")
errWaitForChart = errors.New("must wait for chart")
)
func (r *HelmReleaseReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts HelmReleaseReconcilerOptions) error {
// Index the HelmRelease by the HelmChart references they point at
if err := mgr.GetFieldIndexer().IndexField(ctx, &v2.HelmRelease{}, v2.SourceIndexKey,
@ -167,6 +173,13 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request)
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
}
// We do not want to return these errors, but rather wait for the
// designated RequeueAfter to expire and try again.
// However, not returning an error will cause the patch helper to
// patch the observed generation, which we do not want. So we ignore
// these errors here after patching.
retErr = interrors.Ignore(retErr, errWaitForDependency, errWaitForChart)
if err := patchHelper.Patch(ctx, obj, patchOpts...); err != nil {
if !obj.DeletionTimestamp.IsZero() {
err = apierrutil.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) })
@ -230,7 +243,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
// Exponential backoff would cause execution to be prolonged too much,
// instead we requeue on a fixed interval.
return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
return ctrl.Result{RequeueAfter: r.requeueDependency}, errWaitForDependency
}
log.Info("all dependencies are ready")
@ -262,7 +275,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
conditions.MarkFalse(obj, meta.ReadyCondition, "HelmChartNotReady", msg)
// Do not requeue immediately, when the artifact is created
// the watcher should trigger a reconciliation.
return jitter.JitteredRequeueInterval(ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}), nil
return jitter.JitteredRequeueInterval(ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}), errWaitForChart
}
// Compose values based from the spec and references.
@ -276,10 +289,10 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
loadedChart, err := loader.SecureLoadChartFromURL(r.httpClient, hc.GetArtifact().URL, hc.GetArtifact().Digest)
if err != nil {
if errors.Is(err, loader.ErrFileNotFound) {
msg := fmt.Sprintf("Chart not ready: artifact not found. Retrying in %s", r.requeueDependency)
msg := fmt.Sprintf("Chart not ready: artifact not found. Retrying in %s", r.requeueDependency.String())
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, msg)
log.Info(msg)
return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
return ctrl.Result{RequeueAfter: r.requeueDependency}, errWaitForDependency
}
conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, fmt.Sprintf("Could not load chart: %s", err.Error()))

10
internal/controller/helmrelease_controller_test.go
View File

@ -108,7 +108,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
}
res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(err).To(Equal(errWaitForDependency))
g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
@ -222,7 +222,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
}
res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(err).To(Equal(errWaitForChart))
g.Expect(res.RequeueAfter).To(Equal(obj.Spec.Interval.Duration))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
@ -274,7 +274,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
}
res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(err).To(Equal(errWaitForChart))
g.Expect(res.RequeueAfter).To(Equal(obj.Spec.Interval.Duration))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
@ -326,7 +326,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
}
res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(err).To(Equal(errWaitForChart))
g.Expect(res.RequeueAfter).To(Equal(obj.Spec.Interval.Duration))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
@ -438,7 +438,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) {
}
res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(err).To(Equal(errWaitForDependency))
g.Expect(res.RequeueAfter).To(Equal(r.requeueDependency))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{

25
internal/errors/ignore.go Normal file
View File

@ -0,0 +1,25 @@
/*
Copyright 2023 The Flux authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package errors
// Ignore returns nil if err is equal to any of the errs.
func Ignore(err error, errs ...error) error {
if IsOneOf(err, errs...) {
return nil
}
return err
}

40
internal/errors/ignore_test.go Normal file
View File

@ -0,0 +1,40 @@
/*
Copyright 2023 The Flux authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package errors
import (
"errors"
"testing"
)
func TestIgnore(t *testing.T) {
err1 := errors.New("error1")
err2 := errors.New("error2")
if err := Ignore(err1, err1, err2); err != nil {
t.Errorf("Expected Ignore to return nil when the error is in the list, but got %v", err)
}
err3 := errors.New("error3")
if err := Ignore(err3, err1, err2); !errors.Is(err, err3) {
t.Errorf("Expected Ignore to return the error when it is not in the list, but got %v", err)
}
if err := Ignore(err1); !errors.Is(err, err1) {
t.Errorf("Expected Ignore to return the error with an empty list of errors, but got %v", err)
}
}