fix: address PR review comments

Addresses all review comments from PR #2446:

1. Add validation for QPS and Burst values
   - QPS must be > 0
   - Burst must be >= 1
   - Returns clear error messages for invalid values

2. Fix client config loading
   - Use clientcmd.NewDefaultClientConfigLoadingRules() for proper fallback
   - Only override ExplicitPath when kubeconfig is provided
   - Ensures standard env var handling and ~/.kube/config fallback

3. Improve cache locking
   - Use double-check locking pattern to avoid holding mutex during client creation
   - Reduces contention when multiple goroutines create clients concurrently

4. Add comprehensive tests
   - Test validation of invalid QPS values
   - Test validation of invalid Burst values
   - Test successful creation with valid values
   - Ensure error messages are descriptive

5. Fix documentation link
   - Updated Kubernetes client-go rate limiting link to point to correct rest.Config documentation
   - Previous link pointed to API server event rate limiting

All tests pass and code quality checks succeed.

Signed-off-by: yxxhero <aiopsclub@163.com>

KUBEDOG_CONFIG.md

@@ -205,4 +205,4 @@ The default values (QPS=100, Burst=200) were chosen to:
 
 - [Issue #2445](https://github.com/helmfile/helmfile/issues/2445) - Original issue that led to configurable QPS/Burst
 - [Kubedog Documentation](https://github.com/werf/kubedog)
-- [Kubernetes Client Go Rate Limiting](https://kubernetes.io/docs/reference/config-api/apiserver-eventratelimit.v1alpha1/)
+- [Kubernetes client-go `rest.Config` QPS/Burst](https://pkg.go.dev/k8s.io/client-go/rest#Config)

pkg/kubedog/tracker.go

@@ -73,6 +73,13 @@ func NewTracker(config *TrackerConfig) (*Tracker, error) {
 		burst = *config.KubedogBurst
 	}
 
+	if qps <= 0 {
+		return nil, fmt.Errorf("invalid kubedog QPS %v: must be > 0", qps)
+	}
+	if burst < 1 {
+		return nil, fmt.Errorf("invalid kubedog burst %v: must be >= 1", burst)
+	}
+
 	clientSet, err := getOrCreateClient(config.KubeContext, kubeconfig, qps, burst)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize kubernetes client: %w", err)
@@ -101,18 +108,15 @@ func getOrCreateClient(kubeContext, kubeconfig string, qps float32, burst int) (
 	}
 
 	kubeInitMu.Lock()
-	defer kubeInitMu.Unlock()
-
 	if client, ok := clientCache[key]; ok {
+		kubeInitMu.Unlock()
 		return client, nil
 	}
+	kubeInitMu.Unlock()
 
-	var explicitPath string
+	loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
 	if kubeconfig != "" {
-		explicitPath = kubeconfig
-	}
-	loadingRules := &clientcmd.ClientConfigLoadingRules{
-		ExplicitPath: explicitPath,
+		loadingRules.ExplicitPath = kubeconfig
 	}
 
 	overrides := &clientcmd.ConfigOverrides{}
@@ -134,6 +138,13 @@ func getOrCreateClient(kubeContext, kubeconfig string, qps float32, burst int) (
 		return nil, fmt.Errorf("failed to create kubernetes client: %w", err)
 	}
 
+	kubeInitMu.Lock()
+	defer kubeInitMu.Unlock()
+
+	if existingClient, ok := clientCache[key]; ok {
+		return existingClient, nil
+	}
+
 	clientCache[key] = client
 
 	return client, nil

pkg/kubedog/tracker_test.go

@@ -128,3 +128,76 @@ func TestTrackerConfig_WithQPSBurst(t *testing.T) {
 	assert.Equal(t, float32(50.0), *config.KubedogQPS)
 	assert.Equal(t, 100, *config.KubedogBurst)
 }
+
+func TestNewTracker_InvalidQPS(t *testing.T) {
+	invalidQPS := float32(-1.0)
+	burst := 100
+
+	cfg := &TrackerConfig{
+		Logger:       nil,
+		Namespace:    "test-ns",
+		KubeContext:  "test-ctx",
+		Kubeconfig:   "/nonexistent/kubeconfig",
+		TrackOptions: NewTrackOptions(),
+		KubedogQPS:   &invalidQPS,
+		KubedogBurst: &burst,
+	}
+
+	tr, err := NewTracker(cfg)
+
+	assert.Error(t, err)
+	assert.Nil(t, tr)
+	assert.Contains(t, err.Error(), "invalid kubedog QPS")
+	assert.Contains(t, err.Error(), "must be > 0")
+}
+
+func TestNewTracker_InvalidBurst(t *testing.T) {
+	qps := float32(50.0)
+	invalidBurst := 0
+
+	cfg := &TrackerConfig{
+		Logger:       nil,
+		Namespace:    "test-ns",
+		KubeContext:  "test-ctx",
+		Kubeconfig:   "/nonexistent/kubeconfig",
+		TrackOptions: NewTrackOptions(),
+		KubedogQPS:   &qps,
+		KubedogBurst: &invalidBurst,
+	}
+
+	tr, err := NewTracker(cfg)
+
+	assert.Error(t, err)
+	assert.Nil(t, tr)
+	assert.Contains(t, err.Error(), "invalid kubedog burst")
+	assert.Contains(t, err.Error(), "must be >= 1")
+}
+
+func TestNewTracker_ValidQPSBurst(t *testing.T) {
+	qps := float32(50.0)
+	burst := 100
+
+	cfg := &TrackerConfig{
+		Logger:       nil,
+		Namespace:    "test-ns",
+		KubeContext:  "",
+		Kubeconfig:   "", // Will use default kubeconfig
+		TrackOptions: NewTrackOptions(),
+		KubedogQPS:   &qps,
+		KubedogBurst: &burst,
+	}
+
+	// This test may fail if no kubeconfig is available, which is expected
+	// in CI environments. The important part is that validation passes.
+	tr, err := NewTracker(cfg)
+
+	// If kubeconfig doesn't exist, we expect an error about loading kubeconfig,
+	// NOT an error about invalid QPS/Burst
+	if err != nil {
+		assert.NotContains(t, err.Error(), "invalid kubedog QPS")
+		assert.NotContains(t, err.Error(), "invalid kubedog burst")
+		assert.Nil(t, tr)
+	} else {
+		assert.NotNil(t, tr)
+	}
+}