Allow all probes and lifecycle for restartable init containers #119168

gjkim42 · 2023-07-08T01:24:36Z

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

This is a follow-up PR for #116429

Allow restartable init containers to have readiness/liveness probes and lifecycle
Generate ContainersReadyCondition with restartable init containers
Add probes and lifecycle hooks e2e tests for restartable init containers

Which issue(s) this PR fixes:

Fixes #
depends on #119179
ref: #115934

Special notes for your reviewer:

Does this PR introduce a user-facing change?

none

wrote "none", as we already added a release note in #116429.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/753-sidecar-containers

k8s-ci-robot · 2023-07-08T01:24:44Z

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

gjkim42 · 2023-07-08T01:24:54Z

/sig node

gjkim42 · 2023-07-09T04:53:05Z

/test pull-kubernetes-node-e2e-containerd-sidecar-containers

gjkim42 · 2023-07-09T05:23:38Z

/test pull-kubernetes-node-e2e-containerd-sidecar-containers

gjkim42 · 2023-07-09T09:18:14Z

/test pull-kubernetes-node-e2e-containerd-sidecar-containers

gjkim42 · 2023-07-17T21:30:27Z

/test pull-kubernetes-node-e2e-containerd-sidecar-containers
/test pull-kubernetes-e2e-gce-cos-alpha-features

thockin · 2023-07-17T21:35:23Z

I have not been tracking this one - is it hoping to merge for 1.28?

gjkim42 · 2023-07-17T21:36:47Z

I have not been tracking this one - is it hoping to merge for 1.28?

Yes, it is targeted at 1.28.

thockin

/approve

for validation

jpbetz · 2023-07-17T21:49:30Z

pkg/kubelet/kuberuntime/kuberuntime_container.go

nit: Would you add a comment explaining this break as well? I spent a few minutes looking at the two above if statements and this break and it wasn't immediately obvious what the rationale is. It's probably right, I just wasn't able to figure it out just from looking at the code..

nit: Would you add a comment explaining this break as well? I spent a few minutes looking at the two above if statements and this break and it wasn't immediately obvious what the rationale is. It's probably right, I just wasn't able to figure it out just from looking at the code..

I think the current code is slightly wrong...

A restartable init container doesn't have to take into account its liveness probe when it determines to start the next init container.

will fix it soon.

fixed kubelet to not take into account the container's livenessProbe when it decides to start the next init container.

https://github.com/kubernetes/kubernetes/compare/8ca77dc4bef1be820137a667ebe082e3721cf7c3..d029f0ef0603256c89617c1b4e7898d2f2f87951

gjkim42 · 2023-07-17T22:55:10Z

/test pull-kubernetes-node-e2e-containerd-sidecar-containers
/test pull-kubernetes-e2e-gce-cos-alpha-features

…ners

gjkim42 · 2023-07-17T23:12:58Z

/test pull-kubernetes-node-e2e-containerd-sidecar-containers
/test pull-kubernetes-e2e-gce-cos-alpha-features

SergeyKanzhelev

/lgtm
/approve

k8s-ci-robot · 2023-07-17T23:15:17Z

LGTM label has been added.

Git tree hash: 7e8554f6ad5af9ef45e4d35c55c68cc9a3681dcc

k8s-ci-robot · 2023-07-17T23:15:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gjkim42, mrunalp, SergeyKanzhelev, thockin, Tusenka

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/apis/OWNERS~~ [thockin]
~~pkg/kubelet/OWNERS~~ [mrunalp,thockin]
~~test/e2e/common/OWNERS~~ [SergeyKanzhelev,thockin]
~~test/e2e_node/OWNERS~~ [SergeyKanzhelev,mrunalp,thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

liggitt · 2023-07-18T19:38:17Z

test/e2e/common/node/lifecycle_hook.go

 	})
 })

+var _ = SIGDescribe("[NodeAlphaFeature:SidecarContainers][Feature:SidecarContainers] Restartable Init Container Lifecycle Hook", func() {


these tests have been flaking, with at least one of them failing in about 50% of the runs since they merged:

https://testgrid.k8s.io/presubmits-kubernetes-nonblocking#pull-kubernetes-e2e-gce-cos-alpha-features&include-filter-by-regex=NodeAlphaFeature:SidecarContainers|Test&width=20&show-stale-tests=

SergeyKanzhelev · 2023-07-18T20:53:16Z

sidecar-specific job pull-kubernetes-node-e2e-containerd-sidecar-containers passes and green. Maybe some interference between jobs

k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 8, 2023

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. area/kubelet sig/apps Categorizes an issue or PR as relevant to SIG Apps. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 8, 2023

k8s-ci-robot requested review from bart0sh and mikedanese July 8, 2023 01:25

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch from ebd9e26 to eaa563a Compare July 8, 2023 01:32

Tusenka approved these changes Jul 8, 2023

View reviewed changes

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch from eaa563a to aeeec82 Compare July 9, 2023 04:52

k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jul 9, 2023

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch from aeeec82 to 0675c82 Compare July 9, 2023 05:23

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch from 0675c82 to 26a7408 Compare July 9, 2023 09:18

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch 2 times, most recently from d3de630 to 72c8951 Compare July 11, 2023 15:04

k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 11, 2023

thockin reviewed Jul 17, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2023

jpbetz reviewed Jul 17, 2023

View reviewed changes

gjkim42 added 3 commits July 18, 2023 07:54

Add validateStartupProbe

8b20dbd

Allow restartable init containers to have readinessProbe

2c8b374

Allow restartable init containers to have livenessProbe

7ef2d67

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch from 8ca77dc to 7e670fd Compare July 17, 2023 22:55

gjkim42 added 4 commits July 18, 2023 08:12

Allow restartable init containers to have lifecycle

3bf2826

Generate containers ready condition including restartable init contai…

9d6c103

…ners

Add restartable init container probe e2e tests

f17e911

Add restartable init container lifecycle hook e2e tests

d029f0e

gjkim42 force-pushed the sidecar-allow-probes-and-lifecycle-hooks branch from 7e670fd to d029f0e Compare July 17, 2023 23:12

SergeyKanzhelev approved these changes Jul 17, 2023

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 17, 2023

k8s-ci-robot merged commit d17f3ba into kubernetes:master Jul 18, 2023

k8s-ci-robot added this to the v1.28 milestone Jul 18, 2023

gjkim42 deleted the sidecar-allow-probes-and-lifecycle-hooks branch July 18, 2023 01:13

gjkim42 mentioned this pull request Jul 18, 2023

Topology manager should consider restartable init containers #119407

Closed

liggitt reviewed Jul 18, 2023

View reviewed changes

gjkim42 mentioned this pull request Jul 19, 2023

Bump timeout for pull-kubernetes-e2e-gce-cos-alpha-features kubernetes/test-infra#30127

Closed

pacoxu mentioned this pull request Jul 20, 2023

E2eNode [NodeFeature:SystemNodeCriticalPod] when create a system-node-critical pod should not be evicted upon DiskPressure #119457

Closed

SergeyKanzhelev mentioned this pull request Aug 5, 2024

adding SergeyKanzhelev as SIG Node approver #126551

Merged

25 tasks

Allow all probes and lifecycle for restartable init containers #119168

Allow all probes and lifecycle for restartable init containers #119168

Uh oh!

Conversation

gjkim42 commented Jul 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Uh oh!

k8s-ci-robot commented Jul 8, 2023

Uh oh!

gjkim42 commented Jul 8, 2023

Uh oh!

gjkim42 commented Jul 9, 2023

Uh oh!

gjkim42 commented Jul 9, 2023

Uh oh!

gjkim42 commented Jul 9, 2023

Uh oh!

gjkim42 commented Jul 17, 2023

Uh oh!

thockin commented Jul 17, 2023

Uh oh!

gjkim42 commented Jul 17, 2023

Uh oh!

thockin left a comment

Choose a reason for hiding this comment

Uh oh!

jpbetz Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

gjkim42 Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

gjkim42 Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

gjkim42 commented Jul 17, 2023

Uh oh!

gjkim42 commented Jul 17, 2023

Uh oh!

SergeyKanzhelev left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jul 17, 2023

Uh oh!

k8s-ci-robot commented Jul 17, 2023

Uh oh!

liggitt Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

SergeyKanzhelev commented Jul 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

gjkim42 commented Jul 8, 2023 •

edited

Loading