Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions test/extended/util/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1272,19 +1272,36 @@ func WaitForAccess(c kubernetes.Interface, allowed bool, review *kubeauthorizati
}

func GetClientConfig(kubeConfigFile string) (*rest.Config, error) {
kubeConfigBytes, err := ioutil.ReadFile(kubeConfigFile)
if err != nil {
return nil, err
}
kubeConfig, err := clientcmd.NewClientConfigFromBytes(kubeConfigBytes)
if err != nil {
return nil, err
}
clientConfig, err := kubeConfig.ClientConfig()
var clientConfig *rest.Config
var lastErr error
err := wait.PollImmediate(200*time.Millisecond, 10*time.Second, func() (bool, error) {
kubeConfigBytes, err := os.ReadFile(kubeConfigFile)
if err != nil {
lastErr = err
if os.IsNotExist(err) {
return false, nil
}
return false, err
}
if len(kubeConfigBytes) == 0 {
lastErr = fmt.Errorf("kubeconfig file %q is empty", kubeConfigFile)
return false, nil
}
kubeConfig, err := clientcmd.NewClientConfigFromBytes(kubeConfigBytes)
if err != nil {
lastErr = err
return false, nil
}
clientConfig, err = kubeConfig.ClientConfig()
if err != nil {
lastErr = err
return false, nil
}
return true, nil
})
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to read kubeconfig %q after retries: %w", kubeConfigFile, lastErr)
}

return clientConfig, nil
}

Expand Down
112 changes: 112 additions & 0 deletions test/extended/util/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package util

import (
"os"
"path/filepath"
"testing"
"time"
)

const validKubeconfig = `apiVersion: v1
kind: Config
clusters:
- cluster:
server: https://localhost:6443
name: test-cluster
contexts:
- context:
cluster: test-cluster
user: test-user
name: test-context
current-context: test-context
users:
- name: test-user
user:
token: test-token
`

func TestGetClientConfigRetryOnMissingFile(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "kubeconfig")

if err := os.WriteFile(path, []byte(validKubeconfig), 0644); err != nil {
t.Fatal(err)
}

go func() {
time.Sleep(200 * time.Millisecond)
os.Remove(path)
time.Sleep(800 * time.Millisecond)
os.WriteFile(path, []byte(validKubeconfig), 0644)
}()

time.Sleep(300 * time.Millisecond)

cfg, err := GetClientConfig(path)
if err != nil {
t.Fatalf("expected retry to succeed, got error: %v", err)
}
if cfg.Host != "https://localhost:6443" {
t.Fatalf("unexpected host: %s", cfg.Host)
}
}

func TestGetClientConfigRetryOnEmptyFile(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "kubeconfig")

if err := os.WriteFile(path, []byte{}, 0644); err != nil {
t.Fatal(err)
}

go func() {
time.Sleep(500 * time.Millisecond)
os.WriteFile(path, []byte(validKubeconfig), 0644)
}()

cfg, err := GetClientConfig(path)
if err != nil {
t.Fatalf("expected retry to succeed, got error: %v", err)
}
if cfg.Host != "https://localhost:6443" {
t.Fatalf("unexpected host: %s", cfg.Host)
}
}
Comment on lines +28 to +74
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Make the retry tests deterministic.

The fixed sleeps and fire-and-forget goroutines can let these tests pass without actually hitting the retry window, and they can also race t.TempDir() cleanup on slower CI workers. Please synchronize the file mutation and check the background write/remove errors.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/extended/util/client_test.go` around lines 28 - 74, The retry tests
TestGetClientConfigRetryOnMissingFile and TestGetClientConfigRetryOnEmptyFile
are flakey due to fixed sleeps and fire-and-forget goroutines; change them to
synchronize the background file mutations and surface any file I/O errors so the
test deterministically exercises GetClientConfig's retry window. Specifically,
replace the anonymous goroutines with a goroutine that reports errors over a
channel (or use a sync.WaitGroup) and signal when the mutation is done, wait for
that signal before finishing the test, and check/require the returned error
values from os.Remove/os.WriteFile in those goroutines; ensure the test waits
for the background worker to start the mutation timing (e.g., signal readiness)
so the GetClientConfig call actually encounters the missing/empty file and
triggers retries.


func TestGetClientConfigFailsOnPermanentMissing(t *testing.T) {
path := filepath.Join(t.TempDir(), "nonexistent", "kubeconfig")

start := time.Now()
_, err := GetClientConfig(path)
elapsed := time.Since(start)

if err == nil {
t.Fatal("expected error for permanently missing file")
}
if elapsed > 15*time.Second {
t.Fatalf("took too long: %v", elapsed)
}
}

func TestGetClientConfigSucceedsImmediately(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "kubeconfig")

if err := os.WriteFile(path, []byte(validKubeconfig), 0644); err != nil {
t.Fatal(err)
}

start := time.Now()
cfg, err := GetClientConfig(path)
elapsed := time.Since(start)

if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if cfg.Host != "https://localhost:6443" {
t.Fatalf("unexpected host: %s", cfg.Host)
}
if elapsed > 1*time.Second {
t.Fatalf("should have succeeded immediately, took %v", elapsed)
}
}