optimize: avoid npe is env missing, and requeue if employer or employ… (

#72) * optimize: avoid npe is env missing, and requeue if employer or employees synced failed exist * replace requeue to backOff err * make reconcileAdapter construct failed if slb client get failed in alibaba_cloud_slb_controller
KusionStack · Aug 29, 2023 · 3a453bb · 3a453bb
1 parent 6dbef98
commit 3a453bb
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 31 deletions.
diff --git a/pkg/controllers/add_alibaba_cloud_slb.go b/pkg/controllers/add_alibaba_cloud_slb.go
@@ -28,5 +28,9 @@ func init() {
 }
 
 func Add(manager manager.Manager) error {
-	return resourceconsist.Add(manager, alibaba_cloud_slb.NewReconcileAdapter(manager.GetClient()))
+	reconcileAdapter, err := alibaba_cloud_slb.NewReconcileAdapter(manager.GetClient())
+	if err != nil {
+		return err
+	}
+	return resourceconsist.Add(manager, reconcileAdapter)
 }
diff --git a/pkg/controllers/alibaba_cloud_slb/alibaba_cloud_slb_client.go b/pkg/controllers/alibaba_cloud_slb/alibaba_cloud_slb_client.go
@@ -30,8 +30,6 @@ var (
 	slbAccessKeyID     string
 	slbAccessKeySecret string
 	slbEndpoint        string
-
-	alibabaCloudSlbClient *AlibabaCloudSlbClient
 )
 
 type AlibabaCloudSlbClient struct {
@@ -89,6 +87,4 @@ func init() {
 	if os.Getenv("ALIYUN_SLB_ENDPOINT") != "" {
 		slbEndpoint = os.Getenv("ALIYUN_SLB_ENDPOINT")
 	}
-
-	alibabaCloudSlbClient, _ = NewAlibabaCloudSlbClient()
 }
diff --git a/pkg/controllers/alibaba_cloud_slb/alibaba_cloud_slb_controller.go b/pkg/controllers/alibaba_cloud_slb/alibaba_cloud_slb_controller.go
@@ -31,12 +31,22 @@ var _ resourceconsist.ReconcileAdapter = &ReconcileAdapter{}
 
 type ReconcileAdapter struct {
 	client.Client
+	slbClient *AlibabaCloudSlbClient
 }
 
-func NewReconcileAdapter(c client.Client) *ReconcileAdapter {
-	return &ReconcileAdapter{
-		Client: c,
+func NewReconcileAdapter(c client.Client) (*ReconcileAdapter, error) {
+	slbClient, err := NewAlibabaCloudSlbClient()
+	if err != nil {
+		return nil, err
 	}
+	if slbClient == nil {
+		return nil, fmt.Errorf("alibaba cloud slb client is nil")
+	}
+
+	return &ReconcileAdapter{
+		Client:    c,
+		slbClient: slbClient,
+	}, nil
 }
 
 func (r *ReconcileAdapter) GetControllerName() string {
@@ -142,7 +152,7 @@ func (r *ReconcileAdapter) GetCurrentEmployee(ctx context.Context, employer clie
 	lbID := svc.GetLabels()[alibabaCloudSlbLbIdLabelKey]
 	bsExistUnderSlb := make(map[string]bool)
 	if lbID != "" {
-		backendServers, err := alibabaCloudSlbClient.GetBackendServers(lbID)
+		backendServers, err := r.slbClient.GetBackendServers(lbID)
 		if err != nil {
 			return nil, fmt.Errorf("get backend servers of slb failed, err: %s", err.Error())
 		}

diff --git a/pkg/controllers/resourceconsist/consister.go b/pkg/controllers/resourceconsist/consister.go
@@ -33,31 +33,32 @@ import (
 	"kusionstack.io/kafed/pkg/controllers/utils"
 )
 
-func (r *Consist) syncEmployer(ctx context.Context, employer client.Object, expectEmployerStatus, currentEmployerStatus []IEmployer) (bool, error) {
+func (r *Consist) syncEmployer(ctx context.Context, employer client.Object, expectEmployerStatus, currentEmployerStatus []IEmployer) (bool, bool, error) {
 	toCudEmployer, err := r.diffEmployer(expectEmployerStatus, currentEmployerStatus)
 	if err != nil {
-		return false, fmt.Errorf("diff employer failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("diff employer failed, err: %s", err.Error())
 	}
-	succCreate, _, err := r.adapter.CreateEmployer(employer, toCudEmployer.ToCreate)
+	succCreate, failCreate, err := r.adapter.CreateEmployer(employer, toCudEmployer.ToCreate)
 	if err != nil {
-		return false, fmt.Errorf("syncCreate failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("syncCreate failed, err: %s", err.Error())
 	}
-	succUpdate, _, err := r.adapter.UpdateEmployer(employer, toCudEmployer.ToUpdate)
+	succUpdate, failUpdate, err := r.adapter.UpdateEmployer(employer, toCudEmployer.ToUpdate)
 	if err != nil {
-		return false, fmt.Errorf("syncUpdate failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("syncUpdate failed, err: %s", err.Error())
 	}
-	succDelete, _, err := r.adapter.DeleteEmployer(employer, toCudEmployer.ToDelete)
+	succDelete, failDelete, err := r.adapter.DeleteEmployer(employer, toCudEmployer.ToDelete)
 	if err != nil {
-		return false, fmt.Errorf("syncDelete failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("syncDelete failed, err: %s", err.Error())
 	}
 
 	err = r.adapter.RecordEmployer(succCreate, succUpdate, succDelete)
 	if err != nil {
-		return false, fmt.Errorf("record employer failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("record employer failed, err: %s", err.Error())
 	}
 
 	isClean := len(toCudEmployer.Unchanged) == 0 && len(toCudEmployer.ToCreate) == 0 && len(toCudEmployer.ToUpdate) == 0 && len(toCudEmployer.ToDelete) == 0
-	return isClean, nil
+	cudFailedExist := len(failCreate) > 0 || len(failUpdate) > 0 || len(failDelete) > 0
+	return isClean, cudFailedExist, nil
 }
 
 func (r *Consist) diffEmployer(expectEmployer, currentEmployer []IEmployer) (ToCUDEmployer, error) {
@@ -174,11 +175,11 @@ func (r *Consist) diffEmployees(expectEmployees, currentEmployees []IEmployee) (
 	}, nil
 }
 
-func (r *Consist) syncEmployees(ctx context.Context, employer client.Object, expectEmployees, currentEmployees []IEmployee) (bool, error) {
+func (r *Consist) syncEmployees(ctx context.Context, employer client.Object, expectEmployees, currentEmployees []IEmployee) (bool, bool, error) {
 	// get expect/current employees diffEmployees
 	toCudEmployees, err := r.diffEmployees(expectEmployees, currentEmployees)
 	if err != nil {
-		return false, err
+		return false, false, err
 	}
 
 	// todo, to be removed, for demo
@@ -187,29 +188,30 @@ func (r *Consist) syncEmployees(ctx context.Context, employer client.Object, exp
 
 	succCreate, failCreate, err := r.adapter.CreateEmployees(employer, toCudEmployees.ToCreate)
 	if err != nil {
-		return false, fmt.Errorf("syncCreate failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("syncCreate failed, err: %s", err.Error())
 	}
-	succUpdate, _, err := r.adapter.UpdateEmployees(employer, toCudEmployees.ToUpdate)
+	succUpdate, failUpdate, err := r.adapter.UpdateEmployees(employer, toCudEmployees.ToUpdate)
 	if err != nil {
-		return false, fmt.Errorf("syncUpdate failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("syncUpdate failed, err: %s", err.Error())
 	}
 	succDelete, failDelete, err := r.adapter.DeleteEmployees(employer, toCudEmployees.ToDelete)
 	if err != nil {
-		return false, fmt.Errorf("syncDelete failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("syncDelete failed, err: %s", err.Error())
 	}
 
 	toAddLifecycleFlzEmployees, toDeleteLifecycleFlzEmployees := r.getToAddDeleteLifecycleFlzEmployees(
-		succCreate, failCreate, succDelete, failDelete, succUpdate, toCudEmployees.Unchanged)
+		succCreate, succDelete, succUpdate, toCudEmployees.Unchanged)
 
 	ns := employer.GetNamespace()
 	lifecycleFlz := GenerateLifecycleFinalizer(employer.GetName())
 	err = r.ensureLifecycleFinalizer(ctx, ns, lifecycleFlz, toAddLifecycleFlzEmployees, toDeleteLifecycleFlzEmployees)
 	if err != nil {
-		return false, fmt.Errorf("ensureLifecycleFinalizer failed, err: %s", err.Error())
+		return false, false, fmt.Errorf("ensureLifecycleFinalizer failed, err: %s", err.Error())
 	}
 
 	isClean := len(toCudEmployees.ToCreate) == 0 && len(toCudEmployees.ToUpdate) == 0 && len(toCudEmployees.Unchanged) == 0 && len(failDelete) == 0
-	return isClean, nil
+	cudFailedExist := len(failCreate) > 0 || len(failUpdate) > 0 || len(failDelete) > 0
+	return isClean, cudFailedExist, nil
 }
 
 // ensureExpectFinalizer add expected finalizer to employee's available condition anno
@@ -530,7 +532,7 @@ func (r *Consist) ensureLifecycleFinalizer(ctx context.Context, ns, lifecycleFlz
 	return err
 }
 
-func (r *Consist) getToAddDeleteLifecycleFlzEmployees(succCreate, failCreate, succDelete, failDelete, succUpdate, unchanged []IEmployee) ([]string, []string) {
+func (r *Consist) getToAddDeleteLifecycleFlzEmployees(succCreate, succDelete, succUpdate, unchanged []IEmployee) ([]string, []string) {
 	toAddLifecycleFlz := make([]string, len(succCreate)+len(succUpdate)+len(unchanged))
 	toDeleteLifecycleFlz := make([]string, len(succDelete)+len(succUpdate)+len(unchanged))
 	toAddIdx, toDeleteIdx := 0, 0

diff --git a/pkg/controllers/resourceconsist/resourceconsist_controller.go b/pkg/controllers/resourceconsist/resourceconsist_controller.go
@@ -18,6 +18,7 @@ package resourceconsist
 
 import (
 	"context"
+	"fmt"
 
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -181,7 +182,7 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec
 			"get current employer status failed: %s", err.Error())
 		return reconcile.Result{}, err
 	}
-	isCleanEmployer, err := r.syncEmployer(ctx, employer, expectEmployer, currentEmployer)
+	isCleanEmployer, syncEmployerFailedExist, err := r.syncEmployer(ctx, employer, expectEmployer, currentEmployer)
 	if err != nil {
 		r.logger.Errorf("sync employer status failed: %s", err.Error())
 		r.recorder.Eventf(employer, v1.EventTypeWarning, "syncEmployerFailed",
@@ -203,7 +204,7 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec
 			"get current employees status failed: %s", err.Error())
 		return reconcile.Result{}, err
 	}
-	isCleanEmployee, err := r.syncEmployees(ctx, employer, expectEmployees, currentEmployees)
+	isCleanEmployee, syncEmployeeFailedExist, err := r.syncEmployees(ctx, employer, expectEmployees, currentEmployees)
 	if err != nil {
 		r.logger.Errorf("sync employees status failed: %s", err.Error())
 		r.recorder.Eventf(employer, v1.EventTypeWarning, "syncEmployeesFailed",
@@ -221,6 +222,11 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec
 		}
 	}
 
+	if syncEmployerFailedExist || syncEmployeeFailedExist {
+		r.recorder.Eventf(employer, v1.EventTypeNormal, "ReconcileFailed", "employer or employees synced failed exist")
+		return reconcile.Result{}, fmt.Errorf("employer or employees synced failed exist")
+	}
+
 	r.recorder.Eventf(employer, v1.EventTypeNormal, "ReconcileSucceed", "")
 	return reconcile.Result{}, nil
 }