You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by ji...@apache.org on 2026/02/25 16:02:43 UTC
(incubator-seata-k8s) branch master updated: optimize: optimize synchronizers.go context (#56)
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 <sl...@163.com>
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: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org