Skip to content

Commit

Permalink
Merge pull request #122456 from AxeZhan/beta3960
Browse files Browse the repository at this point in the history 
[KEP 3960]: graduate PodLifecycleSleepAction to beta
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Feb 19, 2024
2 parents 64386c5 + c74ec3d commit 3516bc6
Show file tree
Hide file tree
Showing 8 changed files with 444 additions and 37 deletions.
75 changes: 46 additions & 29 deletions pkg/api/pod/util.go
View file
Expand Up @@ -594,39 +594,56 @@ func dropDisabledFields(
// For other types of containers, validateContainers will handle them.
}

if !utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepAction) && !podLifecycleSleepActionInUse(oldPodSpec) {
for i := range podSpec.Containers {
if podSpec.Containers[i].Lifecycle == nil {
continue
}
if podSpec.Containers[i].Lifecycle.PreStop != nil {
podSpec.Containers[i].Lifecycle.PreStop.Sleep = nil
}
if podSpec.Containers[i].Lifecycle.PostStart != nil {
podSpec.Containers[i].Lifecycle.PostStart.Sleep = nil
dropPodLifecycleSleepAction(podSpec, oldPodSpec)
}

func dropPodLifecycleSleepAction(podSpec, oldPodSpec *api.PodSpec) {
if utilfeature.DefaultFeatureGate.Enabled(features.PodLifecycleSleepAction) || podLifecycleSleepActionInUse(oldPodSpec) {
return
}

adjustLifecycle := func(lifecycle *api.Lifecycle) {
if lifecycle.PreStop != nil && lifecycle.PreStop.Sleep != nil {
lifecycle.PreStop.Sleep = nil
if lifecycle.PreStop.Exec == nil && lifecycle.PreStop.HTTPGet == nil && lifecycle.PreStop.TCPSocket == nil {
lifecycle.PreStop = nil
}
}
for i := range podSpec.InitContainers {
if podSpec.InitContainers[i].Lifecycle == nil {
continue
}
if podSpec.InitContainers[i].Lifecycle.PreStop != nil {
podSpec.InitContainers[i].Lifecycle.PreStop.Sleep = nil
}
if podSpec.InitContainers[i].Lifecycle.PostStart != nil {
podSpec.InitContainers[i].Lifecycle.PostStart.Sleep = nil
if lifecycle.PostStart != nil && lifecycle.PostStart.Sleep != nil {
lifecycle.PostStart.Sleep = nil
if lifecycle.PostStart.Exec == nil && lifecycle.PostStart.HTTPGet == nil && lifecycle.PostStart.TCPSocket == nil {
lifecycle.PostStart = nil
}
}
for i := range podSpec.EphemeralContainers {
if podSpec.EphemeralContainers[i].Lifecycle == nil {
continue
}
if podSpec.EphemeralContainers[i].Lifecycle.PreStop != nil {
podSpec.EphemeralContainers[i].Lifecycle.PreStop.Sleep = nil
}
if podSpec.EphemeralContainers[i].Lifecycle.PostStart != nil {
podSpec.EphemeralContainers[i].Lifecycle.PostStart.Sleep = nil
}
}

for i := range podSpec.Containers {
if podSpec.Containers[i].Lifecycle == nil {
continue
}
adjustLifecycle(podSpec.Containers[i].Lifecycle)
if podSpec.Containers[i].Lifecycle.PreStop == nil && podSpec.Containers[i].Lifecycle.PostStart == nil {
podSpec.Containers[i].Lifecycle = nil
}
}

for i := range podSpec.InitContainers {
if podSpec.InitContainers[i].Lifecycle == nil {
continue
}
adjustLifecycle(podSpec.InitContainers[i].Lifecycle)
if podSpec.InitContainers[i].Lifecycle.PreStop == nil && podSpec.InitContainers[i].Lifecycle.PostStart == nil {
podSpec.InitContainers[i].Lifecycle = nil
}
}

for i := range podSpec.EphemeralContainers {
if podSpec.EphemeralContainers[i].Lifecycle == nil {
continue
}
adjustLifecycle(podSpec.EphemeralContainers[i].Lifecycle)
if podSpec.EphemeralContainers[i].Lifecycle.PreStop == nil && podSpec.EphemeralContainers[i].Lifecycle.PostStart == nil {
podSpec.EphemeralContainers[i].Lifecycle = nil
}
}
}
Expand Down
233 changes: 233 additions & 0 deletions pkg/api/pod/util_test.go
View file
Expand Up @@ -3472,3 +3472,236 @@ func TestDropClusterTrustBundleProjectedVolumes(t *testing.T) {
})
}
}

func TestDropPodLifecycleSleepAction(t *testing.T) {
makeSleepHandler := func() *api.LifecycleHandler {
return &api.LifecycleHandler{
Sleep: &api.SleepAction{Seconds: 1},
}
}

makeExecHandler := func() *api.LifecycleHandler {
return &api.LifecycleHandler{
Exec: &api.ExecAction{Command: []string{"foo"}},
}
}

makeHTTPGetHandler := func() *api.LifecycleHandler {
return &api.LifecycleHandler{
HTTPGet: &api.HTTPGetAction{Host: "foo"},
}
}

makeContainer := func(preStop, postStart *api.LifecycleHandler) api.Container {
container := api.Container{Name: "foo"}
if preStop != nil || postStart != nil {
container.Lifecycle = &api.Lifecycle{
PostStart: postStart,
PreStop: preStop,
}
}
return container
}

makeEphemeralContainer := func(preStop, postStart *api.LifecycleHandler) api.EphemeralContainer {
container := api.EphemeralContainer{
EphemeralContainerCommon: api.EphemeralContainerCommon{Name: "foo"},
}
if preStop != nil || postStart != nil {
container.Lifecycle = &api.Lifecycle{
PostStart: postStart,
PreStop: preStop,
}
}
return container
}

makePod := func(containers []api.Container, initContainers []api.Container, ephemeralContainers []api.EphemeralContainer) *api.PodSpec {
return &api.PodSpec{
Containers: containers,
InitContainers: initContainers,
EphemeralContainers: ephemeralContainers,
}
}

testCases := []struct {
gateEnabled bool
oldLifecycleHandler *api.LifecycleHandler
newLifecycleHandler *api.LifecycleHandler
expectLifecycleHandler *api.LifecycleHandler
}{
// nil -> nil
{
gateEnabled: false,
oldLifecycleHandler: nil,
newLifecycleHandler: nil,
expectLifecycleHandler: nil,
},
{
gateEnabled: true,
oldLifecycleHandler: nil,
newLifecycleHandler: nil,
expectLifecycleHandler: nil,
},
// nil -> exec
{
gateEnabled: false,
oldLifecycleHandler: nil,
newLifecycleHandler: makeExecHandler(),
expectLifecycleHandler: makeExecHandler(),
},
{
gateEnabled: true,
oldLifecycleHandler: nil,
newLifecycleHandler: makeExecHandler(),
expectLifecycleHandler: makeExecHandler(),
},
// nil -> sleep
{
gateEnabled: false,
oldLifecycleHandler: nil,
newLifecycleHandler: makeSleepHandler(),
expectLifecycleHandler: nil,
},
{
gateEnabled: true,
oldLifecycleHandler: nil,
newLifecycleHandler: makeSleepHandler(),
expectLifecycleHandler: makeSleepHandler(),
},
// exec -> exec
{
gateEnabled: false,
oldLifecycleHandler: makeExecHandler(),
newLifecycleHandler: makeExecHandler(),
expectLifecycleHandler: makeExecHandler(),
},
{
gateEnabled: true,
oldLifecycleHandler: makeExecHandler(),
newLifecycleHandler: makeExecHandler(),
expectLifecycleHandler: makeExecHandler(),
},
// exec -> http
{
gateEnabled: false,
oldLifecycleHandler: makeExecHandler(),
newLifecycleHandler: makeHTTPGetHandler(),
expectLifecycleHandler: makeHTTPGetHandler(),
},
{
gateEnabled: true,
oldLifecycleHandler: makeExecHandler(),
newLifecycleHandler: makeHTTPGetHandler(),
expectLifecycleHandler: makeHTTPGetHandler(),
},
// exec -> sleep
{
gateEnabled: false,
oldLifecycleHandler: makeExecHandler(),
newLifecycleHandler: makeSleepHandler(),
expectLifecycleHandler: nil,
},
{
gateEnabled: true,
oldLifecycleHandler: makeExecHandler(),
newLifecycleHandler: makeSleepHandler(),
expectLifecycleHandler: makeSleepHandler(),
},
// sleep -> exec
{
gateEnabled: false,
oldLifecycleHandler: makeSleepHandler(),
newLifecycleHandler: makeExecHandler(),
expectLifecycleHandler: makeExecHandler(),
},
{
gateEnabled: true,
oldLifecycleHandler: makeSleepHandler(),
newLifecycleHandler: makeExecHandler(),
expectLifecycleHandler: makeExecHandler(),
},
// sleep -> sleep
{
gateEnabled: false,
oldLifecycleHandler: makeSleepHandler(),
newLifecycleHandler: makeSleepHandler(),
expectLifecycleHandler: makeSleepHandler(),
},
{
gateEnabled: true,
oldLifecycleHandler: makeSleepHandler(),
newLifecycleHandler: makeSleepHandler(),
expectLifecycleHandler: makeSleepHandler(),
},
}

for i, tc := range testCases {
t.Run(fmt.Sprintf("test_%d", i), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodLifecycleSleepAction, tc.gateEnabled)()

// preStop
// container
{
oldPod := makePod([]api.Container{makeContainer(tc.oldLifecycleHandler.DeepCopy(), nil)}, nil, nil)
newPod := makePod([]api.Container{makeContainer(tc.newLifecycleHandler.DeepCopy(), nil)}, nil, nil)
expectPod := makePod([]api.Container{makeContainer(tc.expectLifecycleHandler.DeepCopy(), nil)}, nil, nil)
dropDisabledFields(newPod, nil, oldPod, nil)
if diff := cmp.Diff(expectPod, newPod); diff != "" {
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
}
}
// InitContainer
{
oldPod := makePod(nil, []api.Container{makeContainer(tc.oldLifecycleHandler.DeepCopy(), nil)}, nil)
newPod := makePod(nil, []api.Container{makeContainer(tc.newLifecycleHandler.DeepCopy(), nil)}, nil)
expectPod := makePod(nil, []api.Container{makeContainer(tc.expectLifecycleHandler.DeepCopy(), nil)}, nil)
dropDisabledFields(newPod, nil, oldPod, nil)
if diff := cmp.Diff(expectPod, newPod); diff != "" {
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
}
}
// EphemeralContainer
{
oldPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.oldLifecycleHandler.DeepCopy(), nil)})
newPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.newLifecycleHandler.DeepCopy(), nil)})
expectPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(tc.expectLifecycleHandler.DeepCopy(), nil)})
dropDisabledFields(newPod, nil, oldPod, nil)
if diff := cmp.Diff(expectPod, newPod); diff != "" {
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
}
}
// postStart
// container
{
oldPod := makePod([]api.Container{makeContainer(nil, tc.oldLifecycleHandler.DeepCopy())}, nil, nil)
newPod := makePod([]api.Container{makeContainer(nil, tc.newLifecycleHandler.DeepCopy())}, nil, nil)
expectPod := makePod([]api.Container{makeContainer(nil, tc.expectLifecycleHandler.DeepCopy())}, nil, nil)
dropDisabledFields(newPod, nil, oldPod, nil)
if diff := cmp.Diff(expectPod, newPod); diff != "" {
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
}
}
// InitContainer
{
oldPod := makePod(nil, []api.Container{makeContainer(nil, tc.oldLifecycleHandler.DeepCopy())}, nil)
newPod := makePod(nil, []api.Container{makeContainer(nil, tc.newLifecycleHandler.DeepCopy())}, nil)
expectPod := makePod(nil, []api.Container{makeContainer(nil, tc.expectLifecycleHandler.DeepCopy())}, nil)
dropDisabledFields(newPod, nil, oldPod, nil)
if diff := cmp.Diff(expectPod, newPod); diff != "" {
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
}
}
// EphemeralContainer
{
oldPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(nil, tc.oldLifecycleHandler.DeepCopy())})
newPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(nil, tc.newLifecycleHandler.DeepCopy())})
expectPod := makePod(nil, nil, []api.EphemeralContainer{makeEphemeralContainer(nil, tc.expectLifecycleHandler.DeepCopy())})
dropDisabledFields(newPod, nil, oldPod, nil)
if diff := cmp.Diff(expectPod, newPod); diff != "" {
t.Fatalf("Unexpected modification to new pod; diff (-got +want)\n%s", diff)
}
}
})
}
}
3 changes: 2 additions & 1 deletion pkg/features/kube_features.go
View file
Expand Up @@ -609,6 +609,7 @@ const (
// owner: @AxeZhan
// kep: http://kep.k8s.io/3960
// alpha: v1.29
// beta: v1.30
//
// Enables SleepAction in container lifecycle hooks
PodLifecycleSleepAction featuregate.Feature = "PodLifecycleSleepAction"
Expand Down Expand Up @@ -1078,7 +1079,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

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

PodLifecycleSleepAction: {Default: false, PreRelease: featuregate.Alpha},
PodLifecycleSleepAction: {Default: true, PreRelease: featuregate.Beta},

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

Expand Down
1 change: 1 addition & 0 deletions pkg/kubelet/lifecycle/handlers.go
View file
Expand Up @@ -133,6 +133,7 @@ func (hr *handlerRunner) runSleepHandler(ctx context.Context, seconds int64) err
select {
case <-ctx.Done():
// unexpected termination
metrics.LifecycleHandlerSleepTerminated.Inc()
return fmt.Errorf("container terminated before sleep hook finished")
case <-c:
return nil
Expand Down
10 changes: 10 additions & 0 deletions pkg/kubelet/metrics/metrics.go
View file
Expand Up @@ -859,6 +859,15 @@ var (
},
[]string{"image_size_in_bytes"},
)

LifecycleHandlerSleepTerminated = metrics.NewCounter(
&metrics.CounterOpts{
Subsystem: KubeletSubsystem,
Name: "sleep_action_terminated_early_total",
Help: "The number of times lifecycle sleep handler got terminated before it finishes",
StabilityLevel: metrics.ALPHA,
},
)
)

var registerMetrics sync.Once
Expand Down Expand Up @@ -942,6 +951,7 @@ func Register(collectors ...metrics.StableCollector) {
}

legacyregistry.MustRegister(LifecycleHandlerHTTPFallbacks)
legacyregistry.MustRegister(LifecycleHandlerSleepTerminated)
})
}

Expand Down

0 comments on commit 3516bc6

Please sign in to comment.