Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mark pod-infra-container-image flag as deprecated #108045

Merged
merged 1 commit into from
Feb 15, 2022
Merged

Mark pod-infra-container-image flag as deprecated #108045

merged 1 commit into from
Feb 15, 2022

Conversation

hakman
Copy link
Member

@hakman hakman commented Feb 10, 2022

What type of PR is this?

/kind cleanup
/kind deprecation
/sig node
/cc @adisky @endocrimes @dims

What this PR does / why we need it:

pod-infra-container-image is only used for preventing the GC of the pause image since the dockershim removal.
It will be soon replaced by image pinning. There is already a PR for this feature for containerd: containerd/containerd#6456.

It makes sense to deprecate the flag and plan to remove it in kubernetes 1.27, which should be enough time for operators to adopt newer CRI versions supporting image pinning.

Which issue(s) this PR fixes:

Refs #106893

Special notes for your reviewer:

n/a

Does this PR introduce a user-facing change?

--pod-infra-container-image kubelet flag is deprecated and will be removed in future releases

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

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. area/kubelet labels Feb 10, 2022
@hakman
Copy link
Member Author

hakman commented Feb 10, 2022

/test pull-kubernetes-unit

@dims
Copy link
Member

dims commented Feb 10, 2022

/assign @ehashman @endocrimes

@hakman
Copy link
Member Author

hakman commented Feb 10, 2022

/test pull-kubernetes-e2e-gce-ubuntu-containerd

@dims
Copy link
Member

dims commented Feb 11, 2022

/approve
/lgtm

/hold for a couple of days to see if we can get additional eyes. feel free to unhold this early next week @hakman

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 11, 2022
@hakman
Copy link
Member Author

hakman commented Feb 11, 2022

Thanks @dims! 😄

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 11, 2022
@ehashman ehashman added this to Triage in SIG Node PR Triage Feb 11, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 12, 2022
@hakman hakman force-pushed the deprecate_pod-infra-container-image branch from 5792ae4 to e4695d5 Compare February 12, 2022 14:49
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 12, 2022
@hakman
Copy link
Member Author

hakman commented Feb 14, 2022

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2022
@hakman hakman force-pushed the deprecate_pod-infra-container-image branch from e4695d5 to fc1b44d Compare February 14, 2022 06:24
@hakman
Mark pod-infra-container-image flag as deprecated
57638ae 
Signed-off-by: Ciprian Hacman <ciprian@hakman.dev>
@hakman hakman force-pushed the deprecate_pod-infra-container-image branch from fc1b44d to 57638ae Compare February 14, 2022 07:12
@hakman
Copy link
Member Author

hakman commented Feb 14, 2022

/test pull-kubernetes-node-e2e-containerd

@hakman hakman requested a review from adisky February 14, 2022 08:10
@adisky
Copy link
Contributor

adisky commented Feb 14, 2022 •
edited

Thanks @hakman as Dims suggested to unhold early this week, lets wait little more to get some eyes
/cc @SergeyKanzhelev

endocrimes
endocrimes approved these changes Feb 15, 2022
View reviewed changes
Copy link
Member

@endocrimes endocrimes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

will unhold tomorrow if nobody else takes a look beforehand

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, endocrimes, hakman

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:

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

@ehashman
Copy link
Member

/priority important-soon
/triage accepted

@k8s-ci-robot 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. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 15, 2022
@ehashman ehashman moved this from Triage to Needs Approver in SIG Node PR Triage Feb 15, 2022
ehashman
ehashman reviewed Feb 15, 2022
View reviewed changes
Copy link
Member

@ehashman ehashman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/hold cancel
/lgtm

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 15, 2022
@k8s-ci-robot k8s-ci-robot merged commit efa5692 into kubernetes:master Feb 15, 2022
SIG Node PR Triage automation moved this from Needs Approver to Done Feb 15, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Feb 15, 2022
@SergeyKanzhelev
Copy link
Member

Filed a follow up: #108144

@SergeyKanzhelev SergeyKanzhelev mentioned this pull request Mar 3, 2022
Open
@rfranzke rfranzke mentioned this pull request May 5, 2022
Closed
12 tasks
@BenTheElder
Copy link
Member

There is already a PR for this feature for containerd: containerd/containerd#6456.

Worth noting that we're approaching 1.25 and this containerd feature is not merged (also unsure about cri-o ...), so users are unable to begin migrating. I think this should be considered and possibly delay the removal of the kubelet flag.

@BenTheElder BenTheElder mentioned this pull request Aug 11, 2022
Closed
@endocrimes
Copy link
Member

I think this should be considered and possibly delay the removal of the kubelet flag.

+1, we should delay to 1.28

@afbjorklund afbjorklund mentioned this pull request Apr 13, 2023
Open
@demonCoder95 demonCoder95 mentioned this pull request May 25, 2023
Merged
5 tasks
@sohankunkerkar sohankunkerkar mentioned this pull request Jun 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ehashman ehashman ehashman left review comments

@endocrimes endocrimes endocrimes approved these changes

@dims dims Awaiting requested review from dims

@adisky adisky Awaiting requested review from adisky

@SergeyKanzhelev SergeyKanzhelev Awaiting requested review from SergeyKanzhelev

Assignees

@dims dims

@endocrimes endocrimes

@ehashman ehashman

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node PR Triage
Archived in project
SIG Node PR Triage
Done
Milestone
v1.24
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@hakman @dims @adisky @k8s-ci-robot @ehashman @SergeyKanzhelev @BenTheElder @endocrimes