Bugfix: WorkloadSpread cannot patch priorityClassName

Signed-off-by: AiRanthem <[email protected]>

Author: AiRanthem

pkg/util/workloadspread/workloadspread.go

@@ -648,6 +648,14 @@ func injectWorkloadSpreadIntoPod(ws *appsv1alpha1.WorkloadSpread, pod *corev1.Po
 			klog.ErrorS(err, "failed to unmarshal to Pod", "pod", modified)
 			return false, err
 		}
+		if newPod.Spec.PriorityClassName != pod.Spec.PriorityClassName {
+			// Workloadspread webhook is called after builtin admission plugin,
+			// which means the priority is already set before priorityClassName being patched.
+			// Mismatched priorityClassName and priority value will be rejected by apiserver.
+			// we have to clear priority to avoid the problem, and the builtin admission plugin
+			// will be reinvoked after kruise webhook to setting the correct priority value.
+			newPod.Spec.Priority = nil
+		}
 		*pod = *newPod
 	}
 

pkg/util/workloadspread/workloadspread_test.go

@@ -1121,6 +1121,52 @@ func TestWorkloadSpreadMutatingPod(t *testing.T) {
 				return errors.IsNotFound(err)
 			},
 		},
+		{
+			name: "operation = create, pod priority class name is patched",
+			getPod: func() *corev1.Pod {
+				pod := podDemo.DeepCopy()
+				pod.Spec.Priority = utilpointer.Int32(10000)
+				pod.Spec.PriorityClassName = "high"
+				return pod
+			},
+			getWorkloadSpread: func() *appsv1alpha1.WorkloadSpread {
+				ws := workloadSpreadDemo.DeepCopy()
+				ws.Spec.Subsets[0].Patch = runtime.RawExtension{
+					Raw: []byte(`{"spec":{"priorityClassName":"low"}}`),
+				}
+				ws.Spec.Subsets[0].RequiredNodeSelectorTerm = nil
+				ws.Spec.Subsets[0].PreferredNodeSelectorTerms = nil
+				ws.Spec.Subsets[0].Tolerations = nil
+				return ws
+			},
+			getOperation: func() Operation {
+				return CreateOperation
+			},
+			expectPod: func() *corev1.Pod {
+				pod := podDemo.DeepCopy()
+				pod.Spec.Priority = nil
+				pod.Spec.PriorityClassName = "low"
+				pod.Spec.Affinity = &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{}}
+				pod.Annotations[MatchedWorkloadSpreadSubsetAnnotations] = `{"name":"test-ws","subset":"subset-a"}`
+				return pod
+			},
+			expectWorkloadSpread: func() *appsv1alpha1.WorkloadSpread {
+				ws := workloadSpreadDemo.DeepCopy()
+				ws.Spec.Subsets[0].Patch = runtime.RawExtension{
+					Raw: []byte(`{"spec":{"priorityClassName":"low"}}`),
+				}
+				ws.ResourceVersion = "1"
+				ws.Status.SubsetStatuses[0].MissingReplicas = 4
+				ws.Status.SubsetStatuses[0].CreatingPods[podDemo.Name] = metav1.Time{Time: time.Now()}
+				ws.Status.VersionedSubsetStatuses = map[string][]appsv1alpha1.WorkloadSpreadSubsetStatus{
+					VersionIgnored: ws.Status.SubsetStatuses,
+				}
+				ws.Spec.Subsets[0].RequiredNodeSelectorTerm = nil
+				ws.Spec.Subsets[0].PreferredNodeSelectorTerms = nil
+				ws.Spec.Subsets[0].Tolerations = nil
+				return ws
+			},
+		},
 	}
 	for _, cs := range cases {
 		t.Run(cs.name, func(t *testing.T) {

test/e2e/apps/workloadspread.go

@@ -27,6 +27,7 @@ import (
 	"github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	schedulingv1 "k8s.io/api/scheduling/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -1036,6 +1037,14 @@ var _ = SIGDescribe("workloadspread", func() {
 		})
 
 		framework.ConformanceIt("only one subset, zone-a=nil", func() {
+			priorityClass := &schedulingv1.PriorityClass{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-priority-class",
+				},
+				Value:         100,
+				GlobalDefault: false,
+			}
+			tester.CreatePriorityClass(priorityClass)
 			cloneSet := tester.NewBaseCloneSet(ns)
 			// create workloadSpread
 			targetRef := appsv1alpha1.TargetReference{
@@ -1056,7 +1065,7 @@ var _ = SIGDescribe("workloadspread", func() {
 				},
 				MaxReplicas: nil,
 				Patch: runtime.RawExtension{
-					Raw: []byte(`{"metadata":{"annotations":{"subset":"zone-a"}}}`),
+					Raw: []byte(`{"metadata":{"annotations":{"subset":"zone-a"}},"spec":{"priorityClassName":"test-priority-class"}}`),
 				},
 			}
 
@@ -1083,6 +1092,8 @@ var _ = SIGDescribe("workloadspread", func() {
 						gomega.Expect(injectWorkloadSpread.Name).To(gomega.Equal(workloadSpread.Name))
 						gomega.Expect(pod.Spec.Affinity).NotTo(gomega.BeNil())
 						gomega.Expect(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions).To(gomega.Equal(subset1.RequiredNodeSelectorTerm.MatchExpressions))
+						gomega.Expect(pod.Spec.PriorityClassName).To(gomega.Equal("test-priority-class"))
+						gomega.Expect(*pod.Spec.Priority).To(gomega.BeEquivalentTo(100))
 						gomega.Expect(pod.Annotations[workloadspread.PodDeletionCostAnnotation]).To(gomega.Equal("100"))
 					}
 				} else {
@@ -1123,6 +1134,8 @@ var _ = SIGDescribe("workloadspread", func() {
 						gomega.Expect(injectWorkloadSpread.Name).To(gomega.Equal(workloadSpread.Name))
 						gomega.Expect(pod.Spec.Affinity).NotTo(gomega.BeNil())
 						gomega.Expect(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions).To(gomega.Equal(subset1.RequiredNodeSelectorTerm.MatchExpressions))
+						gomega.Expect(pod.Spec.PriorityClassName).To(gomega.Equal("test-priority-class"))
+						gomega.Expect(*pod.Spec.Priority).To(gomega.BeEquivalentTo(100))
 						gomega.Expect(pod.Annotations[workloadspread.PodDeletionCostAnnotation]).To(gomega.Equal("100"))
 					}
 				} else {
@@ -1163,6 +1176,8 @@ var _ = SIGDescribe("workloadspread", func() {
 						gomega.Expect(injectWorkloadSpread.Name).To(gomega.Equal(workloadSpread.Name))
 						gomega.Expect(pod.Spec.Affinity).NotTo(gomega.BeNil())
 						gomega.Expect(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions).To(gomega.Equal(subset1.RequiredNodeSelectorTerm.MatchExpressions))
+						gomega.Expect(pod.Spec.PriorityClassName).To(gomega.Equal("test-priority-class"))
+						gomega.Expect(*pod.Spec.Priority).To(gomega.BeEquivalentTo(100))
 						gomega.Expect(pod.Annotations[workloadspread.PodDeletionCostAnnotation]).To(gomega.Equal("100"))
 					}
 				} else {

test/e2e/framework/workloadspread_util.go

@@ -29,6 +29,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "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"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -40,6 +41,7 @@ import (
 	kubecontroller "k8s.io/kubernetes/pkg/controller"
 	imageutils "k8s.io/kubernetes/test/utils/image"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type WorkloadSpreadTester struct {
@@ -352,6 +354,17 @@ func (t *WorkloadSpreadTester) WaitForWorkloadSpreadRunning(ws *appsv1alpha1.Wor
 	}
 }
 
+func (t *WorkloadSpreadTester) CreatePriorityClass(pc *schedulingv1.PriorityClass) *schedulingv1.PriorityClass {
+	gomega.Eventually(func(g gomega.Gomega) {
+		Logf("create priorityClass (%s)", pc.Name)
+		_, err := t.C.SchedulingV1().PriorityClasses().Create(context.TODO(), pc, metav1.CreateOptions{})
+		g.Expect(client.IgnoreAlreadyExists(err)).NotTo(gomega.HaveOccurred())
+		Logf("create priorityClass (%s/%s) success", pc.Namespace, pc.Name)
+		pc, _ = t.C.SchedulingV1().PriorityClasses().Get(context.TODO(), pc.Name, metav1.GetOptions{})
+	}).WithTimeout(20 * time.Second).WithPolling(time.Second).Should(gomega.Succeed())
+	return pc
+}
+
 func (t *WorkloadSpreadTester) CreateCloneSet(cloneSet *appsv1alpha1.CloneSet) *appsv1alpha1.CloneSet {
 	Logf("create CloneSet (%s/%s)", cloneSet.Namespace, cloneSet.Name)
 	_, err := t.KC.AppsV1alpha1().CloneSets(cloneSet.Namespace).Create(context.TODO(), cloneSet, metav1.CreateOptions{})