feat: add RecordErrorConditions in StatusRecordOptions (#49)

Author: WeichengWang1

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"
 )

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
 		}
 	}

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
 }

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() {

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