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 commandline parameters related to dockershim as deprecated #98730

Merged
merged 1 commit into from Feb 16, 2021
Merged

Mark commandline parameters related to dockershim as deprecated #98730

merged 1 commit into from Feb 16, 2021

Conversation

dims
Copy link
Member

@dims dims commented Feb 3, 2021 •
edited

marking Dockershim only flags as deprecated.

Signed-off-by: Davanum Srinivas davanum@gmail.com

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

Related to kubernetes/enhancements#2221

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Several flags related to the deprecated dockershim which are present in the kubelet command line are now deprecated.

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

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. 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 Feb 3, 2021
@dims
Copy link
Member Author

dims commented Feb 3, 2021

/assign @SergeyKanzhelev @sjenning

@dims
Copy link
Member Author

dims commented Feb 3, 2021

/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 3, 2021
@dims
Copy link
Member Author

dims commented Feb 3, 2021

/priority important-soon

@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. area/kubelet and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 3, 2021
Cheef5
Cheef5 approved these changes Feb 3, 2021
View reviewed changes
Copy link

@Cheef5 Cheef5 left a comment

Choose a reason for hiding this comment

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

/lgtm approve

@ehashman ehashman added this to Triage in SIG Node PR Triage Feb 3, 2021
ehashman
ehashman suggested changes Feb 4, 2021
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.

/triage accepted

pkg/kubelet/config/flags.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 4, 2021
@ehashman ehashman moved this from Triage to Waiting on Author in SIG Node PR Triage Feb 4, 2021
@dims dims force-pushed the mark-cmd-line-params-related-to-dockershim-as-deprecated branch from 0eac1ca to c558272 Compare February 4, 2021 03:10
adisky
adisky reviewed Feb 4, 2021
View reviewed changes
pkg/kubelet/config/flags.go Outdated
@@ -95,15 +95,23 @@ func (s *ContainerRuntimeOptions) AddFlags(fs *pflag.FlagSet) {
fs.StringVar(&s.DockershimRootDirectory, "experimental-dockershim-root-directory", s.DockershimRootDirectory, "Path to the dockershim root directory.")
fs.MarkHidden("experimental-dockershim-root-directory")
fs.StringVar(&s.PodSandboxImage, "pod-infra-container-image", s.PodSandboxImage, fmt.Sprintf("The image whose network/ipc namespaces containers in each pod will use. %s", dockerOnlyWarning))
fs.MarkDeprecated("pod-infra-container-image", "will be removed along with dockershim.")
Copy link
Contributor

Choose a reason for hiding this comment

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

if we remove this one with docker-shim, We need some way to get information about sandbox image from container runtime #81756.

Copy link
Member Author

Choose a reason for hiding this comment

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

@adisky indeed. let's track that as a new issue, we may prefer another flag as this one started with docker and then was co-opted by gc manager if i remember correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

#98765

Copy link
Member

@neolit123 neolit123 Feb 4, 2021 •
edited

Choose a reason for hiding this comment

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

+1 on re-using the flag because of GC.

eventually CRI implementers should dump their info properly the way containerd is doing today on it's socket - you can try crictl info for that.
but currently not e.g. cri-o and dockershim.

at that point we can deprecate this flag, if we don't want to allow the override.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the deprecation from this flag. improved the description as well.

This was referenced Feb 4, 2021
Closed
Closed
@dims dims force-pushed the mark-cmd-line-params-related-to-dockershim-as-deprecated branch from c558272 to 96ecba2 Compare February 8, 2021 20:17
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels Feb 8, 2021
ehashman
ehashman reviewed Feb 11, 2021
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.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 11, 2021
@ehashman
Copy link
Member

@dims from verify: Run ./hack/update-gofmt.sh

@dims
Mark commandline parameters related to dockershim as deprecated
6150378 
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
@dims dims force-pushed the mark-cmd-line-params-related-to-dockershim-as-deprecated branch from 96ecba2 to 6150378 Compare February 11, 2021 02:04
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 11, 2021
@dims
Copy link
Member Author

dims commented Feb 11, 2021

@ehashman whoops. thanks. done.

@dims
Copy link
Member Author

dims commented Feb 11, 2021

/retest

@kikisdeliveryservice kikisdeliveryservice moved this from Waiting on Author to Needs Reviewer in SIG Node PR Triage Feb 11, 2021
ehashman
ehashman reviewed Feb 11, 2021
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.

re-
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 11, 2021
@ehashman ehashman moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Feb 11, 2021
@mrunalp
Copy link
Contributor

mrunalp commented Feb 16, 2021

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cheef5, dims, mrunalp

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 16, 2021
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit a4b3d31 into kubernetes:master Feb 16, 2021
SIG Node PR Triage automation moved this from Needs Approver to Done Feb 16, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Feb 16, 2021
@adisky adisky mentioned this pull request Feb 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123 neolit123 left review comments

@ehashman ehashman ehashman requested changes

@adisky adisky adisky left review comments

@Cheef5 Cheef5 Cheef5 approved these changes

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@mtaufen mtaufen Awaiting requested review from mtaufen

@SergeyKanzhelev SergeyKanzhelev Awaiting requested review from SergeyKanzhelev

Assignees

@sjenning sjenning

@ehashman ehashman

@SergeyKanzhelev SergeyKanzhelev

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. 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/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node PR Triage
Done
Milestone
v1.21
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@dims @ehashman @mrunalp @k8s-ci-robot @fejta-bot @neolit123 @adisky @Cheef5 @sjenning @SergeyKanzhelev