-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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.
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
Remove support for Kubernetes 1.8 and earlier #8208
Conversation
Hi @johngmyers. 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 Once the patch is verified, the new status will be reflected by the 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. |
e5deb07
to
e8d3aa4
Compare
e8d3aa4
to
782a2d5
Compare
/ok-to-test |
782a2d5
to
d843915
Compare
// OldestRecommendedKubernetesVersion is the oldest kubernetes version that is not deprecated in Kops | ||
OldestRecommendedKubernetesVersion = "1.10.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's just me, but I would not recommend any k8s version where the default etcd is not 3.x. Anyone having to got through the upgrade from 2.x to 3.x will be pretty annoyed.
kops/pkg/model/components/etcdmanager/options.go
Lines 60 to 61 in 298f796
if b.IsKubernetesGTE("1.11") { | |
etcdCluster.Version = "3.2.18" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current policy as I understand it is to remove support from 1.8 and earlier in kops 1.18, then do rolling removals in subsequent versions. That means removing support from 1.9 in kops 1.19. There was generalized talk about narrowing the gap in future releases, but no specific decisions there have been made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have anything against supporting these versions, but the name says "recommended" version. I think "non-deprecated" is not quite the same as "recommended".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not able to find a better word for "non-deprecated". The code uses "recommended" for "non-deprecated" later in the file, where it's dealing with channels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use "legacy" here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word "recommended" is not made visible to the user. The constant only demarcates what is not described as "deprecated".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if & when we come up with better words we can rename this constant without affecting anything user-visible!
This LGTM - thanks @johngmyers Agree that we have a lot of version-thresholds now - deprecated, minimum-recommended, unsupported etc :-) I imagine we'll figure this out over time - or maybe just align them! /approve |
Readding LGTM - thanks for fixing the staticcheck error @johngmyers /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johngmyers, justinsb 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 |
/lgtm |
No description provided.