diff --git a/internal/controller/helmrelease_controller.go b/internal/controller/helmrelease_controller.go index e4d201f15..c8336a411 100644 --- a/internal/controller/helmrelease_controller.go +++ b/internal/controller/helmrelease_controller.go @@ -288,6 +288,13 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, msg := fmt.Sprintf("could not get Source object: %s", err.Error()) conditions.MarkFalse(obj, meta.ReadyCondition, v2.ArtifactFailedReason, "%s", msg) + + if apierrors.IsNotFound(err) { + // Exponential backoff would cause execution to be prolonged too much, + // instead we requeue on a fixed interval. + log.Info(fmt.Sprintf("%s: retrying in %s", msg, r.DependencyRequeueInterval.String())) + return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, errWaitForDependency + } return ctrl.Result{}, err } // Remove any stale corresponding Ready=False condition with Unknown. @@ -318,6 +325,13 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, if err != nil { conditions.MarkFalse(obj, meta.ReadyCondition, "ValuesError", "%s", err) r.Eventf(obj, corev1.EventTypeWarning, "ValuesError", "%s", err.Error()) + + if errors.Is(err, chartutil.ErrResourceNotFound) { + // Exponential backoff would cause execution to be prolonged too much, + // instead we requeue on a fixed interval. + log.Info(fmt.Sprintf("%s: retrying in %s", err.Error(), r.DependencyRequeueInterval.String())) + return ctrl.Result{RequeueAfter: r.DependencyRequeueInterval}, errWaitForDependency + } return ctrl.Result{}, err } // Remove any stale corresponding Ready=False condition with Unknown. diff --git a/internal/controller/helmrelease_controller_test.go b/internal/controller/helmrelease_controller_test.go index 090092c82..c3ee018fd 100644 --- a/internal/controller/helmrelease_controller_test.go +++ b/internal/controller/helmrelease_controller_test.go @@ -134,11 +134,20 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { }, } + mockErr := errors.New("mock get failure") r := &HelmReleaseReconciler{ Client: fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithStatusSubresource(&v2.HelmRelease{}). WithObjects(obj). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*sourcev1.HelmChart); ok { + return mockErr + } + return client.Get(ctx, key, obj, opts...) + }, + }). Build(), } r.APIReader = r.Client @@ -152,6 +161,39 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { })) }) + t.Run("waits for HelmChart to exist", func(t *testing.T) { + g := NewWithT(t) + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Status: v2.HelmReleaseStatus{ + HelmChart: "mock/chart", + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(obj). + Build(), + DependencyRequeueInterval: 10 * time.Second, + } + r.APIReader = r.Client + + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj, nil) + g.Expect(err).To(Equal(errWaitForDependency)) + g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval)) + + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"), + *conditions.FalseCondition(meta.ReadyCondition, v2.ArtifactFailedReason, "could not get Source object"), + })) + }) + t.Run("handles ACL error for HelmChart", func(t *testing.T) { g := NewWithT(t) @@ -323,6 +365,16 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { }, } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "values", + Namespace: "mock", + }, + Data: map[string][]byte{ + "foo": []byte("bar"), + }, + } + obj := &v2.HelmRelease{ ObjectMeta: metav1.ObjectMeta{ Name: "release", @@ -332,7 +384,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { ValuesFrom: []meta.ValuesReference{ { Kind: "Secret", - Name: "missing", + Name: "values", }, }, }, @@ -345,7 +397,7 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { Client: fake.NewClientBuilder(). WithScheme(NewTestScheme()). WithStatusSubresource(&v2.HelmRelease{}). - WithObjects(chart, obj). + WithObjects(chart, secret, obj). Build(), EventRecorder: record.NewFakeRecorder(32), } @@ -354,6 +406,69 @@ func TestHelmReleaseReconciler_reconcileRelease(t *testing.T) { _, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj, nil) g.Expect(err).To(HaveOccurred()) + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"), + *conditions.FalseCondition(meta.ReadyCondition, "ValuesError", "could not resolve Secret chart values reference 'mock/values' with key 'values.yaml'"), + })) + }) + + t.Run("waits for values reference to exist", func(t *testing.T) { + g := NewWithT(t) + + chart := &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: "chart", + Namespace: "mock", + Generation: 2, + }, + Spec: sourcev1.HelmChartSpec{ + Interval: metav1.Duration{Duration: 1 * time.Second}, + }, + Status: sourcev1.HelmChartStatus{ + ObservedGeneration: 2, + Artifact: &meta.Artifact{}, + Conditions: []metav1.Condition{ + { + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + }, + }, + }, + } + + obj := &v2.HelmRelease{ + ObjectMeta: metav1.ObjectMeta{ + Name: "release", + Namespace: "mock", + }, + Spec: v2.HelmReleaseSpec{ + ValuesFrom: []meta.ValuesReference{ + { + Kind: "Secret", + Name: "missing", + }, + }, + }, + Status: v2.HelmReleaseStatus{ + HelmChart: "mock/chart", + }, + } + + r := &HelmReleaseReconciler{ + Client: fake.NewClientBuilder(). + WithScheme(NewTestScheme()). + WithStatusSubresource(&v2.HelmRelease{}). + WithObjects(chart, obj). + Build(), + EventRecorder: record.NewFakeRecorder(32), + DependencyRequeueInterval: 10 * time.Second, + } + r.APIReader = r.Client + + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj, nil) + g.Expect(err).To(Equal(errWaitForDependency)) + g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval)) + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"), *conditions.FalseCondition(meta.ReadyCondition, "ValuesError", "could not resolve Secret chart values reference 'mock/missing' with key 'values.yaml'"), @@ -1119,11 +1234,13 @@ func TestHelmReleaseReconciler_reconcileReleaseFromHelmChartSource(t *testing.T) WithStatusSubresource(&v2.HelmRelease{}). WithObjects(obj). Build(), - EventRecorder: record.NewFakeRecorder(32), + EventRecorder: record.NewFakeRecorder(32), + DependencyRequeueInterval: 10 * time.Second, } - _, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj, nil) - g.Expect(err).To(HaveOccurred()) + res, err := r.reconcileRelease(context.TODO(), patch.NewSerialPatcher(obj, r.Client), obj, nil) + g.Expect(err).To(Equal(errWaitForDependency)) + g.Expect(res.RequeueAfter).To(Equal(r.DependencyRequeueInterval)) g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "Fulfilling prerequisites"),