From 900f7cd6ed022e2c1d0dcbdb0aaa4bd5d035d7b8 Mon Sep 17 00:00:00 2001 From: Jiawei Zhao Date: Wed, 10 Jan 2024 12:24:57 +0800 Subject: [PATCH] refactor: refactor AsResult method in shared.go Signed-off-by: Phoeniix Zhao --- api/v1alpha1/xlinecluster_types.go | 4 +++ internal/controller/sharded.go | 19 ++----------- internal/controller/sharded_test.go | 42 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 17 deletions(-) create mode 100644 internal/controller/sharded_test.go diff --git a/api/v1alpha1/xlinecluster_types.go b/api/v1alpha1/xlinecluster_types.go index 820bd54e..e9cfce4b 100644 --- a/api/v1alpha1/xlinecluster_types.go +++ b/api/v1alpha1/xlinecluster_types.go @@ -23,6 +23,8 @@ import ( "k8s.io/apimachinery/pkg/types" ) +// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + // XlineCluster is the Schema for the xlineclusters API // +kubebuilder:object:root=true // +kubebuilder:subresource:status @@ -83,12 +85,14 @@ const ( StageComplete XlineClusterOprStage = "complete" ) +// XlineClusterRecStatus represents XlineCluster reconcile status type XlineClusterRecStatus struct { Stage XlineClusterOprStage `json:"stage,omitempty"` StageStatus OprStageStatus `json:"stageStatus,omitempty"` LastMessage string `json:"lastMessage,omitempty"` } +// XlineClusterSyncStatus represents XlineCluster sync status type XlineClusterSyncStatus struct { Image string `json:"image,omitempty"` StatefulSetRef NamespacedName `json:"statefulSetRef,omitempty"` diff --git a/internal/controller/sharded.go b/internal/controller/sharded.go index af348cd5..6b71ca56 100644 --- a/internal/controller/sharded.go +++ b/internal/controller/sharded.go @@ -32,15 +32,14 @@ func (r *StCtrlErrSet) AsResult() (ctrl.Result, error) { if r.Update != nil { errMap["update-status"] = r.Update } - mergedErr := MergeErrorsWithTag(errMap) - if mergedErr == nil { + if len(errMap) == 0 { if updateConflict { return ctrl.Result{Requeue: true}, nil } else { return ctrl.Result{}, nil } } else { - return ctrl.Result{Requeue: true}, mergedErr + return ctrl.Result{Requeue: true}, &MultiTaggedError{Errors: errMap} } } @@ -56,17 +55,3 @@ func (e *MultiTaggedError) Error() string { } return strings.Join(errStrs, "; ") } - -// MergeErrorsWithTag merges multiple errors into one with tags. -func MergeErrorsWithTag(errors map[string]error) *MultiTaggedError { - errMap := make(map[string]error) - for tag, err := range errors { - if err != nil { - errMap[tag] = err - } - } - if len(errMap) == 0 { - return nil - } - return &MultiTaggedError{Errors: errMap} -} diff --git a/internal/controller/sharded_test.go b/internal/controller/sharded_test.go new file mode 100644 index 00000000..2756203a --- /dev/null +++ b/internal/controller/sharded_test.go @@ -0,0 +1,42 @@ +package controller + +import ( + "errors" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + update_errors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" +) + +func TestStCtrlErrSet_AsResult(t *testing.T) { + // Test case 1: No errors + errSet := StCtrlErrSet{} + result, err := errSet.AsResult() + assert.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + + // Test case 2: Update conflict error + statusErr := update_errors.StatusError{ErrStatus: metav1.Status{ + Code: http.StatusConflict, + Reason: metav1.StatusReasonConflict, + }} + errSet = StCtrlErrSet{ + Update: error(&statusErr), + } + result, err = errSet.AsResult() + assert.NoError(t, err) + assert.Equal(t, ctrl.Result{Requeue: true}, result) + + // Test case 3: Other errors + errSet = StCtrlErrSet{ + Rec: errors.New("reconciliation error"), + Sync: errors.New("sync error"), + Update: errors.New("update status error"), + } + result, err = errSet.AsResult() + assert.Error(t, err) + assert.Equal(t, ctrl.Result{Requeue: true}, result) +}