change preempting to PreemptionPolicy

plugin/pkg/admission/priority/BUILD

@@ -37,6 +37,7 @@ go_library(
         "//pkg/apis/scheduling:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/kubelet/types:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/api/scheduling/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

plugin/pkg/admission/priority/admission.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 
+	apiv1 "k8s.io/api/core/v1"
 	schedulingv1 "k8s.io/api/scheduling/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -30,6 +31,7 @@ import (
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes"
 	schedulingv1listers "k8s.io/client-go/listers/scheduling/v1"
+	"k8s.io/kubernetes/pkg/apis/core"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/scheduling"
 	"k8s.io/kubernetes/pkg/features"
@@ -179,7 +181,7 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error {
 
 	if operation == admission.Create {
 		var priority int32
-		var preempting *bool
+		var preemptionPolicy *apiv1.PreemptionPolicy
 		// TODO: @ravig - This is for backwards compatibility to ensure that critical pods with annotations just work fine.
 		// Remove when no longer needed.
 		if len(pod.Spec.PriorityClassName) == 0 &&
@@ -190,7 +192,7 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error {
 		if len(pod.Spec.PriorityClassName) == 0 {
 			var err error
 			var pcName string
-			pcName, priority, preempting, err = p.getDefaultPriority()
+			pcName, priority, preemptionPolicy, err = p.getDefaultPriority()
 			if err != nil {
 				return fmt.Errorf("failed to get default priority class: %v", err)
 			}
@@ -212,14 +214,24 @@ func (p *priorityPlugin) admitPod(a admission.Attributes) error {
 			}
 
 			priority = pc.Value
-			preempting = pc.Preempting
+			preemptionPolicy = pc.PreemptionPolicy
 		}
 		// 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
-		pod.Spec.Preempting = preempting
+
+		if utilfeature.DefaultFeatureGate.Enabled(features.NonPreemptingPriority) {
+			var corePolicy core.PreemptionPolicy
+			if preemptionPolicy != nil {
+				corePolicy = core.PreemptionPolicy(*preemptionPolicy)
+				if pod.Spec.PreemptionPolicy != nil && *pod.Spec.PreemptionPolicy != corePolicy {
+					return admission.NewForbidden(a, fmt.Errorf("the string value of PreemptionPolicy (%s) must not be provided in pod spec; priority admission controller computed %s from the given PriorityClass name", *pod.Spec.PreemptionPolicy, corePolicy))
+				}
+				pod.Spec.PreemptionPolicy = &corePolicy
+			}
+		}
 	}
 	return nil
 }
@@ -265,15 +277,14 @@ func (p *priorityPlugin) getDefaultPriorityClass() (*schedulingv1.PriorityClass,
 	return defaultPC, nil
 }
 
-func (p *priorityPlugin) getDefaultPriority() (string, int32, *bool, error) {
+func (p *priorityPlugin) getDefaultPriority() (string, int32, *apiv1.PreemptionPolicy, error) {
 	dpc, err := p.getDefaultPriorityClass()
 	if err != nil {
 		return "", 0, nil, err
 	}
 	if dpc != nil {
-		return dpc.Name, dpc.Value, dpc.Preempting, nil
+		return dpc.Name, dpc.Value, dpc.PreemptionPolicy, nil
 	}
-	preempting := true
-
-	return "", int32(scheduling.DefaultPriorityWhenNoDefaultClassExists), &preempting, nil
+	preemptLowerPriority := apiv1.PreemptLowerPriority
+	return "", int32(scheduling.DefaultPriorityWhenNoDefaultClassExists), &preemptLowerPriority, nil
 }

plugin/pkg/admission/priority/admission_test.go

@@ -50,6 +50,11 @@ func addPriorityClasses(ctrl *priorityPlugin, priorityClasses []*scheduling.Prio
 	return nil
 }
 
+var (
+	preemptNever         = api.PreemptNever
+	preemptLowerPriority = api.PreemptLowerPriority
+)
+
 var defaultClass1 = &scheduling.PriorityClass{
 	TypeMeta: metav1.TypeMeta{
 		Kind: "PriorityClass",
@@ -94,6 +99,32 @@ var systemClusterCritical = &scheduling.PriorityClass{
 	GlobalDefault: true,
 }
 
+var neverPreemptionPolicyClass = &scheduling.PriorityClass{
+	TypeMeta: metav1.TypeMeta{
+		Kind: "PriorityClass",
+	},
+	ObjectMeta: metav1.ObjectMeta{
+		Name: "nopreemptionpolicy",
+	},
+	Value:            2000,
+	Description:      "Just a test priority class",
+	GlobalDefault:    true,
+	PreemptionPolicy: &preemptNever,
+}
+
+var preemptionPolicyClass = &scheduling.PriorityClass{
+	TypeMeta: metav1.TypeMeta{
+		Kind: "PriorityClass",
+	},
+	ObjectMeta: metav1.ObjectMeta{
+		Name: "nopreemptionpolicy",
+	},
+	Value:            2000,
+	Description:      "Just a test priority class",
+	GlobalDefault:    true,
+	PreemptionPolicy: &preemptLowerPriority,
+}
+
 func TestPriorityClassAdmission(t *testing.T) {
 	var systemClass = &scheduling.PriorityClass{
 		TypeMeta: metav1.TypeMeta{
@@ -468,19 +499,73 @@ func TestPodAdmission(t *testing.T) {
 				Priority:          &systemClusterCritical.Value,
 			},
 		},
+		// pod[12]: Pod without a preemption policy that matches the resolved preemption policy
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "pod-never-preemption-policy-matching-resolved-preemption-policy",
+				Namespace: metav1.NamespaceSystem,
+			},
+			Spec: api.PodSpec{
+				Containers: []api.Container{
+					{
+						Name: containerName,
+					},
+				},
+				PriorityClassName: neverPreemptionPolicyClass.Name,
+				Priority:          &neverPreemptionPolicyClass.Value,
+				PreemptionPolicy:  nil,
+			},
+		},
+		// pod[13]: Pod with a preemption policy that matches the resolved preemption policy
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "pod-preemption-policy-matching-resolved-preemption-policy",
+				Namespace: metav1.NamespaceSystem,
+			},
+			Spec: api.PodSpec{
+				Containers: []api.Container{
+					{
+						Name: containerName,
+					},
+				},
+				PriorityClassName: preemptionPolicyClass.Name,
+				Priority:          &preemptionPolicyClass.Value,
+				PreemptionPolicy:  &preemptLowerPriority,
+			},
+		},
+		// pod[14]: Pod with a preemption policy that does't match the resolved preemption policy
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "pod-preemption-policy-not-matching-resolved-preemption-policy",
+				Namespace: metav1.NamespaceSystem,
+			},
+			Spec: api.PodSpec{
+				Containers: []api.Container{
+					{
+						Name: containerName,
+					},
+				},
+				PriorityClassName: preemptionPolicyClass.Name,
+				Priority:          &preemptionPolicyClass.Value,
+				PreemptionPolicy:  &preemptNever,
+			},
+		},
 	}
 	// Enable PodPriority feature gate.
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodPriority, true)()
 	// Enable ExperimentalCriticalPodAnnotation feature gate.
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExperimentalCriticalPodAnnotation, true)()
+	// Enable NonPreemptingPriority feature gate.
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NonPreemptingPriority, true)()
 	tests := []struct {
 		name            string
 		existingClasses []*scheduling.PriorityClass
 		// Admission controller changes pod spec. So, we take an api.Pod instead of
 		// *api.Pod to avoid interfering with other tests.
-		pod              api.Pod
-		expectedPriority int32
-		expectError      bool
+		pod                    api.Pod
+		expectedPriority       int32
+		expectError            bool
+		expectPreemptionPolicy *api.PreemptionPolicy
 	}{
 		{
 			"Pod with priority class",
@@ -488,6 +573,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[0],
 			1000,
 			false,
+			nil,
 		},
 
 		{
@@ -496,6 +582,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[1],
 			1000,
 			false,
+			nil,
 		},
 		{
 			"pod without priority class and no existing priority class",
@@ -503,6 +590,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[1],
 			scheduling.DefaultPriorityWhenNoDefaultClassExists,
 			false,
+			nil,
 		},
 		{
 			"pod without priority class and no default class",
@@ -510,6 +598,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[1],
 			scheduling.DefaultPriorityWhenNoDefaultClassExists,
 			false,
+			nil,
 		},
 		{
 			"pod with a system priority class",
@@ -517,6 +606,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[4],
 			scheduling.SystemCriticalPriority,
 			false,
+			nil,
 		},
 		{
 			"Pod with non-existing priority class",
@@ -524,6 +614,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[2],
 			0,
 			true,
+			nil,
 		},
 		{
 			"pod with integer priority",
@@ -531,6 +622,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[3],
 			0,
 			true,
+			nil,
 		},
 		{
 			"mirror pod with system priority class",
@@ -538,6 +630,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[5],
 			scheduling.SystemCriticalPriority,
 			false,
+			nil,
 		},
 		{
 			"mirror pod with integer priority",
@@ -545,6 +638,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[6],
 			0,
 			true,
+			nil,
 		},
 		{
 			"pod with critical pod annotation",
@@ -552,6 +646,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[7],
 			scheduling.SystemCriticalPriority,
 			false,
+			nil,
 		},
 		{
 			"pod with system critical priority in non-system namespace",
@@ -559,6 +654,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[8],
 			scheduling.SystemCriticalPriority,
 			true,
+			nil,
 		},
 		{
 			"pod with priority that matches computed priority",
@@ -566,6 +662,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[9],
 			0,
 			false,
+			nil,
 		},
 		{
 			"pod with priority that matches default priority",
@@ -573,6 +670,7 @@ func TestPodAdmission(t *testing.T) {
 			*pods[10],
 			defaultClass2.Value,
 			false,
+			nil,
 		},
 		{
 			"pod with priority that matches resolved priority",
@@ -580,6 +678,31 @@ func TestPodAdmission(t *testing.T) {
 			*pods[11],
 			systemClusterCritical.Value,
 			false,
+			nil,
+		},
+		{
+			"pod with nil preemtpion policy",
+			[]*scheduling.PriorityClass{preemptionPolicyClass},
+			*pods[12],
+			preemptionPolicyClass.Value,
+			false,
+			nil,
+		},
+		{
+			"pod with preemtpion policy that matches resolved preemtpion policy",
+			[]*scheduling.PriorityClass{preemptionPolicyClass},
+			*pods[13],
+			preemptionPolicyClass.Value,
+			false,
+			&preemptLowerPriority,
+		},
+		{
+			"pod with preemtpion policy that does't matches resolved preemtpion policy",
+			[]*scheduling.PriorityClass{preemptionPolicyClass},
+			*pods[14],
+			preemptionPolicyClass.Value,
+			true,
+			&preemptLowerPriority,
 		},
 	}
 
@@ -613,6 +736,8 @@ func TestPodAdmission(t *testing.T) {
 				t.Errorf("Test %q: unexpected error received: %v", test.name, err)
 			} else if *test.pod.Spec.Priority != test.expectedPriority {
 				t.Errorf("Test %q: expected priority is %d, but got %d.", test.name, test.expectedPriority, *test.pod.Spec.Priority)
+			} else if test.pod.Spec.PreemptionPolicy != nil && test.expectPreemptionPolicy != nil && *test.pod.Spec.PreemptionPolicy != *test.expectPreemptionPolicy {
+				t.Errorf("Test %q: expected preemption policy is %s, but got %s.", test.name, *test.expectPreemptionPolicy, *test.pod.Spec.PreemptionPolicy)
 			}
 		}
 		if err == nil && test.expectError {

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go

@@ -48,7 +48,6 @@ const (
 	extensionsGroup     = "extensions"
 	policyGroup         = "policy"
 	rbacGroup           = "rbac.authorization.k8s.io"
-	schedulingGroup     = "scheduling.k8s.io"
 	storageGroup        = "storage.k8s.io"
 	resMetricsGroup     = "metrics.k8s.io"
 	customMetricsGroup  = "custom.metrics.k8s.io"
@@ -443,7 +442,6 @@ func ClusterRoles() []rbacv1.ClusterRole {
 				// Needed to check API access.  These creates are non-mutating
 				rbacv1helpers.NewRule("create").Groups(authenticationGroup).Resources("tokenreviews").RuleOrDie(),
 				rbacv1helpers.NewRule("create").Groups(authorizationGroup).Resources("subjectaccessreviews").RuleOrDie(),
-				rbacv1helpers.NewRule(Read...).Groups(schedulingGroup).Resources("priorityclasses").RuleOrDie(),
 			},
 		},
 		{

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml

@@ -801,14 +801,6 @@ items:
     - subjectaccessreviews
     verbs:
     - create
-  - apiGroups:
-    - scheduling.k8s.io
-    resources:
-    - priorityclasses
-    verbs:
-    - get
-    - list
-    - watch
 - apiVersion: rbac.authorization.k8s.io/v1
   kind: ClusterRole
   metadata: