Skip to content

Commit 3516bc6

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#122456 from AxeZhan/beta3960
[KEP 3960]: graduate PodLifecycleSleepAction to beta
2 parents 64386c5 + c74ec3d commit 3516bc6

File tree

8 files changed

+444
-37
lines changed

8 files changed

+444
-37
lines changed

‎pkg/api/pod/util.go‎

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -594,39 +594,56 @@ func dropDisabledFields(
594594
// For other types of containers, validateContainers will handle them.
595595
}
596596

597-
if !utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepAction) && !podLifecycleSleepActionInUse(oldPodSpec) {
598-
for i := range podSpec.Containers {
599-
if podSpec.Containers[i].Lifecycle == nil {
600-
continue
601-
}
602-
if podSpec.Containers[i].Lifecycle.PreStop != nil {
603-
podSpec.Containers[i].Lifecycle.PreStop.Sleep = nil
604-
}
605-
if podSpec.Containers[i].Lifecycle.PostStart != nil {
606-
podSpec.Containers[i].Lifecycle.PostStart.Sleep = nil
597+
dropPodLifecycleSleepAction(podSpec, oldPodSpec)
598+
}
599+
600+
func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) {
601+
if utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepAction) || podLifecycleSleepActionInUse(oldPodSpec) {
602+
return
603+
}
604+
605+
adjustLifecycle := func(lifecycle *api.Lifecycle) {
606+
if lifecycle.PreStop != nil && lifecycle.PreStop.Sleep != nil {
607+
lifecycle.PreStop.Sleep = nil
608+
if lifecycle.PreStop.Exec == nil && lifecycle.PreStop.HTTPGet == nil && lifecycle.PreStop.TCPSocket == nil {
609+
lifecycle.PreStop = nil
607610
}
608611
}
609-
for i := range podSpec.InitContainers {
610-
if podSpec.InitContainers[i].Lifecycle == nil {
611-
continue
612-
}
613-
if podSpec.InitContainers[i].Lifecycle.PreStop != nil {
614-
podSpec.InitContainers[i].Lifecycle.PreStop.Sleep = nil
615-
}
616-
if podSpec.InitContainers[i].Lifecycle.PostStart != nil {
617-
podSpec.InitContainers[i].Lifecycle.PostStart.Sleep = nil
612+
if lifecycle.PostStart != nil && lifecycle.PostStart.Sleep != nil {
613+
lifecycle.PostStart.Sleep = nil
614+
if lifecycle.PostStart.Exec == nil && lifecycle.PostStart.HTTPGet == nil && lifecycle.PostStart.TCPSocket == nil {
615+
lifecycle.PostStart = nil
618616
}
619617
}
620-
for i := range podSpec.EphemeralContainers {
621-
if podSpec.EphemeralContainers[i].Lifecycle == nil {
622-
continue
623-
}
624-
if podSpec.EphemeralContainers[i].Lifecycle.PreStop != nil {
625-
podSpec.EphemeralContainers[i].Lifecycle.PreStop.Sleep = nil
626-
}
627-
if podSpec.EphemeralContainers[i].Lifecycle.PostStart != nil {
628-
podSpec.EphemeralContainers[i].Lifecycle.PostStart.Sleep = nil
629-
}
618+
}
619+
620+
for i := range podSpec.Containers {
621+
if podSpec.Containers[i].Lifecycle == nil {
622+
continue
623+
}
624+
adjustLifecycle(podSpec.Containers[i].Lifecycle)
625+
if podSpec.Containers[i].Lifecycle.PreStop == nil && podSpec.Containers[i].Lifecycle.PostStart == nil {
626+
podSpec.Containers[i].Lifecycle = nil
627+
}
628+
}
629+
630+
for i := range podSpec.InitContainers {
631+
if podSpec.InitContainers[i].Lifecycle == nil {
632+
continue
633+
}
634+
adjustLifecycle(podSpec.InitContainers[i].Lifecycle)
635+
if podSpec.InitContainers[i].Lifecycle.PreStop == nil && podSpec.InitContainers[i].Lifecycle.PostStart == nil {
636+
podSpec.InitContainers[i].Lifecycle = nil
637+
}
638+
}
639+
640+
for i := range podSpec.EphemeralContainers {
641+
if podSpec.EphemeralContainers[i].Lifecycle == nil {
642+
continue
643+
}
644+
adjustLifecycle(podSpec.EphemeralContainers[i].Lifecycle)
645+
if podSpec.EphemeralContainers[i].Lifecycle.PreStop == nil && podSpec.EphemeralContainers[i].Lifecycle.PostStart == nil {
646+
podSpec.EphemeralContainers[i].Lifecycle = nil
630647
}
631648
}
632649
}

‎pkg/api/pod/util_test.go‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3472,3 +3472,236 @@ func TestDropClusterTrustBundleProjectedVolumes(t *testing.T) {
34723472
})
34733473
}
34743474
}
3475+
3476+
func TestDropPodLifecycleSleepAction(t *testing.T) {
3477+
makeSleepHandler := func() *api.LifecycleHandler {
3478+
return &api.LifecycleHandler{
3479+
Sleep: &api.SleepAction{Seconds: 1},
3480+
}
3481+
}
3482+
3483+
makeExecHandler := func() *api.LifecycleHandler {
3484+
return &api.LifecycleHandler{
3485+
Exec: &api.ExecAction{Command: []string{"foo"}},
3486+
}
3487+
}
3488+
3489+
makeHTTPGetHandler := func() *api.LifecycleHandler {
3490+
return &api.LifecycleHandler{
3491+
HTTPGet: &api.HTTPGetAction{Host: "foo"},
3492+
}
3493+
}
3494+
3495+
makeContainer := func(preStop, postStart *api.LifecycleHandler) api.Container {
3496+
container := api.Container{Name: "foo"}
3497+
if preStop != nil || postStart != nil {
3498+
container.Lifecycle = &api.Lifecycle{
3499+
PostStart: postStart,
3500+
PreStop: preStop,
3501+
}
3502+
}
3503+
return container
3504+
}
3505+
3506+
makeEphemeralContainer := func(preStop, postStart *api.LifecycleHandler) api.EphemeralContainer {
3507+
container := api.EphemeralContainer{
3508+
EphemeralContainerCommon: api.EphemeralContainerCommon{Name: "foo"},
3509+
}
3510+
if preStop != nil || postStart != nil {
3511+
container.Lifecycle = &api.Lifecycle{
3512+
PostStart: postStart,
3513+
PreStop: preStop,
3514+
}
3515+
}
3516+
return container
3517+
}
3518+
3519+
makePod := func(containers []api.Container, initContainers []api.Container, ephemeralContainers []api.EphemeralContainer) *api.PodSpec {
3520+
return &api.PodSpec{
3521+
Containers: containers,
3522+
InitContainers: initContainers,
3523+
EphemeralContainers: ephemeralContainers,
3524+
}
3525+
}
3526+
3527+
testCases := []struct {
3528+
gateEnabled bool
3529+
oldLifecycleHandler *api.LifecycleHandler
3530+
newLifecycleHandler *api.LifecycleHandler
3531+
expectLifecycleHandler *api.LifecycleHandler
3532+
}{
3533+
// nil -> nil
3534+
{
3535+
gateEnabled: false,
3536+
oldLifecycleHandler: nil,
3537+
newLifecycleHandler: nil,
3538+
expectLifecycleHandler: nil,
3539+
},
3540+
{
3541+
gateEnabled: true,
3542+
oldLifecycleHandler: nil,
3543+
newLifecycleHandler: nil,
3544+
expectLifecycleHandler: nil,
3545+
},
3546+
// nil -> exec
3547+
{
3548+
gateEnabled: false,
3549+
oldLifecycleHandler: nil,
3550+
newLifecycleHandler: makeExecHandler(),
3551+
expectLifecycleHandler: makeExecHandler(),
3552+
},
3553+
{
3554+
gateEnabled: true,
3555+
oldLifecycleHandler: nil,
3556+
newLifecycleHandler: makeExecHandler(),
3557+
expectLifecycleHandler: makeExecHandler(),
3558+
},
3559+
// nil -> sleep
3560+
{
3561+
gateEnabled: false,
3562+
oldLifecycleHandler: nil,
3563+
newLifecycleHandler: makeSleepHandler(),
3564+
expectLifecycleHandler: nil,
3565+
},
3566+
{
3567+
gateEnabled: true,
3568+
oldLifecycleHandler: nil,
3569+
newLifecycleHandler: makeSleepHandler(),
3570+
expectLifecycleHandler: makeSleepHandler(),
3571+
},
3572+
// exec -> exec
3573+
{
3574+
gateEnabled: false,
3575+
oldLifecycleHandler: makeExecHandler(),
3576+
newLifecycleHandler: makeExecHandler(),
3577+
expectLifecycleHandler: makeExecHandler(),
3578+
},
3579+
{
3580+
gateEnabled: true,
3581+
oldLifecycleHandler: makeExecHandler(),
3582+
newLifecycleHandler: makeExecHandler(),
3583+
expectLifecycleHandler: makeExecHandler(),
3584+
},
3585+
// exec -> http
3586+
{
3587+
gateEnabled: false,
3588+
oldLifecycleHandler: makeExecHandler(),
3589+
newLifecycleHandler: makeHTTPGetHandler(),
3590+
expectLifecycleHandler: makeHTTPGetHandler(),
3591+
},
3592+
{
3593+
gateEnabled: true,
3594+
oldLifecycleHandler: makeExecHandler(),
3595+
newLifecycleHandler: makeHTTPGetHandler(),
3596+
expectLifecycleHandler: makeHTTPGetHandler(),
3597+
},
3598+
// exec -> sleep
3599+
{
3600+
gateEnabled: false,
3601+
oldLifecycleHandler: makeExecHandler(),
3602+
newLifecycleHandler: makeSleepHandler(),
3603+
expectLifecycleHandler: nil,
3604+
},
3605+
{
3606+
gateEnabled: true,
3607+
oldLifecycleHandler: makeExecHandler(),
3608+
newLifecycleHandler: makeSleepHandler(),
3609+
expectLifecycleHandler: makeSleepHandler(),
3610+
},
3611+
// sleep -> exec
3612+
{
3613+
gateEnabled: false,
3614+
oldLifecycleHandler: makeSleepHandler(),
3615+
newLifecycleHandler: makeExecHandler(),
3616+
expectLifecycleHandler: makeExecHandler(),
3617+
},
3618+
{
3619+
gateEnabled: true,
3620+
oldLifecycleHandler: makeSleepHandler(),
3621+
newLifecycleHandler: makeExecHandler(),
3622+
expectLifecycleHandler: makeExecHandler(),
3623+
},
3624+
// sleep -> sleep
3625+
{
3626+
gateEnabled: false,
3627+
oldLifecycleHandler: makeSleepHandler(),
3628+
newLifecycleHandler: makeSleepHandler(),
3629+
expectLifecycleHandler: makeSleepHandler(),
3630+
},
3631+
{
3632+
gateEnabled: true,
3633+
oldLifecycleHandler: makeSleepHandler(),
3634+
newLifecycleHandler: makeSleepHandler(),
3635+
expectLifecycleHandler: makeSleepHandler(),
3636+
},
3637+
}
3638+
3639+
for i, tc := range testCases {
3640+
t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) {
3641+
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodLifecycleSleepAction, tc.gateEnabled)()
3642+
3643+
// preStop
3644+
// container
3645+
{
3646+
oldPod := makePod([]api.Container{makeContainer(tc.oldLifecycleHandler.DeepCopy(), nil)}, nil, nil)
3647+
newPod := makePod([]api.Container{makeContainer(tc.newLifecycleHandler.DeepCopy(), nil)}, nil, nil)
3648+
expectPod := makePod([]api.Container{makeContainer(tc.expectLifecycleHandler.DeepCopy(), nil)}, nil, nil)
3649+
dropDisabledFields(newPod, nil, oldPod, nil)
3650+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3651+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3652+
}
3653+
}
3654+
// InitContainer
3655+
{
3656+
oldPod := makePod(nil, []api.Container{makeContainer(tc.oldLifecycleHandler.DeepCopy(), nil)}, nil)
3657+
newPod := makePod(nil, []api.Container{makeContainer(tc.newLifecycleHandler.DeepCopy(), nil)}, nil)
3658+
expectPod := makePod(nil, []api.Container{makeContainer(tc.expectLifecycleHandler.DeepCopy(), nil)}, nil)
3659+
dropDisabledFields(newPod, nil, oldPod, nil)
3660+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3661+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3662+
}
3663+
}
3664+
// EphemeralContainer
3665+
{
3666+
oldPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.oldLifecycleHandler.DeepCopy(), nil)})
3667+
newPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.newLifecycleHandler.DeepCopy(), nil)})
3668+
expectPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.expectLifecycleHandler.DeepCopy(), nil)})
3669+
dropDisabledFields(newPod, nil, oldPod, nil)
3670+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3671+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3672+
}
3673+
}
3674+
// postStart
3675+
// container
3676+
{
3677+
oldPod := makePod([]api.Container{makeContainer(nil, tc.oldLifecycleHandler.DeepCopy())}, nil, nil)
3678+
newPod := makePod([]api.Container{makeContainer(nil, tc.newLifecycleHandler.DeepCopy())}, nil, nil)
3679+
expectPod := makePod([]api.Container{makeContainer(nil, tc.expectLifecycleHandler.DeepCopy())}, nil, nil)
3680+
dropDisabledFields(newPod, nil, oldPod, nil)
3681+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3682+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3683+
}
3684+
}
3685+
// InitContainer
3686+
{
3687+
oldPod := makePod(nil, []api.Container{makeContainer(nil, tc.oldLifecycleHandler.DeepCopy())}, nil)
3688+
newPod := makePod(nil, []api.Container{makeContainer(nil, tc.newLifecycleHandler.DeepCopy())}, nil)
3689+
expectPod := makePod(nil, []api.Container{makeContainer(nil, tc.expectLifecycleHandler.DeepCopy())}, nil)
3690+
dropDisabledFields(newPod, nil, oldPod, nil)
3691+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3692+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3693+
}
3694+
}
3695+
// EphemeralContainer
3696+
{
3697+
oldPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(nil, tc.oldLifecycleHandler.DeepCopy())})
3698+
newPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(nil, tc.newLifecycleHandler.DeepCopy())})
3699+
expectPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(nil, tc.expectLifecycleHandler.DeepCopy())})
3700+
dropDisabledFields(newPod, nil, oldPod, nil)
3701+
if diff := cmp.Diff(expectPod, newPod); diff != "" {
3702+
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
3703+
}
3704+
}
3705+
})
3706+
}
3707+
}

‎pkg/features/kube_features.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,7 @@ const (
609609
// owner: @AxeZhan
610610
// kep: http://kep.k8s.io/3960
611611
// alpha: v1.29
612+
// beta: v1.30
612613
//
613614
// Enables SleepAction in container lifecycle hooks
614615
PodLifecycleSleepAction featuregate.Feature = "PodLifecycleSleepAction"
@@ -1078,7 +1079,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
10781079

10791080
PodHostIPs: {Default: true, PreRelease: featuregate.Beta},
10801081

1081-
PodLifecycleSleepAction: {Default: false, PreRelease: featuregate.Alpha},
1082+
PodLifecycleSleepAction: {Default: true, PreRelease: featuregate.Beta},
10821083

10831084
PodSchedulingReadiness: {Default: true, PreRelease: featuregate.Beta},
10841085

‎pkg/kubelet/lifecycle/handlers.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ func (hr *handlerRunner) runSleepHandler(ctx context.Context, seconds int64) err
133133
select {
134134
case <-ctx.Done():
135135
// unexpected termination
136+
metrics.LifecycleHandlerSleepTerminated.Inc()
136137
return fmt.Errorf("container terminated before sleep hook finished")
137138
case <-c:
138139
return nil

‎pkg/kubelet/metrics/metrics.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,15 @@ var (
859859
},
860860
[]string{"image_size_in_bytes"},
861861
)
862+
863+
LifecycleHandlerSleepTerminated = metrics.NewCounter(
864+
&metrics.CounterOpts{
865+
Subsystem: KubeletSubsystem,
866+
Name: "sleep_action_terminated_early_total",
867+
Help: "The number of times lifecycle sleep handler got terminated before it finishes",
868+
StabilityLevel: metrics.ALPHA,
869+
},
870+
)
862871
)
863872

864873
var registerMetrics sync.Once
@@ -942,6 +951,7 @@ func Register(collectors ...metrics.StableCollector) {
942951
}
943952

944953
legacyregistry.MustRegister(LifecycleHandlerHTTPFallbacks)
954+
legacyregistry.MustRegister(LifecycleHandlerSleepTerminated)
945955
})
946956
}
947957

0 commit comments

Comments
 (0)