diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index dd648196..0b2ec2bd 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -556,13 +556,13 @@ type ControllerSetupFunc func(c ControllerSetup) error // - Deployed - a release for this CR is deployed (but not necessarily ready). // - ReleaseFailed - an installation or upgrade failed. // - Irreconcilable - an error occurred during reconciliation -func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, err error) { log := r.log.WithValues(strings.ToLower(r.gvk.Kind), req.NamespacedName) log.V(1).Info("Reconciliation triggered") obj := &unstructured.Unstructured{} obj.SetGroupVersionKind(*r.gvk) - err := r.client.Get(ctx, req.NamespacedName, obj) + err = r.client.Get(ctx, req.NamespacedName, obj) if apierrors.IsNotFound(err) { log.V(1).Info("Resource %s/%s not found, nothing to do", req.NamespacedName.Namespace, req.NamespacedName.Name) return ctrl.Result{}, nil diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 9114ada3..09dde645 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -738,8 +738,15 @@ var _ = Describe("Reconciler", func() { Patch: func(ctx context.Context, _ client.WithWatch, fakeObj client.Object, patch client.Patch, opts ...client.PatchOption) error { return mgr.GetClient().Patch(ctx, fakeObj, patch, opts...) }, - SubResource: func(_ client.WithWatch, subresource string) client.SubResourceClient { - return mgr.GetClient().SubResource(subresource) + SubResourceUpdate: func(ctx context.Context, _ client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + err := mgr.GetClient().SubResource(subResourceName).Update(ctx, obj, opts...) + if err != nil { + if apierrors.IsConflict(err) { + // workaround https://github.com/kubernetes/kubernetes/issues/124347 + return apierrors.NewNotFound(gvk.GroupVersion().WithResource("testapps").GroupResource(), obj.GetName()) + } + } + return nil }, }).WithStatusSubresource(obj).Build() r.client = cl @@ -752,6 +759,44 @@ var _ = Describe("Reconciler", func() { }) }) }) + When("errors that were not 'NotFound' occurred while applying CR status", func() { + It("should propagate apply errors to reconciler", func() { + By("configuring a client that will error during apply", func() { + cl := fake.NewClientBuilder().WithObjects(obj).WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, _ client.WithWatch, fakeObj client.Object, opts ...client.CreateOption) error { + return mgr.GetClient().Create(ctx, fakeObj, opts...) + }, + Delete: func(ctx context.Context, _ client.WithWatch, fakeObj client.Object, opts ...client.DeleteOption) error { + return mgr.GetClient().Delete(ctx, fakeObj, opts...) + }, + DeleteAllOf: func(ctx context.Context, _ client.WithWatch, fakeObj client.Object, opts ...client.DeleteAllOfOption) error { + return mgr.GetClient().DeleteAllOf(ctx, fakeObj, opts...) + }, + Update: func(ctx context.Context, _ client.WithWatch, fakeObj client.Object, opts ...client.UpdateOption) error { + return mgr.GetClient().Update(ctx, fakeObj, opts...) + }, + Patch: func(ctx context.Context, _ client.WithWatch, fakeObj client.Object, patch client.Patch, opts ...client.PatchOption) error { + return mgr.GetClient().Patch(ctx, fakeObj, patch, opts...) + }, + SubResourceUpdate: func(_ context.Context, _ client.Client, _ string, _ client.Object, _ ...client.SubResourceUpdateOption) error { + return apierrors.NewBadRequest("XXXInvalidXXX") + }, + }).WithStatusSubresource(obj).Build() + r.client = cl + }) + + By("propagating the error from status update", func() { + res, err := r.Reconcile(ctx, req) + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsBadRequest(err)).To(BeTrue()) + + var statusErr apierrors.APIStatus + Expect(errors.As(err, &statusErr)).To(BeTrue()) + Expect(statusErr.Status().Message).To(Equal("XXXInvalidXXX")) + }) + }) + }) When("CR is deleted, release is not present, but uninstall finalizer exists", func() { It("removes the finalizer", func() { By("adding the uninstall finalizer and deleting the CR", func() {