From 5732c6370ac3f449c2b3416b30f31e67e4874de4 Mon Sep 17 00:00:00 2001 From: draveness Date: Tue, 6 Aug 2019 20:17:28 +0800 Subject: [PATCH] feat: update runtime class admission plugin --- plugin/pkg/admission/runtimeclass/BUILD | 1 + .../pkg/admission/runtimeclass/admission.go | 73 +++++-- .../admission/runtimeclass/admission_test.go | 190 ++++++++++++++++-- 3 files changed, 226 insertions(+), 38 deletions(-) diff --git a/plugin/pkg/admission/runtimeclass/BUILD b/plugin/pkg/admission/runtimeclass/BUILD index 8b55378a447..49fc4539670 100644 --- a/plugin/pkg/admission/runtimeclass/BUILD +++ b/plugin/pkg/admission/runtimeclass/BUILD @@ -10,6 +10,7 @@ go_library( "//pkg/apis/node:go_default_library", "//pkg/apis/node/v1beta1:go_default_library", "//pkg/features:go_default_library", + "//pkg/util/tolerations:go_default_library", "//staging/src/k8s.io/api/node/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/plugin/pkg/admission/runtimeclass/admission.go b/plugin/pkg/admission/runtimeclass/admission.go index ac68de7f829..b8c9831546a 100644 --- a/plugin/pkg/admission/runtimeclass/admission.go +++ b/plugin/pkg/admission/runtimeclass/admission.go @@ -38,6 +38,7 @@ import ( node "k8s.io/kubernetes/pkg/apis/node" nodev1beta1 "k8s.io/kubernetes/pkg/apis/node/v1beta1" "k8s.io/kubernetes/pkg/features" + "k8s.io/kubernetes/pkg/util/tolerations" ) // PluginName indicates name of admission plugin. @@ -81,6 +82,9 @@ func (r *RuntimeClass) ValidateInitialization() error { // Admit makes an admission decision based on the request attributes func (r *RuntimeClass) Admit(ctx context.Context, attributes admission.Attributes, o admission.ObjectInterfaces) error { + if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) { + return nil + } // Ignore all calls to subresources or resources other than pods. if shouldIgnore(attributes) { @@ -92,18 +96,20 @@ func (r *RuntimeClass) Admit(ctx context.Context, attributes admission.Attribute return err } if utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) { - err = setOverhead(attributes, pod, runtimeClass) - if err != nil { + if err := setOverhead(attributes, pod, runtimeClass); err != nil { return err } } + if err := setScheduling(attributes, pod, runtimeClass); err != nil { + return err + } + return nil } // Validate makes sure that pod adhere's to RuntimeClass's definition func (r *RuntimeClass) Validate(ctx context.Context, attributes admission.Attributes, o admission.ObjectInterfaces) error { - // Ignore all calls to subresources or resources other than pods. if shouldIgnore(attributes) { return nil @@ -114,7 +120,7 @@ func (r *RuntimeClass) Validate(ctx context.Context, attributes admission.Attrib return err } if utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) { - if err = validateOverhead(attributes, pod, runtimeClass); err != nil { + if err := validateOverhead(attributes, pod, runtimeClass); err != nil { return err } } @@ -131,34 +137,33 @@ func NewRuntimeClass() *RuntimeClass { // prepareObjects returns pod and runtimeClass types from the given admission attributes func (r *RuntimeClass) prepareObjects(attributes admission.Attributes) (pod *api.Pod, runtimeClass *v1beta1.RuntimeClass, err error) { - pod, ok := attributes.GetObject().(*api.Pod) if !ok { return nil, nil, apierrors.NewBadRequest("Resource was marked with kind Pod but was unable to be converted") } - // get RuntimeClass object - if pod.Spec.RuntimeClassName != nil { - runtimeClass, err = r.runtimeClassLister.Get(*pod.Spec.RuntimeClassName) - if err != nil { - return pod, nil, err - } + if pod.Spec.RuntimeClassName == nil { + return pod, nil, nil } - // return the pod and runtimeClass. If no RuntimeClass is specified in PodSpec, runtimeClass will be nil - return pod, runtimeClass, nil + // get RuntimeClass object + runtimeClass, err = r.runtimeClassLister.Get(*pod.Spec.RuntimeClassName) + if apierrors.IsNotFound(err) { + return pod, nil, admission.NewForbidden(attributes, fmt.Errorf("pod rejected: RuntimeClass %q not found", *pod.Spec.RuntimeClassName)) + } + + // return the pod and runtimeClass. + return pod, runtimeClass, err } func setOverhead(a admission.Attributes, pod *api.Pod, runtimeClass *v1beta1.RuntimeClass) (err error) { - if runtimeClass == nil || runtimeClass.Overhead == nil { return nil } // convert to internal type and assign to pod's Overhead nodeOverhead := &node.Overhead{} - err = nodev1beta1.Convert_v1beta1_Overhead_To_node_Overhead(runtimeClass.Overhead, nodeOverhead, nil) - if err != nil { + if err = nodev1beta1.Convert_v1beta1_Overhead_To_node_Overhead(runtimeClass.Overhead, nodeOverhead, nil); err != nil { return err } @@ -172,13 +177,43 @@ func setOverhead(a admission.Attributes, pod *api.Pod, runtimeClass *v1beta1.Run return nil } -func validateOverhead(a admission.Attributes, pod *api.Pod, runtimeClass *v1beta1.RuntimeClass) (err error) { +func setScheduling(a admission.Attributes, pod *api.Pod, runtimeClass *v1beta1.RuntimeClass) (err error) { + if runtimeClass == nil || runtimeClass.Scheduling == nil { + return nil + } + // convert to internal type and assign to pod's Scheduling + nodeScheduling := &node.Scheduling{} + if err = nodev1beta1.Convert_v1beta1_Scheduling_To_node_Scheduling(runtimeClass.Scheduling, nodeScheduling, nil); err != nil { + return err + } + + runtimeNodeSelector := nodeScheduling.NodeSelector + newNodeSelector := pod.Spec.NodeSelector + if newNodeSelector == nil { + newNodeSelector = runtimeNodeSelector + } else { + for key, runtimeClassValue := range runtimeNodeSelector { + if podValue, ok := newNodeSelector[key]; ok && podValue != runtimeClassValue { + return admission.NewForbidden(a, fmt.Errorf("conflict: runtimeClass.scheduling.nodeSelector[%s] = %s; pod.spec.nodeSelector[%s] = %s", key, runtimeClassValue, key, podValue)) + } + newNodeSelector[key] = runtimeClassValue + } + } + + newTolerations := tolerations.MergeTolerations(pod.Spec.Tolerations, nodeScheduling.Tolerations) + + pod.Spec.NodeSelector = newNodeSelector + pod.Spec.Tolerations = newTolerations + + return nil +} + +func validateOverhead(a admission.Attributes, pod *api.Pod, runtimeClass *v1beta1.RuntimeClass) (err error) { if runtimeClass != nil && runtimeClass.Overhead != nil { // If the Overhead set doesn't match what is provided in the RuntimeClass definition, reject the pod nodeOverhead := &node.Overhead{} - err := nodev1beta1.Convert_v1beta1_Overhead_To_node_Overhead(runtimeClass.Overhead, nodeOverhead, nil) - if err != nil { + if err := nodev1beta1.Convert_v1beta1_Overhead_To_node_Overhead(runtimeClass.Overhead, nodeOverhead, nil); err != nil { return err } if !apiequality.Semantic.DeepEqual(nodeOverhead.PodFixed, pod.Spec.Overhead) { diff --git a/plugin/pkg/admission/runtimeclass/admission_test.go b/plugin/pkg/admission/runtimeclass/admission_test.go index d3db68e7091..5e2d74d7827 100644 --- a/plugin/pkg/admission/runtimeclass/admission_test.go +++ b/plugin/pkg/admission/runtimeclass/admission_test.go @@ -22,6 +22,7 @@ import ( "testing" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/api/node/v1beta1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,7 +37,7 @@ import ( "github.com/stretchr/testify/assert" ) -func validPod(name string, numContainers int, resources core.ResourceRequirements, setOverhead bool) *core.Pod { +func newOverheadValidPod(name string, numContainers int, resources core.ResourceRequirements, setOverhead bool) *core.Pod { pod := &core.Pod{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"}, Spec: core.PodSpec{}, @@ -59,6 +60,16 @@ func validPod(name string, numContainers int, resources core.ResourceRequirement return pod } +func newSchedulingValidPod(name string, nodeSelector map[string]string, tolerations []core.Toleration) *core.Pod { + return &core.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"}, + Spec: core.PodSpec{ + NodeSelector: nodeSelector, + Tolerations: tolerations, + }, + } +} + func getGuaranteedRequirements() core.ResourceRequirements { resources := core.ResourceList{ core.ResourceName(core.ResourceCPU): resource.MustParse("1"), @@ -69,7 +80,6 @@ func getGuaranteedRequirements() core.ResourceRequirements { } func TestSetOverhead(t *testing.T) { - tests := []struct { name string runtimeClass *v1beta1.RuntimeClass @@ -89,9 +99,9 @@ func TestSetOverhead(t *testing.T) { }, }, }, - pod: validPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, false), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, false), expectError: false, - expectedPod: validPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), + expectedPod: newOverheadValidPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), }, { name: "overhead, guaranteed pod", @@ -105,9 +115,9 @@ func TestSetOverhead(t *testing.T) { }, }, }, - pod: validPod("guaranteed", 1, getGuaranteedRequirements(), false), + pod: newOverheadValidPod("guaranteed", 1, getGuaranteedRequirements(), false), expectError: false, - expectedPod: validPod("guaranteed", 1, core.ResourceRequirements{}, true), + expectedPod: newOverheadValidPod("guaranteed", 1, core.ResourceRequirements{}, true), }, { name: "overhead, pod with differing overhead already set", @@ -121,7 +131,7 @@ func TestSetOverhead(t *testing.T) { }, }, }, - pod: validPod("empty-requiremennts-overhead", 1, core.ResourceRequirements{}, true), + pod: newOverheadValidPod("empty-requiremennts-overhead", 1, core.ResourceRequirements{}, true), expectError: true, expectedPod: nil, }, @@ -137,7 +147,7 @@ func TestSetOverhead(t *testing.T) { }, }, }, - pod: validPod("empty-requiremennts-overhead", 1, core.ResourceRequirements{}, true), + pod: newOverheadValidPod("empty-requiremennts-overhead", 1, core.ResourceRequirements{}, true), expectError: false, expectedPod: nil, }, @@ -157,6 +167,151 @@ func TestSetOverhead(t *testing.T) { }) } } + +func TestSetScheduling(t *testing.T) { + tests := []struct { + name string + runtimeClass *v1beta1.RuntimeClass + pod *core.Pod + expectError bool + expectedPod *core.Pod + }{ + { + name: "scheduling, nil scheduling", + runtimeClass: &v1beta1.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Handler: "bar", + Scheduling: nil, + }, + pod: newSchedulingValidPod("pod-with-conflict-node-selector", map[string]string{"foo": "bar"}, []core.Toleration{}), + expectError: false, + expectedPod: newSchedulingValidPod("pod-with-conflict-node-selector", map[string]string{"foo": "bar"}, []core.Toleration{}), + }, + { + name: "scheduling, conflict node selector", + runtimeClass: &v1beta1.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Handler: "bar", + Scheduling: &v1beta1.Scheduling{ + NodeSelector: map[string]string{ + "foo": "conflict", + }, + }, + }, + pod: newSchedulingValidPod("pod-with-conflict-node-selector", map[string]string{"foo": "bar"}, []core.Toleration{}), + expectError: true, + }, + { + name: "scheduling, nil node selector", + runtimeClass: &v1beta1.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Handler: "bar", + Scheduling: &v1beta1.Scheduling{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + }, + }, + pod: newSchedulingValidPod("pod-with-conflict-node-selector", nil, nil), + expectError: false, + expectedPod: newSchedulingValidPod("pod-with-conflict-node-selector", map[string]string{"foo": "bar"}, nil), + }, + { + name: "scheduling, node selector with the same key value", + runtimeClass: &v1beta1.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Handler: "bar", + Scheduling: &v1beta1.Scheduling{ + NodeSelector: map[string]string{ + "foo": "bar", + }, + }, + }, + pod: newSchedulingValidPod("pod-with-same-key-value-node-selector", map[string]string{"foo": "bar"}, nil), + expectError: false, + expectedPod: newSchedulingValidPod("pod-with-same-key-value-node-selector", map[string]string{"foo": "bar"}, nil), + }, + { + name: "scheduling, node selector with different key value", + runtimeClass: &v1beta1.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Handler: "bar", + Scheduling: &v1beta1.Scheduling{ + NodeSelector: map[string]string{ + "foo": "bar", + "fizz": "buzz", + }, + }, + }, + pod: newSchedulingValidPod("pod-with-different-key-value-node-selector", map[string]string{"foo": "bar"}, nil), + expectError: false, + expectedPod: newSchedulingValidPod("pod-with-different-key-value-node-selector", map[string]string{"foo": "bar", "fizz": "buzz"}, nil), + }, + { + name: "scheduling, multiple tolerations", + runtimeClass: &v1beta1.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Handler: "bar", + Scheduling: &v1beta1.Scheduling{ + Tolerations: []v1.Toleration{ + { + Key: "foo", + Operator: v1.TolerationOpEqual, + Value: "bar", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "fizz", + Operator: v1.TolerationOpEqual, + Value: "buzz", + Effect: v1.TaintEffectNoSchedule, + }, + }, + }, + }, + pod: newSchedulingValidPod("pod-with-tolerations", map[string]string{"foo": "bar"}, + []core.Toleration{ + { + Key: "foo", + Operator: core.TolerationOpEqual, + Value: "bar", + Effect: core.TaintEffectNoSchedule, + }, + }), + expectError: false, + expectedPod: newSchedulingValidPod("pod-with-tolerations", map[string]string{"foo": "bar"}, + []core.Toleration{ + { + Key: "foo", + Operator: core.TolerationOpEqual, + Value: "bar", + Effect: core.TaintEffectNoSchedule, + }, + { + Key: "fizz", + Operator: core.TolerationOpEqual, + Value: "buzz", + Effect: core.TaintEffectNoSchedule, + }, + }), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + attrs := admission.NewAttributesRecord(tc.pod, nil, core.Kind("Pod").WithVersion("version"), tc.pod.Namespace, tc.pod.Name, core.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, &user.DefaultInfo{}) + + errs := setScheduling(attrs, tc.pod, tc.runtimeClass) + if tc.expectError { + assert.NotEmpty(t, errs) + } else { + assert.Equal(t, tc.expectedPod, tc.pod) + assert.Empty(t, errs) + } + }) + } +} + func NewObjectInterfacesForTest() admission.ObjectInterfaces { scheme := runtime.NewScheme() corev1.AddToScheme(scheme) @@ -178,7 +333,7 @@ func TestValidate(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Handler: "bar", }, - pod: validPod("no-resource-req-no-overhead", 1, getGuaranteedRequirements(), true), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, getGuaranteedRequirements(), true), expectError: true, }, { @@ -193,7 +348,7 @@ func TestValidate(t *testing.T) { }, }, }, - pod: validPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), expectError: true, }, { @@ -208,7 +363,7 @@ func TestValidate(t *testing.T) { }, }, }, - pod: validPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, false), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, false), expectError: false, }, } @@ -230,8 +385,6 @@ func TestValidate(t *testing.T) { } func TestValidateOverhead(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodOverhead, true)() - tests := []struct { name string runtimeClass *v1beta1.RuntimeClass @@ -250,7 +403,7 @@ func TestValidateOverhead(t *testing.T) { }, }, }, - pod: validPod("no-requirements", 1, core.ResourceRequirements{}, false), + pod: newOverheadValidPod("no-requirements", 1, core.ResourceRequirements{}, false), expectError: true, }, { @@ -259,13 +412,13 @@ func TestValidateOverhead(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Handler: "bar", }, - pod: validPod("no-resource-req-no-overhead", 1, getGuaranteedRequirements(), true), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, getGuaranteedRequirements(), true), expectError: true, }, { name: "No RunntimeClass, Overhead set in pod", runtimeClass: nil, - pod: validPod("no-resource-req-no-overhead", 1, getGuaranteedRequirements(), true), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, getGuaranteedRequirements(), true), expectError: true, }, { @@ -280,7 +433,7 @@ func TestValidateOverhead(t *testing.T) { }, }, }, - pod: validPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), expectError: true, }, { @@ -295,14 +448,13 @@ func TestValidateOverhead(t *testing.T) { }, }, }, - pod: validPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), + pod: newOverheadValidPod("no-resource-req-no-overhead", 1, core.ResourceRequirements{}, true), expectError: false, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - attrs := admission.NewAttributesRecord(tc.pod, nil, core.Kind("Pod").WithVersion("version"), tc.pod.Namespace, tc.pod.Name, core.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, &user.DefaultInfo{}) errs := validateOverhead(attrs, tc.pod, tc.runtimeClass)