diff --git a/KUBEDOG_CONFIG.md b/KUBEDOG_CONFIG.md index fd87d117..1f095f8f 100644 --- a/KUBEDOG_CONFIG.md +++ b/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) diff --git a/pkg/kubedog/tracker.go b/pkg/kubedog/tracker.go index 666ced83..7d2b040f 100644 --- a/pkg/kubedog/tracker.go +++ b/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 diff --git a/pkg/kubedog/tracker_test.go b/pkg/kubedog/tracker_test.go index 6bc3eb2f..049adb08 100644 --- a/pkg/kubedog/tracker_test.go +++ b/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) + } +}