[FG:InPlacePodVerticalScaling] Fixed the apiserver panic issue that occurred when adding a container during pod updates in the InPlacePodVerticalScaling scenario. #127291

zhifei92 · 2024-09-11T08:25:58Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Before upgrading a pod, the kube-apiserver validates the pod using strategy.ValidateUpdate. Adding or removing containers from the pod is deemed invalid. However, strategy.PrepareForUpdate(MarkPodProposedForResize is called) is invoked prior to strategy.ValidateUpdate, causing a panic when InPlacePodVerticalScaling is enabled.

Which issue(s) this PR fixes:

Fixes #127282

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

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

k8s-ci-robot · 2024-09-11T08:26:09Z

Hi @zhifei92. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

zhifei92 · 2024-09-11T08:45:29Z

/sig api-machinery

pkg/api/pod/util.go

bouaouda-achraf · 2024-09-11T13:16:45Z

Should we consider adding a test case related to this issue? It could help catch it during a pipeline stage.

seans3 · 2024-09-12T20:10:42Z

/triage accepted
/ok-to-test

zhifei92 · 2024-09-13T07:33:38Z

/retest-required

zhifei92 · 2024-09-13T08:54:24Z

/retest-required

pkg/api/pod/util_test.go

tallclair · 2024-09-19T20:21:07Z

/assign

k8s-ci-robot · 2024-10-14T22:54:06Z

LGTM label has been added.

Git tree hash: f3c6e80ae7a469cb624dffd282b4a95c1bfb7c53

tallclair · 2024-10-22T23:37:42Z

/milestone v1.32
/priority important-soon

knabben · 2024-10-23T16:47:17Z

Hello @tallclair @zhifei92
The code freeze is starting 02:00 UTC Friday November 8th 2024 (about 2 weeks from now), and while there is still time, we want to ensure that each PR has a chance to be merged on time. Just a gently reminder of the date since PR is progressing.
Thank you!

zhifei92 · 2024-10-28T08:59:26Z

@tallclair Hi, can this PR be merged now?

pkg/api/pod/util_test.go

tallclair · 2024-10-28T18:04:18Z

Please add the comment to the validation, and fix the test names.

add unit test refactor: Merge the test cases into TestMarkPodProposedForResize. chore: Add the comment and fix the test names

zhifei92 · 2024-10-29T03:26:40Z

/test pull-kubernetes-e2e-kind

tallclair · 2024-10-29T23:28:58Z

/lgtm
/approve

k8s-ci-robot · 2024-10-29T23:29:04Z

LGTM label has been added.

Git tree hash: 574aee4a24ff1b959c678607c156af9f3e91248b

tallclair · 2024-10-29T23:30:44Z

/assign @thockin
for api approval

thockin

Thanks!

/lgtm
/approve

k8s-ci-robot · 2024-10-29T23:37:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tallclair, thockin, zhifei92

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/api/OWNERS~~ [thockin]

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

zhifei92 · 2024-10-30T01:40:45Z

@tallclair Hi, this issue was found in v1.31, so does this PR need to be cherry-picked to v1.31?

tallclair · 2024-10-30T15:36:55Z

I don't think we usually backport fixes for alpha features, at least not for corner cases like this.

zhifei92 · 2024-10-31T01:52:48Z

I don't think we usually backport fixes for alpha features, at least not for corner cases like this.

get

k8s-ci-robot requested review from smarterclayton and thockin September 11, 2024 08:26

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 11, 2024

k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 11, 2024

bouaouda-achraf reviewed Sep 11, 2024

View reviewed changes

pkg/api/pod/util.go Show resolved Hide resolved

tallclair reviewed Sep 19, 2024

View reviewed changes

pkg/api/pod/util_test.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned tallclair Sep 19, 2024

k8s-ci-robot added this to the v1.32 milestone Oct 22, 2024

k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 22, 2024

tallclair mentioned this pull request Oct 23, 2024

[FG:InPlacePodVerticalScaling] Implement resize for sidecar containers #128272

Closed

tallclair reviewed Oct 28, 2024

View reviewed changes

pkg/api/pod/util_test.go Outdated Show resolved Hide resolved

tallclair reviewed Oct 28, 2024

View reviewed changes

pkg/api/pod/util_test.go Outdated Show resolved Hide resolved

Fix the apiserver panic caused by adding a container when updating a pod

5c01709

add unit test refactor: Merge the test cases into TestMarkPodProposedForResize. chore: Add the comment and fix the test names

zhifei92 force-pushed the fix-apiserver-unexpected-panic branch from c2e043a to 5c01709 Compare October 29, 2024 02:53

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 29, 2024

k8s-ci-robot requested a review from tallclair October 29, 2024 02:53

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 29, 2024

k8s-ci-robot assigned thockin Oct 29, 2024

thockin reviewed Oct 29, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2024

k8s-ci-robot merged commit d702d26 into kubernetes:master Oct 30, 2024
14 checks passed

zhifei92 mentioned this pull request Dec 20, 2024

REQUEST: New membership for zhifei92 kubernetes/org#5317

Closed

11 tasks

tallclair mentioned this pull request Mar 21, 2025

In-Place Update of Pod Resources kubernetes/enhancements#1287

Open

95 tasks

[FG:InPlacePodVerticalScaling] Fixed the apiserver panic issue that occurred when adding a container during pod updates in the InPlacePodVerticalScaling scenario. #127291

[FG:InPlacePodVerticalScaling] Fixed the apiserver panic issue that occurred when adding a container during pod updates in the InPlacePodVerticalScaling scenario. #127291

Uh oh!

Conversation

zhifei92 commented Sep 11, 2024 • 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 Sep 11, 2024

Uh oh!

zhifei92 commented Sep 11, 2024

Uh oh!

Uh oh!

bouaouda-achraf commented Sep 11, 2024

Uh oh!

seans3 commented Sep 12, 2024

Uh oh!

zhifei92 commented Sep 13, 2024

Uh oh!

zhifei92 commented Sep 13, 2024

Uh oh!

Uh oh!

tallclair commented Sep 19, 2024

Uh oh!

k8s-ci-robot commented Oct 14, 2024

Uh oh!

tallclair commented Oct 22, 2024

Uh oh!

knabben commented Oct 23, 2024

Uh oh!

zhifei92 commented Oct 28, 2024

Uh oh!

Uh oh!

Uh oh!

tallclair commented Oct 28, 2024

Uh oh!

zhifei92 commented Oct 29, 2024

Uh oh!

tallclair commented Oct 29, 2024

Uh oh!

k8s-ci-robot commented Oct 29, 2024

Uh oh!

tallclair commented Oct 29, 2024

Uh oh!

thockin left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Oct 29, 2024

Uh oh!

Uh oh!

zhifei92 commented Oct 30, 2024

Uh oh!

tallclair commented Oct 30, 2024

Uh oh!

zhifei92 commented Oct 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zhifei92 commented Sep 11, 2024 •

edited

Loading