From d08727f4729973d5bb45861f2d271b25d6e61447 Mon Sep 17 00:00:00 2001 From: WeichengWang Date: Thu, 9 Oct 2025 16:18:43 +0800 Subject: [PATCH] feat: add RecordErrorConditions in StatusRecordOptions --- pkg/frame/controller/consts.go | 6 +++++ .../controller/resourceconsist_controller.go | 23 ++++++++++++++++++ .../resourceconsist_controller_suite_test.go | 20 ++++++++++++++++ .../resourceconsit_controller_test.go | 24 +++++++++++++++++++ pkg/frame/controller/types.go | 6 +++++ 5 files changed, 79 insertions(+) diff --git a/pkg/frame/controller/consts.go b/pkg/frame/controller/consts.go index 5ea9bf2..a85c1bf 100644 --- a/pkg/frame/controller/consts.go +++ b/pkg/frame/controller/consts.go @@ -30,8 +30,14 @@ const ( EnsureEmployerCleanFinalizerFailed = "EnsureEmployerCleanFinalizerFailed" EnsureEmployerCleanFinalizerSucceed = "EnsureEmployerCleanFinalizerSucceed" EnsureExpectedFinalizerFailed = "EnsureExpectedFinalizerFailed" + GetExpectedEmployerFailed = "GetExpectedEmployerFailed" + GetCurrentEmployerFailed = "GetCurrentEmployerFailed" SyncEmployerFailed = "SyncEmployerFailed" + GetExpectedEmployeesFailed = "GetExpectedEmployeesFailed" + GetCurrentEmployeesFailed = "GetCurrentEmployeesFailed" SyncEmployeesFailed = "SyncEmployeesFailed" CleanEmployerCleanFinalizerFailed = "CleanEmployerCleanFinalizerFailed" CleanEmployerCleanFinalizerSucceed = "CleanEmployerCleanFinalizerSucceed" + RecordStatusesFailed = "RecordStatusesFailed" + RecordErrorConditionsFailed = "RecordErrorConditionsFailed" ) diff --git a/pkg/frame/controller/resourceconsist_controller.go b/pkg/frame/controller/resourceconsist_controller.go index bdeab3f..4ed0c8c 100644 --- a/pkg/frame/controller/resourceconsist_controller.go +++ b/pkg/frame/controller/resourceconsist_controller.go @@ -159,6 +159,19 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec return reconcile.Result{}, err } + defer func() { + if err != nil { + if recordOptions, ok := r.adapter.(StatusRecordOptions); ok { + err = recordOptions.RecordErrorConditions(ctx, employer, err) + if err != nil { + logger.Error(err, "record error conditions failed") + r.recorder.Eventf(employer, corev1.EventTypeWarning, RecordErrorConditionsFailed, + "record error conditions failed: %s", err.Error()) + } + } + } + }() + // Ensure employer-clean finalizer firstly, employer-clean finalizer should be cleaned at the end updated, err := r.ensureEmployerCleanFlz(ctx, employer) if err != nil { @@ -186,11 +199,15 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec expectedEmployer, err := r.adapter.GetExpectedEmployer(ctx, employer) if err != nil { logger.Error(err, "get expect employer failed") + r.recorder.Eventf(employer, corev1.EventTypeWarning, GetExpectedEmployerFailed, + "get expect employer failed: %s", err.Error()) return reconcile.Result{}, err } currentEmployer, err := r.adapter.GetCurrentEmployer(ctx, employer) if err != nil { logger.Error(err, "get current employer failed") + r.recorder.Eventf(employer, corev1.EventTypeWarning, GetCurrentEmployerFailed, + "get current employer failed: %s", err.Error()) return reconcile.Result{}, err } isCleanEmployer, syncEmployerFailedExist, cudEmployerResults, err := r.syncEmployer(ctx, employer, expectedEmployer, currentEmployer) @@ -205,11 +222,15 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec expectedEmployees, err := r.adapter.GetExpectedEmployee(ctx, employer) if err != nil { logger.Error(err, "get expect employees failed") + r.recorder.Eventf(employer, corev1.EventTypeWarning, GetExpectedEmployeesFailed, + "get expect employees failed: %s", err.Error()) return reconcile.Result{}, err } currentEmployees, err := r.adapter.GetCurrentEmployee(ctx, employer) if err != nil { logger.Error(err, "get current employees failed") + r.recorder.Eventf(employer, corev1.EventTypeWarning, GetCurrentEmployeesFailed, + "get current employees failed: %s", err.Error()) return reconcile.Result{}, err } isCleanEmployee, syncEmployeeFailedExist, cudEmployeeResults, err := r.syncEmployees(ctx, employer, expectedEmployees, currentEmployees) @@ -245,6 +266,8 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec err = recordOptions.RecordStatuses(ctx, employer, cudEmployerResults, cudEmployeeResults) if err != nil { logger.Error(err, "record status failed") + r.recorder.Eventf(employer, corev1.EventTypeWarning, RecordStatusesFailed, + "record status failed: %s", err.Error()) return reconcile.Result{}, err } } diff --git a/pkg/frame/controller/resourceconsist_controller_suite_test.go b/pkg/frame/controller/resourceconsist_controller_suite_test.go index 6d6802a..a201a46 100644 --- a/pkg/frame/controller/resourceconsist_controller_suite_test.go +++ b/pkg/frame/controller/resourceconsist_controller_suite_test.go @@ -35,6 +35,7 @@ type DemoControllerAdapter struct { var _ ReconcileAdapter = &DemoControllerAdapter{} var _ ReconcileLifecycleOptions = &DemoControllerAdapter{} +var _ StatusRecordOptions = &DemoControllerAdapter{} var needRecordEmployees = false @@ -45,6 +46,25 @@ func NewDemoReconcileAdapter(c client.Client, rc *DemoResourceProviderClient) Re } } +func (r *DemoControllerAdapter) RecordStatuses(ctx context.Context, employer client.Object, + cudEmployerResults CUDEmployerResults, cudEmployeeResults CUDEmployeeResults) error { + if employer.GetAnnotations()["resource-consist.kusionstack.io/demo-condition"] != "" { + employer.GetAnnotations()["resource-consist.kusionstack.io/demo-condition"] = "" + return r.Update(ctx, employer) + } + return nil +} + +func (r *DemoControllerAdapter) RecordErrorConditions(ctx context.Context, employer client.Object, err error) error { + annos := employer.GetAnnotations() + if annos == nil { + annos = make(map[string]string) + } + annos["resource-consist.kusionstack.io/demo-condition"] = err.Error() + employer.SetAnnotations(annos) + return r.Update(ctx, employer) +} + func (r *DemoControllerAdapter) FollowPodOpsLifeCycle() bool { return true } diff --git a/pkg/frame/controller/resourceconsit_controller_test.go b/pkg/frame/controller/resourceconsit_controller_test.go index 4bf365e..21c8208 100644 --- a/pkg/frame/controller/resourceconsit_controller_test.go +++ b/pkg/frame/controller/resourceconsit_controller_test.go @@ -493,6 +493,18 @@ var _ = Describe("resource-consist-controller", func() { return false }, 3*time.Second, 100*time.Millisecond).Should(BeTrue()) + Eventually(func() bool { + tmp := &corev1.Service{} + err := mgr.GetClient().Get(context.Background(), types.NamespacedName{ + Name: svc2.Name, + Namespace: svc2.Namespace, + }, tmp) + if err != nil { + return false + } + return tmp.GetAnnotations()["resource-consist.kusionstack.io/demo-condition"] == "syncCreate failed, err: fake err" + }, 3*time.Second, 100*time.Millisecond).Should(BeTrue()) + demoResourceVipStatusInProvider.Store(svc2.Name, DemoServiceDetails{ RemoteVIP: "demo-remote-VIP", RemoteVIPQPS: 100, @@ -502,6 +514,18 @@ var _ = Describe("resource-consist-controller", func() { details, exist := demoResourceVipStatusInProvider.Load(svc2.Name) return exist && details.(DemoServiceDetails).RemoteVIP == "demo-remote-VIP" && details.(DemoServiceDetails).RemoteVIPQPS == 100 }, 3*time.Second, 100*time.Millisecond).Should(BeTrue()) + + Eventually(func() bool { + tmp := &corev1.Service{} + err := mgr.GetClient().Get(context.Background(), types.NamespacedName{ + Name: svc2.Name, + Namespace: svc2.Namespace, + }, tmp) + if err != nil { + return false + } + return tmp.GetAnnotations()["resource-consist.kusionstack.io/demo-condition"] == "" + }, 3*time.Second, 100*time.Millisecond).Should(BeTrue()) }) It("create rs resp failed, but actually created in backend provider", func() { diff --git a/pkg/frame/controller/types.go b/pkg/frame/controller/types.go index 5ab74ed..cc218ba 100644 --- a/pkg/frame/controller/types.go +++ b/pkg/frame/controller/types.go @@ -68,8 +68,14 @@ type ExpectedFinalizerRecordOptions interface { // StatusRecordOptions defines methods of record something for adapters type StatusRecordOptions interface { + // RecordStatuses records statuses of employer and employees, called at the end of successful reconcile. + // cudEmployerResults and cudEmployeeResults are the results of create/update/delete operations on employer and employees RecordStatuses(ctx context.Context, employer client.Object, cudEmployerResults CUDEmployerResults, cudEmployeeResults CUDEmployeeResults) error + + // RecordErrorConditions records error conditions, called at the end of failed reconcile. + // usually, something recorded in RecordErrorConditions should be erased in RecordStatuses. + RecordErrorConditions(ctx context.Context, employer client.Object, err error) error } // ReconcileLifecycleOptions defines whether PodOpsLifecycle followed