diff --git a/pkg/kubeapiserver/options/BUILD b/pkg/kubeapiserver/options/BUILD index 33226d5760b..1d9d1f28525 100644 --- a/pkg/kubeapiserver/options/BUILD +++ b/pkg/kubeapiserver/options/BUILD @@ -23,6 +23,7 @@ go_library( "//pkg/api/legacyscheme:go_default_library", "//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/cloudprovider/providers:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubeapiserver/authenticator:go_default_library", "//pkg/kubeapiserver/authorizer:go_default_library", "//pkg/kubeapiserver/authorizer/modes:go_default_library", @@ -64,6 +65,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating:go_default_library", "//staging/src/k8s.io/apiserver/pkg/server:go_default_library", "//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go index a25643a09b7..abcb5b4673a 100644 --- a/pkg/kubeapiserver/options/plugins.go +++ b/pkg/kubeapiserver/options/plugins.go @@ -57,6 +57,8 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" mutatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/mutating" validatingwebhook "k8s.io/apiserver/pkg/admission/plugin/webhook/validating" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/features" ) // AllOrderedPlugins is the list of all the plugins in order. @@ -139,5 +141,9 @@ func DefaultOffAdmissionPlugins() sets.String { resourcequota.PluginName, //ResourceQuota ) + if utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) { + defaultOnPlugins.Insert(podpriority.PluginName) //PodPriority + } + return sets.NewString(AllOrderedPlugins...).Difference(defaultOnPlugins) } diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index c60a01223bc..e7054a47464 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -97,6 +97,10 @@ var ( // Admit checks Pods and admits or rejects them. It also resolves the priority of pods based on their PriorityClass. // Note that pod validation mechanism prevents update of a pod priority. func (p *priorityPlugin) Admit(a admission.Attributes) error { + if !utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) { + return nil + } + operation := a.GetOperation() // Ignore all calls to subresources if len(a.GetSubresource()) != 0 { @@ -105,7 +109,7 @@ func (p *priorityPlugin) Admit(a admission.Attributes) error { switch a.GetResource().GroupResource() { case podResource: - if operation == admission.Create { + if operation == admission.Create || operation == admission.Update { return p.admitPod(a) } return nil @@ -157,11 +161,22 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error { return errors.NewBadRequest("resource was marked with kind Pod but was unable to be converted") } - // Make sure that the client has not set `priority` at the time of pod creation. - if operation == admission.Create && pod.Spec.Priority != nil { - return admission.NewForbidden(a, fmt.Errorf("the integer value of priority must not be provided in pod spec. Priority admission controller populates the value from the given PriorityClass name")) + if operation == admission.Update { + oldPod, ok := a.GetOldObject().(*api.Pod) + if !ok { + return errors.NewBadRequest("resource was marked with kind Pod but was unable to be converted") + } + + // This admission plugin set pod.Spec.Priority on create. + // Ensure the existing priority is preserved on update. + // API validation prevents mutations to Priority and PriorityClassName, so any other changes will fail update validation and not be persisted. + if pod.Spec.Priority == nil && oldPod.Spec.Priority != nil { + pod.Spec.Priority = oldPod.Spec.Priority + } + return nil } - if utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) { + + if operation == admission.Create { var priority int32 // TODO: @ravig - This is for backwards compatibility to ensure that critical pods with annotations just work fine. // Remove when no longer needed. @@ -194,6 +209,10 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error { priority = pc.Value } + // if the pod contained a priority that differs from the one computed from the priority class, error + if pod.Spec.Priority != nil && *pod.Spec.Priority != priority { + return admission.NewForbidden(a, fmt.Errorf("the integer value of priority (%d) must not be provided in pod spec; priority admission controller computed %d from the given PriorityClass name", *pod.Spec.Priority, priority)) + } pod.Spec.Priority = &priority } return nil diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index 21e5bf2e728..05f571f3e32 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -244,6 +244,7 @@ func TestDefaultPriority(t *testing.T) { } } +var zeroPriority = int32(0) var intPriority = int32(1000) func TestPodAdmission(t *testing.T) { @@ -389,6 +390,52 @@ func TestPodAdmission(t *testing.T) { PriorityClassName: scheduling.SystemClusterCritical, }, }, + // pod[9]: Pod with a priority value that matches the resolved priority + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-w-zero-priority-in-nonsystem-namespace", + Namespace: "non-system-namespace", + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: containerName, + }, + }, + Priority: &zeroPriority, + }, + }, + // pod[10]: Pod with a priority value that matches the resolved default priority + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-w-priority-matching-default-priority", + Namespace: "non-system-namespace", + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: containerName, + }, + }, + Priority: &defaultClass2.Value, + }, + }, + // pod[11]: Pod with a priority value that matches the resolved priority + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-w-priority-matching-resolved-default-priority", + Namespace: metav1.NamespaceSystem, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: containerName, + }, + }, + PriorityClassName: systemClusterCritical.Name, + Priority: &systemClusterCritical.Value, + }, + }, } // Enable PodPriority feature gate. utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%s=true", features.PodPriority)) @@ -481,6 +528,27 @@ func TestPodAdmission(t *testing.T) { scheduling.SystemCriticalPriority, true, }, + { + "pod with priority that matches computed priority", + []*scheduling.PriorityClass{nondefaultClass1}, + *pods[9], + 0, + false, + }, + { + "pod with priority that matches default priority", + []*scheduling.PriorityClass{defaultClass2}, + *pods[10], + defaultClass2.Value, + false, + }, + { + "pod with priority that matches resolved priority", + []*scheduling.PriorityClass{systemClusterCritical}, + *pods[11], + systemClusterCritical.Value, + false, + }, } for _, test := range tests {