kubelet: remove the --bootstrap-checkpoint-path feature #91577

knabben · 2020-05-29T13:14:14Z

What type of PR is this?

/kind cleanup
/area kubelet
/area kubelet-api

What this PR does / why we need it:

Removing --bootstrap-checkpoint-path flag and related code.

Which issue(s) this PR fixes:

Fixes #92763
Refs #86843 #72202 (comment)

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

The Kubelet's `--bootstrap-checkpoint-path` option is now removed.

k8s-ci-robot · 2020-05-29T13:14:22Z

Hi @knabben. 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/test-infra repository.

knabben · 2020-05-29T13:14:34Z

/assign @timothysc

We are writing the possible alternatives in the deprecation warning, lmk if you have something to enhace the comment.

timothysc · 2020-05-29T15:19:44Z

We should remove the associated code as well that is plumbed through.

/cc @yastij

knabben · 2020-05-29T15:42:48Z

The process for deprecating these flags is to wait 2-4 releases before the final cleanup, first we add the deprecation warning. So, this is mostly planned for 1.23, does it works for you?

yastij · 2020-05-29T15:54:59Z

cmd/kubelet/app/options/options.go

@@ -394,6 +393,8 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) {
 	fs.MarkDeprecated("chaos-chance", "will be removed in a future version.")
 	fs.StringVar(&f.SeccompProfileRoot, "seccomp-profile-root", f.SeccompProfileRoot, "<Warning: Alpha feature> Directory path for seccomp profiles.")
 	fs.MarkDeprecated("seccomp-profile-root", "will be removed in 1.23, in favor of using the `<root-dir>/seccomp` directory")
+	fs.StringVar(&f.BootstrapCheckpointPath, "bootstrap-checkpoint-path", f.BootstrapCheckpointPath, "<Warning: Alpha feature> Path to the directory where the checkpoints are stored")


this is marked as an alpha feature, as per our deprecation policy https://kubernetes.io/docs/reference/using-api/deprecation-policy/#deprecating-a-flag-or-cli this can be removed immediately as pointed by @timothysc

agree... let's not carry deprecated alpha-level functionality

Thanks for pointing in the docs, I will remove the code/flag in this PR.

Rolling back:

Initial basic bootstrap-checkpoint support #50984, the PR introduced the flag and the initial implementation of pod checkpoint.

parts of Node-level Checkpointing manager: Migrate dockershim and device plugin manager checkpointing #56040 since the original checkpoint ended using the Checkpoint Manager.

I'll defer to sig-node reviewers on the partial rollback

yastij · 2020-05-29T15:55:32Z

cc @liggitt - as we discussed this a while ago

pkg/kubelet/kubelet.go

kow3ns · 2020-06-01T16:17:13Z

/remove-sig apps

yastij · 2020-06-11T16:47:38Z

/lgtm

knabben · 2020-06-12T11:58:08Z

/test pull-kubernetes-e2e-gce-storage-slow

yastij · 2020-06-24T11:24:39Z

ping @kubernetes/sig-node-api-reviews

fejta-bot · 2020-06-24T11:45:42Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

yastij · 2020-06-24T13:01:49Z

/remove-label api-review

k8s-ci-robot · 2020-06-24T13:01:50Z

@yastij: Those labels are not set on the issue: api-review

In response to this:

/remove-label api-review

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.

k8s-ci-robot · 2020-06-24T13:02:48Z

@yastij: The label(s) /remove-label api-change cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash

In response to this:

/remove-label api-change

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.

liggitt · 2020-06-24T15:13:01Z

lgtm, will defer to node approvers

neolit123 · 2020-07-02T17:10:28Z

/retitle kubelet: remove the --bootstrap-checkpoint-path feature

neolit123 · 2020-07-02T17:11:51Z

@knabben

please add Fixes #92763 in the PR description.

#92763

neolit123 · 2020-07-02T17:12:58Z

/assign @derekwaynecarr

derekwaynecarr · 2020-07-08T20:38:45Z

reviewing now.

derekwaynecarr · 2020-07-08T21:33:35Z

/approve
/lgtm

ironically, we are likely to require some alternate form of checkpoint for pod resource consumption to facilitate vertical autoscaling, but we can safely remove this particular use of checkpointing. thanks for the pr.

derekwaynecarr · 2020-07-08T23:39:23Z

@liggitt can you approve api change?

liggitt · 2020-07-08T23:42:05Z

/approve

k8s-ci-robot · 2020-07-08T23:42:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, knabben, liggitt

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:

~~cmd/kubelet/OWNERS~~ [derekwaynecarr,liggitt]
~~pkg/apis/core/OWNERS~~ [liggitt]
~~pkg/kubelet/OWNERS~~ [derekwaynecarr,liggitt]

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 requested review from Random-Liu and vishh May 29, 2020 13:14

k8s-ci-robot assigned timothysc May 29, 2020

k8s-ci-robot requested a review from yastij May 29, 2020 15:19

yastij reviewed May 29, 2020

View reviewed changes

knabben mentioned this pull request May 29, 2020

Deprecating --experimental-allocatable-ignore-eviction flag #91578

Merged

knabben force-pushed the kubelet-bootstrap branch from dd18470 to abb8867 Compare May 31, 2020 16:04

liggitt reviewed Jun 1, 2020

View reviewed changes

pkg/kubelet/kubelet.go Show resolved Hide resolved

k8s-ci-robot removed the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Jun 1, 2020

knabben force-pushed the kubelet-bootstrap branch from abb8867 to 79e4c7f Compare June 3, 2020 01:43

k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Jun 3, 2020

knabben closed this Jun 10, 2020

knabben reopened this Jun 10, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 11, 2020

k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jun 24, 2020

neolit123 mentioned this pull request Jul 2, 2020

sunset and remove https://github.com/kubernetes/enhancements/blob/master/keps/sig-cluster-lifecycle/kubeadm/0004-bootstrap-checkpointing.md kubernetes/enhancements#1103

Closed

k8s-ci-robot changed the title ~~Deprecating Kubelet --bootstrap-checkpoint-path flag~~ kubelet: remove the --bootstrap-checkpoint-path feature Jul 2, 2020

This was referenced Jul 2, 2020

cleanup the code in the kubelet for bootstrap checkpointing kubernetes/kubeadm#2211

Closed

cleanup the code in the kubelet for bootstrap checkpointing #92763

Closed

k8s-ci-robot assigned derekwaynecarr Jul 2, 2020

liggitt added this to the v1.19 milestone Jul 8, 2020

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

k8s-ci-robot merged commit 1e3eeba into kubernetes:master Jul 9, 2020

github-actions bot mentioned this pull request Jul 14, 2020

Week Ending July 12, 2020 dev-obs/actus#193

Open

AlexeyPerevalov mentioned this pull request Nov 3, 2020

Remove useless variable and if in kubelet.go #96141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubelet: remove the --bootstrap-checkpoint-path feature #91577

kubelet: remove the --bootstrap-checkpoint-path feature #91577

knabben commented May 29, 2020 •

edited

Loading

k8s-ci-robot commented May 29, 2020

knabben commented May 29, 2020 •

edited

Loading

timothysc commented May 29, 2020

knabben commented May 29, 2020 •

edited

Loading

yastij May 29, 2020

liggitt May 29, 2020

knabben May 29, 2020

knabben May 31, 2020 •

edited

Loading

liggitt Jun 1, 2020

yastij commented May 29, 2020

kow3ns commented Jun 1, 2020

yastij commented Jun 11, 2020

knabben commented Jun 12, 2020

yastij commented Jun 24, 2020

fejta-bot commented Jun 24, 2020

yastij commented Jun 24, 2020

k8s-ci-robot commented Jun 24, 2020

k8s-ci-robot commented Jun 24, 2020

liggitt commented Jun 24, 2020

neolit123 commented Jul 2, 2020

neolit123 commented Jul 2, 2020 •

edited

Loading

neolit123 commented Jul 2, 2020

derekwaynecarr commented Jul 8, 2020

derekwaynecarr commented Jul 8, 2020

derekwaynecarr commented Jul 8, 2020

liggitt commented Jul 8, 2020

k8s-ci-robot commented Jul 8, 2020

kubelet: remove the --bootstrap-checkpoint-path feature #91577

kubelet: remove the --bootstrap-checkpoint-path feature #91577

Conversation

knabben commented May 29, 2020 • edited Loading

k8s-ci-robot commented May 29, 2020

knabben commented May 29, 2020 • edited Loading

timothysc commented May 29, 2020

knabben commented May 29, 2020 • edited Loading

yastij May 29, 2020

Choose a reason for hiding this comment

liggitt May 29, 2020

Choose a reason for hiding this comment

knabben May 29, 2020

Choose a reason for hiding this comment

knabben May 31, 2020 • edited Loading

Choose a reason for hiding this comment

liggitt Jun 1, 2020

Choose a reason for hiding this comment

yastij commented May 29, 2020

kow3ns commented Jun 1, 2020

yastij commented Jun 11, 2020

knabben commented Jun 12, 2020

yastij commented Jun 24, 2020

fejta-bot commented Jun 24, 2020

yastij commented Jun 24, 2020

k8s-ci-robot commented Jun 24, 2020

k8s-ci-robot commented Jun 24, 2020

liggitt commented Jun 24, 2020

neolit123 commented Jul 2, 2020

neolit123 commented Jul 2, 2020 • edited Loading

neolit123 commented Jul 2, 2020

derekwaynecarr commented Jul 8, 2020

derekwaynecarr commented Jul 8, 2020

derekwaynecarr commented Jul 8, 2020

liggitt commented Jul 8, 2020

k8s-ci-robot commented Jul 8, 2020

knabben commented May 29, 2020 •

edited

Loading

knabben commented May 29, 2020 •

edited

Loading

knabben commented May 29, 2020 •

edited

Loading

knabben May 31, 2020 •

edited

Loading

neolit123 commented Jul 2, 2020 •

edited

Loading