Skip to content

Commit 2ed2cd0

Browse files
FillZppFillZpp
authored
Optimize workloadspread feature-gate and object decode for pod deletion (#734)
Signed-off-by: FillZpp <[email protected]>
1 parent c3a87e0 commit 2ed2cd0

File tree

5 files changed

+45
-21
lines changed

5 files changed

+45
-21
lines changed

‎pkg/features/kruise_features.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ const (
6767
// Protection only pod update request
6868
PodUnavailableBudgetUpdateGate featuregate.Feature = "PodUnavailableBudgetUpdateGate"
6969

70-
// WorkloadSpreadGate enable WorkloadSpread to constrain the spread of the workload.
71-
WorkloadSpreadGate featuregate.Feature = "WorkloadSpreadGate"
70+
// WorkloadSpread enable WorkloadSpread to constrain the spread of the workload.
71+
WorkloadSpread featuregate.Feature = "WorkloadSpread"
7272

7373
// DaemonWatchingPod enables kruise-daemon to list watch pods that belong to the same node.
7474
DaemonWatchingPod featuregate.Feature = "DaemonWatchingPod"
@@ -80,16 +80,16 @@ const (
8080
)
8181

8282
var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
83-
PodWebhook: {Default: true, PreRelease: featuregate.Beta},
84-
KruiseDaemon: {Default: true, PreRelease: featuregate.Beta},
85-
DaemonWatchingPod: {Default: true, PreRelease: featuregate.Beta},
86-
WorkloadSpreadGate: {Default: true, PreRelease: featuregate.Beta},
83+
PodWebhook: {Default: true, PreRelease: featuregate.Beta},
84+
KruiseDaemon: {Default: true, PreRelease: featuregate.Beta},
85+
DaemonWatchingPod: {Default: true, PreRelease: featuregate.Beta},
8786

8887
CloneSetShortHash: {Default: false, PreRelease: featuregate.Alpha},
8988
KruisePodReadinessGate: {Default: false, PreRelease: featuregate.Alpha},
9089
PreDownloadImageForInPlaceUpdate: {Default: false, PreRelease: featuregate.Alpha},
9190
CloneSetPartitionRollback: {Default: false, PreRelease: featuregate.Alpha},
9291
ResourcesDeletionProtection: {Default: false, PreRelease: featuregate.Alpha},
92+
WorkloadSpread: {Default: false, PreRelease: featuregate.Alpha},
9393
PodUnavailableBudgetDeleteGate: {Default: false, PreRelease: featuregate.Alpha},
9494
PodUnavailableBudgetUpdateGate: {Default: false, PreRelease: featuregate.Alpha},
9595
TemplateNoDefaults: {Default: false, PreRelease: featuregate.Alpha},
@@ -116,7 +116,7 @@ func SetDefaultFeatureGates() {
116116
if !utilfeature.DefaultFeatureGate.Enabled(PodWebhook) {
117117
_ = utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=false", KruisePodReadinessGate))
118118
_ = utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=false", ResourcesDeletionProtection))
119-
_ = utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=false", WorkloadSpreadGate))
119+
_ = utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=false", WorkloadSpread))
120120
}
121121
if !utilfeature.DefaultFeatureGate.Enabled(KruiseDaemon) {
122122
_ = utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=false", PreDownloadImageForInPlaceUpdate))

‎pkg/util/workloadspread/workloadspread.go

+5-7
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,13 @@ type workload struct {
6868
}
6969

7070
var (
71-
workloads []workload
71+
workloads = []workload{
72+
{Kind: controllerKruiseKindCS.Kind, Groups: []string{controllerKruiseKindCS.Group}},
73+
{Kind: controllerKindRS.Kind, Groups: []string{controllerKindRS.Group}},
74+
{Kind: controllerKindJob.Kind, Groups: []string{controllerKindJob.Group}},
75+
}
7276
)
7377

74-
func init() {
75-
workloads = append(workloads, workload{Kind: controllerKruiseKindCS.Kind, Groups: []string{controllerKruiseKindCS.Group}})
76-
workloads = append(workloads, workload{Kind: controllerKindRS.Kind, Groups: []string{controllerKindRS.Group}})
77-
workloads = append(workloads, workload{Kind: controllerKindJob.Kind, Groups: []string{controllerKindJob.Group}})
78-
}
79-
8078
type Handler struct {
8179
client.Client
8280
}

‎pkg/webhook/pod/mutating/pod_create_update_handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (h *PodCreateHandler) Handle(ctx context.Context, req admission.Request) ad
6161

6262
injectPodReadinessGate(req, obj)
6363

64-
if utilfeature.DefaultFeatureGate.Enabled(features.WorkloadSpreadGate) {
64+
if utilfeature.DefaultFeatureGate.Enabled(features.WorkloadSpread) {
6565
err = h.workloadSpreadMutatingPod(ctx, req, obj)
6666
if err != nil {
6767
return admission.Errored(http.StatusInternalServerError, err)

‎pkg/webhook/pod/validating/pod_create_update_handler.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
utilfeature "github.com/openkruise/kruise/pkg/util/feature"
2626

2727
admissionv1beta1 "k8s.io/api/admission/v1beta1"
28+
"k8s.io/klog"
2829
"sigs.k8s.io/controller-runtime/pkg/client"
2930
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
3031
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -46,13 +47,18 @@ type PodCreateHandler struct {
4647

4748
func (h *PodCreateHandler) validatingPodFn(ctx context.Context, req admission.Request) (allowed bool, reason string, err error) {
4849
allowed = true
50+
if req.Operation == admissionv1beta1.Delete && len(req.OldObject.Raw) == 0 {
51+
klog.Warningf("Skip to validate pod %s/%s deletion for no old object, maybe because of Kubernetes version < 1.16", req.Namespace, req.Name)
52+
return
53+
}
54+
4955
switch req.Operation {
5056
case admissionv1beta1.Update:
5157
if utilfeature.DefaultFeatureGate.Enabled(features.PodUnavailableBudgetUpdateGate) {
5258
allowed, reason, err = h.podUnavailableBudgetValidatingPod(ctx, req)
5359
}
5460
case admissionv1beta1.Delete, admissionv1beta1.Create:
55-
if utilfeature.DefaultFeatureGate.Enabled(features.WorkloadSpreadGate) {
61+
if utilfeature.DefaultFeatureGate.Enabled(features.WorkloadSpread) {
5662
allowed, reason, err = h.workloadSpreadValidatingPod(ctx, req)
5763
if !allowed || err != nil {
5864
return

‎test/e2e/apps/containerrecreate.go

+25-5
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,11 @@ var _ = SIGDescribe("ContainerRecreateRequest", func() {
102102
return crr.Status.Phase
103103
}, 70*time.Second, time.Second).Should(gomega.Equal(appsv1alpha1.ContainerRecreateRequestCompleted))
104104
gomega.Expect(crr.Status.CompletionTime).ShouldNot(gomega.BeNil())
105-
gomega.Expect(crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]).Should(gomega.Equal(""))
105+
gomega.Eventually(func() string {
106+
crr, err = tester.GetCRR(crr.Name)
107+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
108+
return crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]
109+
}, 5*time.Second, 1*time.Second).Should(gomega.Equal(""))
106110
gomega.Expect(crr.Status.ContainerRecreateStates).Should(gomega.Equal([]appsv1alpha1.ContainerRecreateRequestContainerRecreateState{{Name: "app", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded}}))
107111

108112
ginkgo.By("Check Pod containers recreated and started for minStartedSeconds")
@@ -145,7 +149,11 @@ var _ = SIGDescribe("ContainerRecreateRequest", func() {
145149
return crr.Status.Phase
146150
}, 60*time.Second, 3*time.Second).Should(gomega.Equal(appsv1alpha1.ContainerRecreateRequestCompleted))
147151
gomega.Expect(crr.Status.CompletionTime).ShouldNot(gomega.BeNil())
148-
gomega.Expect(crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]).Should(gomega.Equal(""))
152+
gomega.Eventually(func() string {
153+
crr, err = tester.GetCRR(crr.Name)
154+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
155+
return crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]
156+
}, 5*time.Second, 1*time.Second).Should(gomega.Equal(""))
149157
gomega.Expect(crr.Status.ContainerRecreateStates).Should(gomega.Equal([]appsv1alpha1.ContainerRecreateRequestContainerRecreateState{
150158
{Name: "app", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded},
151159
{Name: "sidecar", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded},
@@ -256,7 +264,11 @@ var _ = SIGDescribe("ContainerRecreateRequest", func() {
256264
return crr.Status.Phase
257265
}, 60*time.Second, 3*time.Second).Should(gomega.Equal(appsv1alpha1.ContainerRecreateRequestCompleted))
258266
gomega.Expect(crr.Status.CompletionTime).ShouldNot(gomega.BeNil())
259-
gomega.Expect(crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]).Should(gomega.Equal(""))
267+
gomega.Eventually(func() string {
268+
crr, err = tester.GetCRR(crr.Name)
269+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
270+
return crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]
271+
}, 5*time.Second, 1*time.Second).Should(gomega.Equal(""))
260272
gomega.Expect(crr.Status.ContainerRecreateStates).Should(gomega.Equal([]appsv1alpha1.ContainerRecreateRequestContainerRecreateState{
261273
{Name: "app", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded},
262274
{Name: "sidecar", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded},
@@ -320,7 +332,11 @@ var _ = SIGDescribe("ContainerRecreateRequest", func() {
320332
return crr.Status.Phase
321333
}, 60*time.Second, 3*time.Second).Should(gomega.Equal(appsv1alpha1.ContainerRecreateRequestCompleted))
322334
gomega.Expect(crr.Status.CompletionTime).ShouldNot(gomega.BeNil())
323-
gomega.Expect(crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]).Should(gomega.Equal(""))
335+
gomega.Eventually(func() string {
336+
crr, err = tester.GetCRR(crr.Name)
337+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
338+
return crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]
339+
}, 5*time.Second, 1*time.Second).Should(gomega.Equal(""))
324340
gomega.Expect(crr.Status.ContainerRecreateStates).Should(gomega.Equal([]appsv1alpha1.ContainerRecreateRequestContainerRecreateState{
325341
{Name: "app", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded},
326342
{Name: "sidecar", Phase: appsv1alpha1.ContainerRecreateRequestSucceeded},
@@ -366,7 +382,11 @@ var _ = SIGDescribe("ContainerRecreateRequest", func() {
366382
return crr.Status.Phase
367383
}, 5*time.Second, 1*time.Second).Should(gomega.Equal(appsv1alpha1.ContainerRecreateRequestCompleted))
368384
gomega.Expect(crr.Status.CompletionTime).ShouldNot(gomega.BeNil())
369-
gomega.Expect(crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]).Should(gomega.Equal(""))
385+
gomega.Eventually(func() string {
386+
crr, err = tester.GetCRR(crr.Name)
387+
gomega.Expect(err).NotTo(gomega.HaveOccurred())
388+
return crr.Labels[appsv1alpha1.ContainerRecreateRequestActiveKey]
389+
}, 5*time.Second, 1*time.Second).Should(gomega.Equal(""))
370390
gomega.Expect(crr.Status.ContainerRecreateStates).Should(gomega.Equal([]appsv1alpha1.ContainerRecreateRequestContainerRecreateState{
371391
{Name: "app", Phase: appsv1alpha1.ContainerRecreateRequestPending},
372392
{Name: "sidecar", Phase: appsv1alpha1.ContainerRecreateRequestPending},

0 commit comments

Comments
 (0)