Skip to content

Commit cd23dc1

Browse files
AiRanthemfurykerry
AiRanthem
authored and
furykerry
committed
Bugfix: WorkloadSpread cannot patch priorityClassName
Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
1 parent 58c1ecb commit cd23dc1

File tree

4 files changed

+83
-1
lines changed

4 files changed

+83
-1
lines changed

‎pkg/util/workloadspread/workloadspread.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,14 @@ func injectWorkloadSpreadIntoPod(ws *appsv1alpha1.WorkloadSpread, pod *corev1.Po
648648
klog.ErrorS(err, "failed to unmarshal to Pod", "pod", modified)
649649
return false, err
650650
}
651+
if newPod.Spec.PriorityClassName != pod.Spec.PriorityClassName {
652+
// Workloadspread webhook is called after builtin admission plugin,
653+
// which means the priority is already set before priorityClassName being patched.
654+
// Mismatched priorityClassName and priority value will be rejected by apiserver.
655+
// we have to clear priority to avoid the problem, and the builtin admission plugin
656+
// will be reinvoked after kruise webhook to setting the correct priority value.
657+
newPod.Spec.Priority = nil
658+
}
651659
*pod = *newPod
652660
}
653661

‎pkg/util/workloadspread/workloadspread_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,52 @@ func TestWorkloadSpreadMutatingPod(t *testing.T) {
11211121
return errors.IsNotFound(err)
11221122
},
11231123
},
1124+
{
1125+
name: "operation = create, pod priority class name is patched",
1126+
getPod: func() *corev1.Pod {
1127+
pod := podDemo.DeepCopy()
1128+
pod.Spec.Priority = utilpointer.Int32(10000)
1129+
pod.Spec.PriorityClassName = "high"
1130+
return pod
1131+
},
1132+
getWorkloadSpread: func() *appsv1alpha1.WorkloadSpread {
1133+
ws := workloadSpreadDemo.DeepCopy()
1134+
ws.Spec.Subsets[0].Patch = runtime.RawExtension{
1135+
Raw: []byte(`{"spec":{"priorityClassName":"low"}}`),
1136+
}
1137+
ws.Spec.Subsets[0].RequiredNodeSelectorTerm = nil
1138+
ws.Spec.Subsets[0].PreferredNodeSelectorTerms = nil
1139+
ws.Spec.Subsets[0].Tolerations = nil
1140+
return ws
1141+
},
1142+
getOperation: func() Operation {
1143+
return CreateOperation
1144+
},
1145+
expectPod: func() *corev1.Pod {
1146+
pod := podDemo.DeepCopy()
1147+
pod.Spec.Priority = nil
1148+
pod.Spec.PriorityClassName = "low"
1149+
pod.Spec.Affinity = &corev1.Affinity{NodeAffinity: &corev1.NodeAffinity{}}
1150+
pod.Annotations[MatchedWorkloadSpreadSubsetAnnotations] = `{"name":"test-ws","subset":"subset-a"}`
1151+
return pod
1152+
},
1153+
expectWorkloadSpread: func() *appsv1alpha1.WorkloadSpread {
1154+
ws := workloadSpreadDemo.DeepCopy()
1155+
ws.Spec.Subsets[0].Patch = runtime.RawExtension{
1156+
Raw: []byte(`{"spec":{"priorityClassName":"low"}}`),
1157+
}
1158+
ws.ResourceVersion = "1"
1159+
ws.Status.SubsetStatuses[0].MissingReplicas = 4
1160+
ws.Status.SubsetStatuses[0].CreatingPods[podDemo.Name] = metav1.Time{Time: time.Now()}
1161+
ws.Status.VersionedSubsetStatuses = map[string][]appsv1alpha1.WorkloadSpreadSubsetStatus{
1162+
VersionIgnored: ws.Status.SubsetStatuses,
1163+
}
1164+
ws.Spec.Subsets[0].RequiredNodeSelectorTerm = nil
1165+
ws.Spec.Subsets[0].PreferredNodeSelectorTerms = nil
1166+
ws.Spec.Subsets[0].Tolerations = nil
1167+
return ws
1168+
},
1169+
},
11241170
}
11251171
for _, cs := range cases {
11261172
t.Run(cs.name, func(t *testing.T) {

‎test/e2e/apps/workloadspread.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/onsi/gomega"
2828
appsv1 "k8s.io/api/apps/v1"
2929
corev1 "k8s.io/api/core/v1"
30+
schedulingv1 "k8s.io/api/scheduling/v1"
3031
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3132
"k8s.io/apimachinery/pkg/runtime"
3233
"k8s.io/apimachinery/pkg/types"
@@ -1036,6 +1037,14 @@ var _ = SIGDescribe("workloadspread", func() {
10361037
})
10371038

10381039
framework.ConformanceIt("only one subset, zone-a=nil", func() {
1040+
priorityClass := &schedulingv1.PriorityClass{
1041+
ObjectMeta: metav1.ObjectMeta{
1042+
Name: "test-priority-class",
1043+
},
1044+
Value: 100,
1045+
GlobalDefault: false,
1046+
}
1047+
tester.CreatePriorityClass(priorityClass)
10391048
cloneSet := tester.NewBaseCloneSet(ns)
10401049
// create workloadSpread
10411050
targetRef := appsv1alpha1.TargetReference{
@@ -1056,7 +1065,7 @@ var _ = SIGDescribe("workloadspread", func() {
10561065
},
10571066
MaxReplicas: nil,
10581067
Patch: runtime.RawExtension{
1059-
Raw: []byte(`{"metadata":{"annotations":{"subset":"zone-a"}}}`),
1068+
Raw: []byte(`{"metadata":{"annotations":{"subset":"zone-a"}},"spec":{"priorityClassName":"test-priority-class"}}`),
10601069
},
10611070
}
10621071

@@ -1083,6 +1092,8 @@ var _ = SIGDescribe("workloadspread", func() {
10831092
gomega.Expect(injectWorkloadSpread.Name).To(gomega.Equal(workloadSpread.Name))
10841093
gomega.Expect(pod.Spec.Affinity).NotTo(gomega.BeNil())
10851094
gomega.Expect(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions).To(gomega.Equal(subset1.RequiredNodeSelectorTerm.MatchExpressions))
1095+
gomega.Expect(pod.Spec.PriorityClassName).To(gomega.Equal("test-priority-class"))
1096+
gomega.Expect(*pod.Spec.Priority).To(gomega.BeEquivalentTo(100))
10861097
gomega.Expect(pod.Annotations[workloadspread.PodDeletionCostAnnotation]).To(gomega.Equal("100"))
10871098
}
10881099
} else {
@@ -1123,6 +1134,8 @@ var _ = SIGDescribe("workloadspread", func() {
11231134
gomega.Expect(injectWorkloadSpread.Name).To(gomega.Equal(workloadSpread.Name))
11241135
gomega.Expect(pod.Spec.Affinity).NotTo(gomega.BeNil())
11251136
gomega.Expect(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions).To(gomega.Equal(subset1.RequiredNodeSelectorTerm.MatchExpressions))
1137+
gomega.Expect(pod.Spec.PriorityClassName).To(gomega.Equal("test-priority-class"))
1138+
gomega.Expect(*pod.Spec.Priority).To(gomega.BeEquivalentTo(100))
11261139
gomega.Expect(pod.Annotations[workloadspread.PodDeletionCostAnnotation]).To(gomega.Equal("100"))
11271140
}
11281141
} else {
@@ -1163,6 +1176,8 @@ var _ = SIGDescribe("workloadspread", func() {
11631176
gomega.Expect(injectWorkloadSpread.Name).To(gomega.Equal(workloadSpread.Name))
11641177
gomega.Expect(pod.Spec.Affinity).NotTo(gomega.BeNil())
11651178
gomega.Expect(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions).To(gomega.Equal(subset1.RequiredNodeSelectorTerm.MatchExpressions))
1179+
gomega.Expect(pod.Spec.PriorityClassName).To(gomega.Equal("test-priority-class"))
1180+
gomega.Expect(*pod.Spec.Priority).To(gomega.BeEquivalentTo(100))
11661181
gomega.Expect(pod.Annotations[workloadspread.PodDeletionCostAnnotation]).To(gomega.Equal("100"))
11671182
}
11681183
} else {

‎test/e2e/framework/workloadspread_util.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
appsv1 "k8s.io/api/apps/v1"
3030
batchv1 "k8s.io/api/batch/v1"
3131
corev1 "k8s.io/api/core/v1"
32+
schedulingv1 "k8s.io/api/scheduling/v1"
3233
"k8s.io/apimachinery/pkg/api/errors"
3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3435
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -40,6 +41,7 @@ import (
4041
kubecontroller "k8s.io/kubernetes/pkg/controller"
4142
imageutils "k8s.io/kubernetes/test/utils/image"
4243
"k8s.io/utils/ptr"
44+
"sigs.k8s.io/controller-runtime/pkg/client"
4345
)
4446

4547
type WorkloadSpreadTester struct {
@@ -352,6 +354,17 @@ func (t *WorkloadSpreadTester) WaitForWorkloadSpreadRunning(ws *appsv1alpha1.Wor
352354
}
353355
}
354356

357+
func (t *WorkloadSpreadTester) CreatePriorityClass(pc *schedulingv1.PriorityClass) *schedulingv1.PriorityClass {
358+
gomega.Eventually(func(g gomega.Gomega) {
359+
Logf("create priorityClass (%s)", pc.Name)
360+
_, err := t.C.SchedulingV1().PriorityClasses().Create(context.TODO(), pc, metav1.CreateOptions{})
361+
g.Expect(client.IgnoreAlreadyExists(err)).NotTo(gomega.HaveOccurred())
362+
Logf("create priorityClass (%s/%s) success", pc.Namespace, pc.Name)
363+
pc, _ = t.C.SchedulingV1().PriorityClasses().Get(context.TODO(), pc.Name, metav1.GetOptions{})
364+
}).WithTimeout(20 * time.Second).WithPolling(time.Second).Should(gomega.Succeed())
365+
return pc
366+
}
367+
355368
func (t *WorkloadSpreadTester) CreateCloneSet(cloneSet *appsv1alpha1.CloneSet) *appsv1alpha1.CloneSet {
356369
Logf("create CloneSet (%s/%s)", cloneSet.Namespace, cloneSet.Name)
357370
_, err := t.KC.AppsV1alpha1().CloneSets(cloneSet.Namespace).Create(context.TODO(), cloneSet, metav1.CreateOptions{})

0 commit comments

Comments
 (0)