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

Removed compatibality code for kubelet 1.2. #48996

Merged
merged 1 commit into from
Oct 26, 2017
Merged

Removed compatibality code for kubelet 1.2. #48996

merged 1 commit into from
Oct 26, 2017

Conversation

k82cn
Copy link
Member

@k82cn k82cn commented Jul 16, 2017

What this PR does / why we need it:
The next release is 1.8, it's time to remove the backward compatibility code for kubelet 1.2.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #48995

Release note:

The NodeController will not support kubelet 1.2.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 16, 2017
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 16, 2017
xiangpengzhao
xiangpengzhao suggested changes Jul 16, 2017
View reviewed changes
Copy link
Contributor

@xiangpengzhao xiangpengzhao left a comment

Choose a reason for hiding this comment

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

There may be also other v1.1.0 things we want to remove.

pkg/controller/node/nodecontroller.go Outdated
@@ -67,10 +67,6 @@ var (
ErrCloudInstance = errors.New("cloud provider doesn't support instances.")
gracefulDeletionVersion = utilversion.MustParseSemantic("v1.1.0")
Copy link
Contributor

Choose a reason for hiding this comment

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

Also remove this v1.1.0 thing? And an associated TODO here: https://github.com/k82cn/kubernetes/blob/f6161bffdc9436edc92d435fb2c5258d40ec567a/pkg/controller/node/controller_utils.go#L177

Copy link
Member Author

Choose a reason for hiding this comment

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

good point; i'll also remove them.

Copy link
Member Author

Choose a reason for hiding this comment

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

done :).

@smarterclayton
Copy link
Contributor

@kubernetes/sig-node-pr-reviews

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jul 17, 2017
@k82cn k82cn force-pushed the remove_nc_old_code branch 2 times, most recently from b2974f0 to c6688fe Compare July 24, 2017 07:11
@k82cn
Copy link
Member Author

k82cn commented Jul 24, 2017

/retest

@k82cn
Copy link
Member Author

k82cn commented Jul 30, 2017

/retest

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2017
@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 4, 2017
@k82cn k82cn mentioned this pull request Sep 13, 2017
Merged
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 23, 2017
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2017
@xiangpengzhao
Copy link
Contributor

I'm surprised this PR is still not merged :)

@yujuhong
Copy link
Contributor

Need to fix bazel. Otherwise, LGTM

/assign

@k82cn
Copy link
Member Author

k82cn commented Oct 24, 2017

done :).

@yujuhong
Copy link
Contributor

Thanks for the cleanup!

/lgtm

I don't have permission to approve though. Ping some folks for approval: @gmarek @dchen1107

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2017
@dchen1107
Copy link
Member

/lgtm

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, k82cn, yujuhong

Associated issue: 48995

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 25, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 53760, 48996, 51267, 54414). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 38cb569 into kubernetes:master Oct 26, 2017
@k82cn k82cn deleted the remove_nc_old_code branch May 16, 2022 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiangpengzhao xiangpengzhao xiangpengzhao requested changes

Assignees

@smarterclayton smarterclayton

@dchen1107 dchen1107

@ingvagabund ingvagabund

@yujuhong yujuhong

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove the backward compatibility code for kubelet 1.2 in NodeController.
8 participants
@k82cn @smarterclayton @xiangpengzhao @yujuhong @dchen1107 @k8s-github-robot @ingvagabund @k8s-ci-robot