From 71690f5f59a46fe8a32ef81250fd60bacfbb6a9b Mon Sep 17 00:00:00 2001 From: AiRanthem Date: Fri, 10 Jan 2025 12:48:27 +0800 Subject: [PATCH] Bugfix: WorkloadSpread cannot patch priorityClassName Signed-off-by: AiRanthem --- pkg/util/workloadspread/workloadspread.go | 8 ++++ .../workloadspread/workloadspread_test.go | 46 +++++++++++++++++++ test/e2e/apps/workloadspread.go | 17 ++++++- test/e2e/framework/workloadspread_util.go | 13 ++++++ 4 files changed, 83 insertions(+), 1 deletion(-) diff --git a/pkg/util/workloadspread/workloadspread.go b/pkg/util/workloadspread/workloadspread.go index ff5cf0d3f3..1b340b124a 100644 --- a/pkg/util/workloadspread/workloadspread.go +++ b/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 } diff --git a/pkg/util/workloadspread/workloadspread_test.go b/pkg/util/workloadspread/workloadspread_test.go index 81399693a3..e7107224b7 100644 --- a/pkg/util/workloadspread/workloadspread_test.go +++ b/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) { diff --git a/test/e2e/apps/workloadspread.go b/test/e2e/apps/workloadspread.go index 290100f356..8bde5fa60a 100644 --- a/test/e2e/apps/workloadspread.go +++ b/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 { diff --git a/test/e2e/framework/workloadspread_util.go b/test/e2e/framework/workloadspread_util.go index e72d5a772e..c13bcbd99e 100644 --- a/test/e2e/framework/workloadspread_util.go +++ b/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{})