This is an automated email from the ASF dual-hosted git repository.

jimin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-seata-k8s.git


The following commit(s) were added to refs/heads/master by this push:
     new 4ded0af  optimize: optimize synchronizers.go context (#56)
4ded0af is described below

commit 4ded0afcd887a488a95a143507a4d010e79de85a
Author: jimin <[email protected]>
AuthorDate: Thu Feb 26 00:02:38 2026 +0800

    optimize: optimize synchronizers.go context (#56)
---
 pkg/seata/synchronizers.go      |  54 ++++++++---
 pkg/seata/synchronizers_test.go | 198 +++++++++++++++++++++++++++++++++-------
 2 files changed, 206 insertions(+), 46 deletions(-)

diff --git a/pkg/seata/synchronizers.go b/pkg/seata/synchronizers.go
index ba0defb..9622965 100644
--- a/pkg/seata/synchronizers.go
+++ b/pkg/seata/synchronizers.go
@@ -43,10 +43,22 @@ const (
 )
 
 func SyncService(curr *apiv1.Service, next *apiv1.Service) {
+       if curr == nil {
+               panic("SyncService: current service cannot be nil")
+       }
+       if next == nil {
+               panic("SyncService: next service cannot be nil")
+       }
        curr.Spec.Ports = next.Spec.Ports
 }
 
 func SyncStatefulSet(curr *appsv1.StatefulSet, next *appsv1.StatefulSet) {
+       if curr == nil {
+               panic("SyncStatefulSet: current statefulset cannot be nil")
+       }
+       if next == nil {
+               panic("SyncStatefulSet: next statefulset cannot be nil")
+       }
        curr.Spec.Template = next.Spec.Template
        curr.Spec.Replicas = next.Spec.Replicas
 }
@@ -58,7 +70,14 @@ type rspData struct {
        Success bool   `json:"success"`
 }
 
-func changeCluster(s *seatav1alpha1.SeataServer, i int32, username string, 
password string) error {
+func changeCluster(ctx context.Context, s *seatav1alpha1.SeataServer, i int32, 
username string, password string) error {
+       if s == nil {
+               return fmt.Errorf("changeCluster: SeataServer cannot be nil")
+       }
+       if ctx == nil {
+               ctx = context.Background()
+       }
+
        // Create HTTP client with timeout
        client := &http.Client{
                Timeout: httpClientTimeout,
@@ -73,10 +92,10 @@ func changeCluster(s *seatav1alpha1.SeataServer, i int32, 
username string, passw
        }
 
        loginURL := fmt.Sprintf("http://%s/api/v1/auth/login";, host)
-       ctx, cancel := context.WithTimeout(context.Background(), 
httpRequestTimeout)
-       defer cancel()
+       loginCtx, loginCancel := context.WithTimeout(ctx, httpRequestTimeout)
+       defer loginCancel()
 
-       req, err := http.NewRequestWithContext(ctx, "POST", loginURL, 
bytes.NewBuffer(jsonValue))
+       req, err := http.NewRequestWithContext(loginCtx, "POST", loginURL, 
bytes.NewBuffer(jsonValue))
        if err != nil {
                return fmt.Errorf("failed to create login request: %w", err)
        }
@@ -110,10 +129,10 @@ func changeCluster(s *seatav1alpha1.SeataServer, i int32, 
username string, passw
        targetURL := 
fmt.Sprintf("http://%s/metadata/v1/changeCluster?raftClusterStr=%s";,
                host, url.QueryEscape(utils.ConcatRaftServerAddress(s)))
 
-       ctx, cancel = context.WithTimeout(context.Background(), 
httpRequestTimeout)
-       defer cancel()
+       clusterCtx, clusterCancel := context.WithTimeout(ctx, 
httpRequestTimeout)
+       defer clusterCancel()
 
-       req, err = http.NewRequestWithContext(ctx, "POST", targetURL, nil)
+       req, err = http.NewRequestWithContext(clusterCtx, "POST", targetURL, 
nil)
        if err != nil {
                return fmt.Errorf("failed to create changeCluster request: %w", 
err)
        }
@@ -146,22 +165,31 @@ func changeCluster(s *seatav1alpha1.SeataServer, i int32, 
username string, passw
 }
 
 func SyncRaftCluster(ctx context.Context, s *seatav1alpha1.SeataServer, 
username string, password string) error {
+       if s == nil {
+               return fmt.Errorf("SyncRaftCluster: SeataServer cannot be nil")
+       }
+       if ctx == nil {
+               ctx = context.Background()
+       }
+
        logger := log.FromContext(ctx)
        group, childContext := errgroup.WithContext(ctx)
 
        for i := int32(0); i < s.Spec.Replicas; i++ {
                finalI := i
                group.Go(func() error {
+                       // Check if context is already cancelled before 
proceeding
                        select {
                        case <-childContext.Done():
-                               return nil
+                               return childContext.Err()
                        default:
-                               err := changeCluster(s, finalI, username, 
password)
-                               if err != nil {
-                                       logger.Error(err, fmt.Sprintf("fail to 
SyncRaftCluster at %d-th pod", finalI))
-                               }
-                               return err
                        }
+
+                       err := changeCluster(childContext, s, finalI, username, 
password)
+                       if err != nil {
+                               logger.Error(err, fmt.Sprintf("fail to 
SyncRaftCluster at %d-th pod", finalI))
+                       }
+                       return err
                })
        }
        return group.Wait()
diff --git a/pkg/seata/synchronizers_test.go b/pkg/seata/synchronizers_test.go
index ba028c9..0b49536 100644
--- a/pkg/seata/synchronizers_test.go
+++ b/pkg/seata/synchronizers_test.go
@@ -272,21 +272,45 @@ func TestSyncStatefulSet(t *testing.T) {
 
 func TestChangeCluster_ErrorHandling(t *testing.T) {
        // Test error handling of changeCluster
-       // Without a real Seata server, this will trigger error path
+       // Without a real Seata server, connection should fail with an error
        tests := []struct {
                name     string
+               ctx      context.Context
                username string
                password string
                index    int32
        }{
                {
                        name:     "empty credentials",
+                       ctx:      context.Background(),
                        username: "",
                        password: "",
                        index:    0,
                },
                {
                        name:     "with credentials",
+                       ctx:      context.Background(),
+                       username: "admin",
+                       password: "admin",
+                       index:    0,
+               },
+               {
+                       name:     "invalid index",
+                       ctx:      context.Background(),
+                       username: "admin",
+                       password: "admin",
+                       index:    999,
+               },
+               {
+                       name:     "nil context (should use default)",
+                       ctx:      nil,
+                       username: "admin",
+                       password: "admin",
+                       index:    0,
+               },
+               {
+                       name:     "nil SeataServer",
+                       ctx:      context.Background(),
                        username: "admin",
                        password: "admin",
                        index:    0,
@@ -295,12 +319,17 @@ func TestChangeCluster_ErrorHandling(t *testing.T) {
 
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
-                       seataServer := createTestSeataServer("test-seata", 
"default", 3)
-                       err := changeCluster(seataServer, tt.index, 
tt.username, tt.password)
+                       var seataServer *seatav1alpha1.SeataServer
+                       if tt.name != "nil SeataServer" {
+                               seataServer = 
createTestSeataServer("test-seata", "default", 3)
+                       }
+
+                       err := changeCluster(tt.ctx, seataServer, tt.index, 
tt.username, tt.password)
+                       // Since there's no real Seata server (or nil server), 
we expect an error
                        if err == nil {
-                               t.Log("changeCluster returned no error 
(expected as there is no real server)")
+                               t.Error("Expected error when connecting to 
non-existent server, but got nil")
                        } else {
-                               t.Logf("changeCluster returned expected error: 
%v", err)
+                               t.Logf("Got expected error: %v", err)
                        }
                })
        }
@@ -308,47 +337,150 @@ func TestChangeCluster_ErrorHandling(t *testing.T) {
 
 func TestSyncRaftCluster_ErrorHandling(t *testing.T) {
        // Test error handling of SyncRaftCluster
-       // Without a real Seata server, this will test error path
-       ctx := context.Background()
-       seataServer := createTestSeataServer("test-seata", "default", 3)
+       // Without a real Seata server, all goroutines should fail
+       tests := []struct {
+               name     string
+               ctx      context.Context
+               server   *seatav1alpha1.SeataServer
+               replicas int32
+               username string
+               password string
+       }{
+               {
+                       name:     "single replica",
+                       ctx:      context.Background(),
+                       replicas: 1,
+                       username: "admin",
+                       password: "admin",
+               },
+               {
+                       name:     "multiple replicas",
+                       ctx:      context.Background(),
+                       replicas: 3,
+                       username: "admin",
+                       password: "admin",
+               },
+               {
+                       name:     "nil context (should use default)",
+                       ctx:      nil,
+                       replicas: 1,
+                       username: "admin",
+                       password: "admin",
+               },
+               {
+                       name:     "nil SeataServer",
+                       ctx:      context.Background(),
+                       server:   nil,
+                       username: "admin",
+                       password: "admin",
+               },
+       }
 
-       err := SyncRaftCluster(ctx, seataServer, "admin", "admin")
-       if err != nil {
-               t.Logf("SyncRaftCluster returned expected error (no real 
server): %v", err)
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       var seataServer *seatav1alpha1.SeataServer
+                       if tt.server == nil && tt.name != "nil SeataServer" {
+                               seataServer = 
createTestSeataServer("test-seata", "default", tt.replicas)
+                       } else {
+                               seataServer = tt.server
+                       }
+
+                       err := SyncRaftCluster(tt.ctx, seataServer, 
tt.username, tt.password)
+                       // Since there's no real Seata server (or nil server), 
we expect an error
+                       if err == nil {
+                               t.Error("Expected error when connecting to 
non-existent server, but got nil")
+                       } else {
+                               t.Logf("Got expected error: %v", err)
+                       }
+               })
        }
 }
 
 // Boundary condition test: nil input
 func TestSyncService_NilInput(t *testing.T) {
-       defer func() {
-               if r := recover(); r != nil {
-                       t.Logf("Expected panic captured: %v", r)
-               }
-       }()
+       tests := []struct {
+               name string
+               curr *apiv1.Service
+               next *apiv1.Service
+       }{
+               {
+                       name: "nil current service",
+                       curr: nil,
+                       next: &apiv1.Service{},
+               },
+               {
+                       name: "nil next service",
+                       curr: &apiv1.Service{},
+                       next: nil,
+               },
+               {
+                       name: "both nil",
+                       curr: nil,
+                       next: nil,
+               },
+       }
 
-       // Test boundary case of nil input
-       var curr *apiv1.Service
-       next := &apiv1.Service{}
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       didPanic := false
+                       func() {
+                               defer func() {
+                                       if r := recover(); r != nil {
+                                               didPanic = true
+                                               t.Logf("Expected panic 
captured: %v", r)
+                                       }
+                               }()
+                               SyncService(tt.curr, tt.next)
+                       }()
 
-       // This should panic because curr is nil
-       SyncService(curr, next)
-       t.Error("Expected panic did not occur")
+                       if !didPanic {
+                               t.Error("Expected panic did not occur")
+                       }
+               })
+       }
 }
 
 func TestSyncStatefulSet_NilInput(t *testing.T) {
-       defer func() {
-               if r := recover(); r != nil {
-                       t.Logf("Expected panic captured: %v", r)
-               }
-       }()
+       tests := []struct {
+               name string
+               curr *appsv1.StatefulSet
+               next *appsv1.StatefulSet
+       }{
+               {
+                       name: "nil current statefulset",
+                       curr: nil,
+                       next: &appsv1.StatefulSet{},
+               },
+               {
+                       name: "nil next statefulset",
+                       curr: &appsv1.StatefulSet{},
+                       next: nil,
+               },
+               {
+                       name: "both nil",
+                       curr: nil,
+                       next: nil,
+               },
+       }
 
-       // Test boundary case of nil input
-       var curr *appsv1.StatefulSet
-       next := &appsv1.StatefulSet{}
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       didPanic := false
+                       func() {
+                               defer func() {
+                                       if r := recover(); r != nil {
+                                               didPanic = true
+                                               t.Logf("Expected panic 
captured: %v", r)
+                                       }
+                               }()
+                               SyncStatefulSet(tt.curr, tt.next)
+                       }()
 
-       // This should panic because curr is nil
-       SyncStatefulSet(curr, next)
-       t.Error("Expected panic did not occur")
+                       if !didPanic {
+                               t.Error("Expected panic did not occur")
+                       }
+               })
+       }
 }
 
 // Helper function: create test SeataServer with custom parameters


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to