From f72d93d4bfe28471501b238827a716b1a88d43d8 Mon Sep 17 00:00:00 2001 From: Harold Cheng Date: Tue, 21 Oct 2025 22:53:37 +0800 Subject: [PATCH 1/3] fix: don't ignore not found error in its controller --- pkg/controller/kubebuilderx/plan_builder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/kubebuilderx/plan_builder.go b/pkg/controller/kubebuilderx/plan_builder.go index de6ba47799e..e7d67438839 100644 --- a/pkg/controller/kubebuilderx/plan_builder.go +++ b/pkg/controller/kubebuilderx/plan_builder.go @@ -281,7 +281,7 @@ func (b *PlanBuilder) updateObject(ctx context.Context, vertex *model.ObjectVert err = b.cli.Update(ctx, vertex.Obj, clientOption(vertex)) reason = "SuccessfulUpdate" } - if err != nil && !apierrors.IsNotFound(err) { + if err != nil { return err } b.emitEvent(vertex.Obj, reason, model.UPDATE) @@ -299,7 +299,7 @@ func (b *PlanBuilder) patchObject(ctx context.Context, vertex *model.ObjectVerte err = b.cli.Patch(ctx, vertex.Obj, patch, clientOption(vertex)) reason = "SuccessfulPatch" } - if err != nil && !apierrors.IsNotFound(err) { + if err != nil { return err } b.emitEvent(vertex.Obj, reason, model.PATCH) From bc72205138121d6a715b7ca563a6531715c04a28 Mon Sep 17 00:00:00 2001 From: Harold Cheng Date: Tue, 21 Oct 2025 23:17:14 +0800 Subject: [PATCH 2/3] more detailed error --- pkg/controller/kubebuilderx/controller.go | 2 +- pkg/controller/kubebuilderx/plan_builder.go | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/controller/kubebuilderx/controller.go b/pkg/controller/kubebuilderx/controller.go index 383157fef31..7130a7582fa 100644 --- a/pkg/controller/kubebuilderx/controller.go +++ b/pkg/controller/kubebuilderx/controller.go @@ -164,7 +164,7 @@ func (c *controller) emitFailureEvent() { return } // TODO(free6om): make error message user-friendly - c.tree.EventRecorder.Eventf(c.tree.GetRoot(), corev1.EventTypeWarning, "FailedReconcile", "reconcile failed: %s", c.err.Error()) + c.tree.EventRecorder.Eventf(c.tree.GetRoot(), corev1.EventTypeWarning, "FailedReconcile", "%s", c.err.Error()) } func NewController(ctx context.Context, cli client.Client, req ctrl.Request, recorder record.EventRecorder, logger logr.Logger) Controller { diff --git a/pkg/controller/kubebuilderx/plan_builder.go b/pkg/controller/kubebuilderx/plan_builder.go index e7d67438839..1d91cdbd390 100644 --- a/pkg/controller/kubebuilderx/plan_builder.go +++ b/pkg/controller/kubebuilderx/plan_builder.go @@ -33,6 +33,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -265,7 +266,7 @@ func (b *PlanBuilder) defaultWalkFunc(v graph.Vertex) error { func (b *PlanBuilder) createObject(ctx context.Context, vertex *model.ObjectVertex) error { err := b.cli.Create(ctx, vertex.Obj, clientOption(vertex)) if err != nil && !apierrors.IsAlreadyExists(err) { - return err + return fmt.Errorf("create %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } b.emitEvent(vertex.Obj, "SuccessfulCreate", model.CREATE) return nil @@ -282,7 +283,7 @@ func (b *PlanBuilder) updateObject(ctx context.Context, vertex *model.ObjectVert reason = "SuccessfulUpdate" } if err != nil { - return err + return fmt.Errorf("update %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } b.emitEvent(vertex.Obj, reason, model.UPDATE) return nil @@ -300,7 +301,7 @@ func (b *PlanBuilder) patchObject(ctx context.Context, vertex *model.ObjectVerte reason = "SuccessfulPatch" } if err != nil { - return err + return fmt.Errorf("patch %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } b.emitEvent(vertex.Obj, reason, model.PATCH) return nil @@ -314,14 +315,14 @@ func (b *PlanBuilder) deleteObject(ctx context.Context, vertex *model.ObjectVert if len(finalizer) > 0 && controllerutil.RemoveFinalizer(vertex.Obj, finalizer) { err := b.cli.Update(ctx, vertex.Obj, clientOption(vertex)) if err != nil && !apierrors.IsNotFound(err) { - b.transCtx.logger.Error(err, fmt.Sprintf("delete %T error: %s", vertex.Obj, vertex.Obj.GetName())) - return err + b.transCtx.logger.Error(err, fmt.Sprintf("delete %T error: %s", vertex.Obj, klog.KObj(vertex.Obj))) + return fmt.Errorf("delete %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } } if !model.IsObjectDeleting(vertex.Obj) { err := b.cli.Delete(ctx, vertex.Obj, clientOption(vertex)) if err != nil && !apierrors.IsNotFound(err) { - return err + return fmt.Errorf("delete %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } b.emitEvent(vertex.Obj, "SuccessfulDelete", model.DELETE) } From 2cfad73068be97cc2fc5a860c692e2c1773e4787 Mon Sep 17 00:00:00 2001 From: Harold Cheng Date: Mon, 3 Nov 2025 17:49:36 +0800 Subject: [PATCH 3/3] review comments --- pkg/controller/kubebuilderx/plan_builder.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/controller/kubebuilderx/plan_builder.go b/pkg/controller/kubebuilderx/plan_builder.go index 1d91cdbd390..7c36b2aa7b2 100644 --- a/pkg/controller/kubebuilderx/plan_builder.go +++ b/pkg/controller/kubebuilderx/plan_builder.go @@ -30,7 +30,6 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/equality" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" @@ -265,7 +264,7 @@ func (b *PlanBuilder) defaultWalkFunc(v graph.Vertex) error { func (b *PlanBuilder) createObject(ctx context.Context, vertex *model.ObjectVertex) error { err := b.cli.Create(ctx, vertex.Obj, clientOption(vertex)) - if err != nil && !apierrors.IsAlreadyExists(err) { + if err != nil { return fmt.Errorf("create %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } b.emitEvent(vertex.Obj, "SuccessfulCreate", model.CREATE) @@ -314,14 +313,13 @@ func (b *PlanBuilder) deleteObject(ctx context.Context, vertex *model.ObjectVert } if len(finalizer) > 0 && controllerutil.RemoveFinalizer(vertex.Obj, finalizer) { err := b.cli.Update(ctx, vertex.Obj, clientOption(vertex)) - if err != nil && !apierrors.IsNotFound(err) { - b.transCtx.logger.Error(err, fmt.Sprintf("delete %T error: %s", vertex.Obj, klog.KObj(vertex.Obj))) + if err != nil { return fmt.Errorf("delete %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } } if !model.IsObjectDeleting(vertex.Obj) { err := b.cli.Delete(ctx, vertex.Obj, clientOption(vertex)) - if err != nil && !apierrors.IsNotFound(err) { + if err != nil { return fmt.Errorf("delete %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } b.emitEvent(vertex.Obj, "SuccessfulDelete", model.DELETE) @@ -331,7 +329,7 @@ func (b *PlanBuilder) deleteObject(ctx context.Context, vertex *model.ObjectVert func (b *PlanBuilder) statusObject(ctx context.Context, vertex *model.ObjectVertex) error { if err := b.cli.Status().Update(ctx, vertex.Obj, clientOption(vertex)); err != nil { - return err + return fmt.Errorf("status %T %s failed: %w", vertex.Obj, klog.KObj(vertex.Obj), err) } return nil }