diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index d10911653..64c3dbba7 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -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,8 @@ func (r *HelmReleaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{}) } + 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 +238,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 +270,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 +284,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())) diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index e432a210b..e5c31ffa5 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -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{ diff --git a/internal/errors/ignore.go b/internal/errors/ignore.go new file mode 100644 index 000000000..8cd247c92 --- /dev/null +++ b/internal/errors/ignore.go @@ -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 +} diff --git a/internal/errors/ignore_test.go b/internal/errors/ignore_test.go new file mode 100644 index 000000000..bcf204ea3 --- /dev/null +++ b/internal/errors/ignore_test.go @@ -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) + } +}