github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-24 04:06:03 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #65739 from liggitt/ravisantoshgudimetla-priorityplugin-default

Browse Source 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Update priority admission to improve interoperability

Builds on https://github.com/kubernetes/kubernetes/pull/65722

Makes the following adjustments to the priority admission plugin:
* allows creation of pods to include an explicit priority field if it matches the computed priority (allows export/import cases to continue to work on the same cluster, between clusters that match priorityClass values, and between clusters where priority is unused and all pods get `priority:0`)
* preserves existing priority if a pod update does not include a priority value and the old pod did (allows POST, PUT, PUT, PUT workflows to continue to work, with the admission-set value on create being preserved by the admission plugin on update)

This should avoid the failures revealed by the kubectl tests exercising the pod API without any awareness of the priority feature

/sig scheduling
/cc @bsalamat

```release-note
kube-apiserver: the `Priority` admission plugin is now enabled by default when using `--enable-admission-plugins`. If using `--admission-control` to fully specify the set of admission plugins, the `Priority` admission plugin should be added if using the `PodPriority` feature, which is enabled by default in 1.11.
```
This commit is contained in:
Kubernetes Submit Queue 2018-07-03 13:22:49 -07:00 committed by GitHub
commit 5a8a979fda
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 100 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/kubeapiserver/options/BUILD
View File

@ -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",

6
pkg/kubeapiserver/options/plugins.go
View File

@ -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)
}

29
plugin/pkg/admission/priority/admission.go
View File

@ -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

68
plugin/pkg/admission/priority/admission_test.go
View File

@ -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 {