github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2026-07-18 04:54:54 +00:00
Code Issues Projects Releases Wiki Activity

webhook admission: avoid risk of flakes

Browse Source 
The unit test flaked at least once so far in the CI. Cannot be reproduced
locally, though.

    === RUN   TestGetMutatingWebhookConfigSmartReload/create_configuration_and_update_moar_of_them
        mutating_webhook_manager_test.go:238: Expected number of webhooks 4 received 0
        mutating_webhook_manager_test.go:245: Expected number of creations 4 received 0
        mutating_webhook_manager_test.go:266: Expected final number of webhooks 5 received 3
    --- FAIL: TestGetMutatingWebhookConfigSmartReload/create_configuration_and_update_moar_of_them (2.00s)

There were two potential root causes:
- The watch must be set up completely before creating objects (a fake
  client-go limitation, fix separately).
- The delay might be too small on a loaded system.

By running the tests in a synctest bubble with synctest.Wait calls at
the right places both problems are avoided. As a welcome side effect
the test also completes faster.

Before:

    ok  	k8s.io/apiserver/pkg/admission/configuration	21.036s

After:

    ok  	k8s.io/apiserver/pkg/admission/configuration	4.029s

    2m0s: 13801 runs so far, 0 failures
This commit is contained in:
Patrick Ohly
2025-12-25 19:44:24 +01:00
parent fd41228d1a
commit 33301aec54
2 changed files with 139 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files

147
staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go
View File

@@ -20,16 +20,17 @@ import (
"context"
"reflect"
"testing"
"time"
"testing/synctest"
"k8s.io/api/admissionregistration/v1"
v1 "k8s.io/api/admissionregistration/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission/plugin/webhook"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
)
func TestGetMutatingWebhookConfig(t *testing.T) {
func TestGetMutatingWebhookConfig(t *testing.T) { synctest.Test(t, testGetMutatingWebhookConfig) }
func testGetMutatingWebhookConfig(t *testing.T) {
// Build a test client that the admission plugin can use to look up the MutatingWebhookConfiguration
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
@@ -40,6 +41,9 @@ func TestGetMutatingWebhookConfig(t *testing.T) {
informerFactory.Start(stop)
informerFactory.WaitForCacheSync(stop)
// Wait for background activity to settle.
synctest.Wait()
// no configurations
if configurations := manager.Webhooks(); len(configurations) != 0 {
t.Errorf("expected empty webhooks, but got %v", configurations)
@@ -55,19 +59,10 @@ func TestGetMutatingWebhookConfig(t *testing.T) {
MutatingWebhookConfigurations().
Create(context.TODO(), webhookConfiguration, metav1.CreateOptions{})
// Wait up to 10s for the notification to be delivered.
// (on my system this takes < 2ms)
startTime := time.Now()
// Wait for the notification to be delivered.
synctest.Wait()
configurations := manager.Webhooks()
for len(configurations) == 0 {
if time.Since(startTime) > 10*time.Second {
break
}
time.Sleep(time.Millisecond)
configurations = manager.Webhooks()
}
// verify presence
if len(configurations) == 0 {
t.Errorf("expected non empty webhooks")
}
@@ -212,72 +207,72 @@ func TestGetMutatingWebhookConfigSmartReload(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
stop := make(chan struct{})
defer close(stop)
manager := NewMutatingWebhookConfigurationManager(informerFactory)
managerStructPtr := manager.(*mutatingWebhookConfigurationManager)
fakeWebhookAccessorCreator := &mockCreateMutatingWebhookAccessor{}
managerStructPtr.createMutatingWebhookAccessor = fakeWebhookAccessorCreator.fn
informerFactory.Start(stop)
informerFactory.WaitForCacheSync(stop)
synctest.Test(t, func(t *testing.T) {
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
stop := make(chan struct{})
defer close(stop)
manager := NewMutatingWebhookConfigurationManager(informerFactory)
managerStructPtr := manager.(*mutatingWebhookConfigurationManager)
fakeWebhookAccessorCreator := &mockCreateMutatingWebhookAccessor{}
managerStructPtr.createMutatingWebhookAccessor = fakeWebhookAccessorCreator.fn
informerFactory.Start(stop)
informerFactory.WaitForCacheSync(stop)
// Create webhooks
for _, configurations := range tt.args.createWebhookConfigurations {
client.
AdmissionregistrationV1().
MutatingWebhookConfigurations().
Create(context.TODO(), configurations, metav1.CreateOptions{})
}
// TODO use channels to wait for manager.createMutatingWebhookAccessor
// to be called instead of using time.Sleep
time.Sleep(1 * time.Second)
webhooks := manager.Webhooks()
if mutatingConfigurationTotalWebhooks(tt.args.createWebhookConfigurations) != len(webhooks) {
t.Errorf("Expected number of webhooks %d received %d",
mutatingConfigurationTotalWebhooks(tt.args.createWebhookConfigurations),
len(webhooks),
)
}
// assert creations
if tt.numberOfCreations != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of creations %d received %d",
tt.numberOfCreations, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// Create webhooks
for _, configurations := range tt.args.createWebhookConfigurations {
client.
AdmissionregistrationV1().
MutatingWebhookConfigurations().
Create(context.TODO(), configurations, metav1.CreateOptions{})
}
// reset mock counter
fakeWebhookAccessorCreator.resetCounter()
synctest.Wait()
webhooks := manager.Webhooks()
if mutatingConfigurationTotalWebhooks(tt.args.createWebhookConfigurations) != len(webhooks) {
t.Errorf("Expected number of webhooks %d received %d",
mutatingConfigurationTotalWebhooks(tt.args.createWebhookConfigurations),
len(webhooks),
)
}
// assert creations
if tt.numberOfCreations != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of creations %d received %d",
tt.numberOfCreations, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// Update webhooks
for _, configurations := range tt.args.updateWebhookConfigurations {
client.
AdmissionregistrationV1().
MutatingWebhookConfigurations().
Update(context.TODO(), configurations, metav1.UpdateOptions{})
}
// TODO use channels to wait for manager.createMutatingWebhookAccessor
// to be called instead of using time.Sleep
time.Sleep(1 * time.Second)
webhooks = manager.Webhooks()
if tt.finalNumberOfWebhookAccessors != len(webhooks) {
t.Errorf("Expected final number of webhooks %d received %d",
tt.finalNumberOfWebhookAccessors,
len(webhooks),
)
}
// reset mock counter
fakeWebhookAccessorCreator.resetCounter()
// assert updates
if tt.numberOfRefreshes != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of refreshes %d received %d",
tt.numberOfRefreshes, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// reset mock counter for the next test cases
fakeWebhookAccessorCreator.resetCounter()
// Update webhooks
for _, configurations := range tt.args.updateWebhookConfigurations {
client.
AdmissionregistrationV1().
MutatingWebhookConfigurations().
Update(context.TODO(), configurations, metav1.UpdateOptions{})
}
synctest.Wait()
webhooks = manager.Webhooks()
if tt.finalNumberOfWebhookAccessors != len(webhooks) {
t.Errorf("Expected final number of webhooks %d received %d",
tt.finalNumberOfWebhookAccessors,
len(webhooks),
)
}
// assert updates
if tt.numberOfRefreshes != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of refreshes %d received %d",
tt.numberOfRefreshes, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// reset mock counter for the next test cases
fakeWebhookAccessorCreator.resetCounter()
})
})
}
}

143
staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go
View File

@@ -20,16 +20,17 @@ import (
"context"
"reflect"
"testing"
"time"
"testing/synctest"
"k8s.io/api/admissionregistration/v1"
v1 "k8s.io/api/admissionregistration/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/admission/plugin/webhook"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
)
func TestGetValidatingWebhookConfig(t *testing.T) {
func TestGetValidatingWebhookConfig(t *testing.T) { synctest.Test(t, testGetValidatingWebhookConfig) }
func testGetValidatingWebhookConfig(t *testing.T) {
// Build a test client that the admission plugin can use to look up the ValidatingWebhookConfiguration
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
@@ -41,6 +42,7 @@ func TestGetValidatingWebhookConfig(t *testing.T) {
informerFactory.WaitForCacheSync(stop)
// no configurations
synctest.Wait()
if configurations := manager.Webhooks(); len(configurations) != 0 {
t.Errorf("expected empty webhooks, but got %v", configurations)
}
@@ -55,17 +57,9 @@ func TestGetValidatingWebhookConfig(t *testing.T) {
ValidatingWebhookConfigurations().
Create(context.TODO(), webhookConfiguration, metav1.CreateOptions{})
// Wait up to 10s for the notification to be delivered.
// (on my system this takes < 2ms)
startTime := time.Now()
// Wait for the notification to be delivered.
synctest.Wait()
configurations := manager.Webhooks()
for len(configurations) == 0 {
if time.Since(startTime) > 10*time.Second {
break
}
time.Sleep(time.Millisecond)
configurations = manager.Webhooks()
}
// verify presence
if len(configurations) == 0 {
@@ -212,72 +206,71 @@ func TestGetValidatingWebhookConfigSmartReload(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
stop := make(chan struct{})
defer close(stop)
manager := NewValidatingWebhookConfigurationManager(informerFactory)
managerStructPtr := manager.(*validatingWebhookConfigurationManager)
fakeWebhookAccessorCreator := &mockCreateValidatingWebhookAccessor{}
managerStructPtr.createValidatingWebhookAccessor = fakeWebhookAccessorCreator.fn
informerFactory.Start(stop)
informerFactory.WaitForCacheSync(stop)
synctest.Test(t, func(t *testing.T) {
client := fake.NewSimpleClientset()
informerFactory := informers.NewSharedInformerFactory(client, 0)
stop := make(chan struct{})
defer close(stop)
manager := NewValidatingWebhookConfigurationManager(informerFactory)
managerStructPtr := manager.(*validatingWebhookConfigurationManager)
fakeWebhookAccessorCreator := &mockCreateValidatingWebhookAccessor{}
managerStructPtr.createValidatingWebhookAccessor = fakeWebhookAccessorCreator.fn
informerFactory.Start(stop)
informerFactory.WaitForCacheSync(stop)
// Create webhooks
for _, configurations := range tt.args.createWebhookConfigurations {
client.
AdmissionregistrationV1().
ValidatingWebhookConfigurations().
Create(context.TODO(), configurations, metav1.CreateOptions{})
}
// TODO use channels to wait for manager.createValidatingWebhookAccessor
// to be called instead of using time.Sleep
time.Sleep(1 * time.Second)
webhooks := manager.Webhooks()
if configurationTotalWebhooks(tt.args.createWebhookConfigurations) != len(webhooks) {
t.Errorf("Expected number of webhooks %d received %d",
configurationTotalWebhooks(tt.args.createWebhookConfigurations),
len(webhooks),
)
}
// assert creations
if tt.numberOfCreations != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of creations %d received %d",
tt.numberOfCreations, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// Create webhooks
for _, configurations := range tt.args.createWebhookConfigurations {
client.
AdmissionregistrationV1().
ValidatingWebhookConfigurations().
Create(context.TODO(), configurations, metav1.CreateOptions{})
}
// reset mock counter
fakeWebhookAccessorCreator.resetCounter()
synctest.Wait()
webhooks := manager.Webhooks()
if configurationTotalWebhooks(tt.args.createWebhookConfigurations) != len(webhooks) {
t.Errorf("Expected number of webhooks %d received %d",
configurationTotalWebhooks(tt.args.createWebhookConfigurations),
len(webhooks),
)
}
// assert creations
if tt.numberOfCreations != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of creations %d received %d",
tt.numberOfCreations, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// Update webhooks
for _, configurations := range tt.args.updateWebhookConfigurations {
client.
AdmissionregistrationV1().
ValidatingWebhookConfigurations().
Update(context.TODO(), configurations, metav1.UpdateOptions{})
}
// TODO use channels to wait for manager.createValidatingWebhookAccessor
// to be called instead of using time.Sleep
time.Sleep(1 * time.Second)
webhooks = manager.Webhooks()
if tt.finalNumberOfWebhookAccessors != len(webhooks) {
t.Errorf("Expected final number of webhooks %d received %d",
tt.finalNumberOfWebhookAccessors,
len(webhooks),
)
}
// reset mock counter
fakeWebhookAccessorCreator.resetCounter()
// assert updates
if tt.numberOfRefreshes != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of refreshes %d received %d",
tt.numberOfRefreshes, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// reset mock counter for the next test cases
fakeWebhookAccessorCreator.resetCounter()
// Update webhooks
for _, configurations := range tt.args.updateWebhookConfigurations {
client.
AdmissionregistrationV1().
ValidatingWebhookConfigurations().
Update(context.TODO(), configurations, metav1.UpdateOptions{})
}
synctest.Wait()
webhooks = manager.Webhooks()
if tt.finalNumberOfWebhookAccessors != len(webhooks) {
t.Errorf("Expected final number of webhooks %d received %d",
tt.finalNumberOfWebhookAccessors,
len(webhooks),
)
}
// assert updates
if tt.numberOfRefreshes != fakeWebhookAccessorCreator.calledNTimes() {
t.Errorf(
"Expected number of refreshes %d received %d",
tt.numberOfRefreshes, fakeWebhookAccessorCreator.calledNTimes(),
)
}
// reset mock counter for the next test cases
fakeWebhookAccessorCreator.resetCounter()
})
})
}
}