Skip to content

Commit 9f5d9a9

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#91315 from jherrera123/master
Fix runtime admission flaky test due to race condition
2 parents 1700acb + a5800ab commit 9f5d9a9

File tree

3 files changed

+181
-4
lines changed

3 files changed

+181
-4
lines changed

‎plugin/pkg/admission/runtimeclass/BUILD‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ go_library(
1414
"//staging/src/k8s.io/api/node/v1beta1:go_default_library",
1515
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
1616
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
17+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
1718
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
1819
"//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
1920
"//staging/src/k8s.io/client-go/informers:go_default_library",
21+
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
22+
"//staging/src/k8s.io/client-go/kubernetes/typed/node/v1beta1:go_default_library",
2023
"//staging/src/k8s.io/client-go/listers/node/v1beta1:go_default_library",
2124
"//staging/src/k8s.io/component-base/featuregate:go_default_library",
2225
],
@@ -28,13 +31,19 @@ go_test(
2831
embed = [":go_default_library"],
2932
deps = [
3033
"//pkg/apis/core:go_default_library",
34+
"//pkg/controller:go_default_library",
35+
"//pkg/features:go_default_library",
3136
"//staging/src/k8s.io/api/core/v1:go_default_library",
3237
"//staging/src/k8s.io/api/node/v1beta1:go_default_library",
3338
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
3439
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
3540
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
3641
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
3742
"//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
43+
"//staging/src/k8s.io/client-go/informers:go_default_library",
44+
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
45+
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
46+
"//staging/src/k8s.io/component-base/featuregate:go_default_library",
3847
"//vendor/github.com/stretchr/testify/assert:go_default_library",
3948
],
4049
)

‎plugin/pkg/admission/runtimeclass/admission.go‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,12 @@ import (
2929
v1beta1 "k8s.io/api/node/v1beta1"
3030
apiequality "k8s.io/apimachinery/pkg/api/equality"
3131
apierrors "k8s.io/apimachinery/pkg/api/errors"
32+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3233
"k8s.io/apiserver/pkg/admission"
3334
genericadmissioninitailizer "k8s.io/apiserver/pkg/admission/initializer"
3435
"k8s.io/client-go/informers"
36+
"k8s.io/client-go/kubernetes"
37+
nodev1beta1client "k8s.io/client-go/kubernetes/typed/node/v1beta1"
3538
nodev1beta1listers "k8s.io/client-go/listers/node/v1beta1"
3639
"k8s.io/component-base/featuregate"
3740
api "k8s.io/kubernetes/pkg/apis/core"
@@ -58,6 +61,7 @@ func Register(plugins *admission.Plugins) {
5861
type RuntimeClass struct {
5962
*admission.Handler
6063
runtimeClassLister nodev1beta1listers.RuntimeClassLister
64+
runtimeClassClient nodev1beta1client.RuntimeClassInterface
6165

6266
inspectedFeatures bool
6367
runtimeClassEnabled bool
@@ -68,6 +72,12 @@ var _ admission.MutationInterface = &RuntimeClass{}
6872
var _ admission.ValidationInterface = &RuntimeClass{}
6973

7074
var _ genericadmissioninitailizer.WantsExternalKubeInformerFactory = &RuntimeClass{}
75+
var _ genericadmissioninitailizer.WantsExternalKubeClientSet = &RuntimeClass{}
76+
77+
// SetExternalKubeClientSet sets the client for the plugin
78+
func (r *RuntimeClass) SetExternalKubeClientSet(client kubernetes.Interface) {
79+
r.runtimeClassClient = client.NodeV1beta1().RuntimeClasses()
80+
}
7181

7282
// InspectFeatureGates allows setting bools without taking a dep on a global variable
7383
func (r *RuntimeClass) InspectFeatureGates(featureGates featuregate.FeatureGate) {
@@ -97,6 +107,9 @@ func (r *RuntimeClass) ValidateInitialization() error {
97107
if r.runtimeClassLister == nil {
98108
return fmt.Errorf("missing RuntimeClass lister")
99109
}
110+
if r.runtimeClassClient == nil {
111+
return fmt.Errorf("missing RuntimeClass client")
112+
}
100113
return nil
101114
}
102115

@@ -111,7 +124,7 @@ func (r *RuntimeClass) Admit(ctx context.Context, attributes admission.Attribute
111124
return nil
112125
}
113126

114-
pod, runtimeClass, err := r.prepareObjects(attributes)
127+
pod, runtimeClass, err := r.prepareObjects(ctx, attributes)
115128
if err != nil {
116129
return err
117130
}
@@ -139,7 +152,7 @@ func (r *RuntimeClass) Validate(ctx context.Context, attributes admission.Attrib
139152
return nil
140153
}
141154

142-
pod, runtimeClass, err := r.prepareObjects(attributes)
155+
pod, runtimeClass, err := r.prepareObjects(ctx, attributes)
143156
if err != nil {
144157
return err
145158
}
@@ -160,7 +173,7 @@ func NewRuntimeClass() *RuntimeClass {
160173
}
161174

162175
// prepareObjects returns pod and runtimeClass types from the given admission attributes
163-
func (r *RuntimeClass) prepareObjects(attributes admission.Attributes) (pod *api.Pod, runtimeClass *v1beta1.RuntimeClass, err error) {
176+
func (r *RuntimeClass) prepareObjects(ctx context.Context, attributes admission.Attributes) (pod *api.Pod, runtimeClass *v1beta1.RuntimeClass, err error) {
164177
pod, ok := attributes.GetObject().(*api.Pod)
165178
if !ok {
166179
return nil, nil, apierrors.NewBadRequest("Resource was marked with kind Pod but was unable to be converted")
@@ -173,7 +186,11 @@ func (r *RuntimeClass) prepareObjects(attributes admission.Attributes) (pod *api
173186
// get RuntimeClass object
174187
runtimeClass, err = r.runtimeClassLister.Get(*pod.Spec.RuntimeClassName)
175188
if apierrors.IsNotFound(err) {
176-
return pod, nil, admission.NewForbidden(attributes, fmt.Errorf("pod rejected: RuntimeClass %q not found", *pod.Spec.RuntimeClassName))
189+
// if not found, our informer cache could be lagging, do a live lookup
190+
runtimeClass, err = r.runtimeClassClient.Get(ctx, *pod.Spec.RuntimeClassName, metav1.GetOptions{})
191+
if apierrors.IsNotFound(err) {
192+
return pod, nil, admission.NewForbidden(attributes, fmt.Errorf("pod rejected: RuntimeClass %q not found", *pod.Spec.RuntimeClassName))
193+
}
177194
}
178195

179196
// return the pod and runtimeClass.

‎plugin/pkg/admission/runtimeclass/admission_test.go‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,14 @@ import (
2929
"k8s.io/apimachinery/pkg/runtime"
3030
"k8s.io/apiserver/pkg/admission"
3131
"k8s.io/apiserver/pkg/authentication/user"
32+
"k8s.io/client-go/informers"
33+
"k8s.io/client-go/kubernetes"
34+
"k8s.io/client-go/kubernetes/fake"
35+
"k8s.io/component-base/featuregate"
3236
"k8s.io/kubernetes/pkg/apis/core"
37+
api "k8s.io/kubernetes/pkg/apis/core"
38+
"k8s.io/kubernetes/pkg/controller"
39+
"k8s.io/kubernetes/pkg/features"
3340

3441
"github.com/stretchr/testify/assert"
3542
)
@@ -315,6 +322,150 @@ func NewObjectInterfacesForTest() admission.ObjectInterfaces {
315322
return admission.NewObjectInterfacesFromScheme(scheme)
316323
}
317324

325+
func newRuntimeClassForTest(runtimeClassEnabled bool,
326+
featureInspection bool,
327+
addLister bool,
328+
listerObject *v1beta1.RuntimeClass,
329+
addClient bool,
330+
clientObject *v1beta1.RuntimeClass) *RuntimeClass {
331+
runtimeClass := NewRuntimeClass()
332+
333+
if featureInspection {
334+
relevantFeatures := map[featuregate.Feature]featuregate.FeatureSpec{
335+
features.RuntimeClass: {Default: runtimeClassEnabled},
336+
features.PodOverhead: {Default: false},
337+
}
338+
fg := featuregate.NewFeatureGate()
339+
fg.Add(relevantFeatures)
340+
runtimeClass.InspectFeatureGates(fg)
341+
}
342+
343+
if addLister {
344+
informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc())
345+
runtimeClass.SetExternalKubeInformerFactory(informerFactory)
346+
if listerObject != nil {
347+
informerFactory.Node().V1beta1().RuntimeClasses().Informer().GetStore().Add(listerObject)
348+
}
349+
}
350+
351+
if addClient {
352+
var client kubernetes.Interface
353+
if clientObject != nil {
354+
client = fake.NewSimpleClientset(clientObject)
355+
} else {
356+
client = fake.NewSimpleClientset()
357+
}
358+
runtimeClass.SetExternalKubeClientSet(client)
359+
}
360+
361+
return runtimeClass
362+
}
363+
364+
func TestValidateInitialization(t *testing.T) {
365+
tests := []struct {
366+
name string
367+
expectError bool
368+
runtimeClass *RuntimeClass
369+
}{
370+
{
371+
name: "runtimeClass disabled, success",
372+
expectError: false,
373+
runtimeClass: newRuntimeClassForTest(false, true, true, nil, true, nil),
374+
},
375+
{
376+
name: "runtimeClass enabled, success",
377+
expectError: false,
378+
runtimeClass: newRuntimeClassForTest(true, true, true, nil, true, nil),
379+
},
380+
{
381+
name: "runtimeClass enabled, no feature inspection",
382+
expectError: true,
383+
runtimeClass: newRuntimeClassForTest(true, false, true, nil, true, nil),
384+
},
385+
{
386+
name: "runtimeClass enabled, no lister",
387+
expectError: true,
388+
runtimeClass: newRuntimeClassForTest(true, true, false, nil, true, nil),
389+
},
390+
{
391+
name: "runtimeClass enabled, no client",
392+
expectError: true,
393+
runtimeClass: newRuntimeClassForTest(true, true, true, nil, false, nil),
394+
},
395+
}
396+
397+
for _, tc := range tests {
398+
t.Run(tc.name, func(t *testing.T) {
399+
err := tc.runtimeClass.ValidateInitialization()
400+
if tc.expectError {
401+
assert.NotEmpty(t, err)
402+
} else {
403+
assert.Empty(t, err)
404+
}
405+
})
406+
}
407+
}
408+
409+
func TestAdmit(t *testing.T) {
410+
runtimeClassName := "runtimeClassName"
411+
412+
rc := &v1beta1.RuntimeClass{
413+
ObjectMeta: metav1.ObjectMeta{Name: runtimeClassName},
414+
}
415+
416+
pod := api.Pod{
417+
ObjectMeta: metav1.ObjectMeta{Name: "podname"},
418+
Spec: api.PodSpec{
419+
RuntimeClassName: &runtimeClassName,
420+
},
421+
}
422+
423+
attributes := admission.NewAttributesRecord(&pod,
424+
nil,
425+
api.Kind("kind").WithVersion("version"),
426+
"",
427+
"",
428+
api.Resource("pods").WithVersion("version"),
429+
"",
430+
admission.Create,
431+
nil,
432+
false,
433+
nil)
434+
435+
tests := []struct {
436+
name string
437+
expectError bool
438+
runtimeClass *RuntimeClass
439+
}{
440+
{
441+
name: "runtimeClass found by lister",
442+
expectError: false,
443+
runtimeClass: newRuntimeClassForTest(true, true, true, rc, true, nil),
444+
},
445+
{
446+
name: "runtimeClass found by client",
447+
expectError: false,
448+
runtimeClass: newRuntimeClassForTest(true, true, true, nil, true, rc),
449+
},
450+
{
451+
name: "runtimeClass not found by lister nor client",
452+
expectError: true,
453+
runtimeClass: newRuntimeClassForTest(true, true, true, nil, true, nil),
454+
},
455+
}
456+
457+
for _, tc := range tests {
458+
t.Run(tc.name, func(t *testing.T) {
459+
err := tc.runtimeClass.Admit(context.TODO(), attributes, nil)
460+
if tc.expectError {
461+
assert.NotEmpty(t, err)
462+
} else {
463+
assert.Empty(t, err)
464+
}
465+
})
466+
}
467+
}
468+
318469
func TestValidate(t *testing.T) {
319470
tests := []struct {
320471
name string

0 commit comments

Comments
 (0)