kubeadm - deprecate feature-gates HighAvailability, SelfHosting, CertsInSecrets #67786
Conversation
k8s-ci-robot
added
the
release-note
|
@fabriziopandini: GitHub didn't allow me to request PR reviews from the following users: fabriziopandini. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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
added
sig/cluster-lifecycle
|
thanks for the PR @fabriziopandini added a couple of text related comments only. |
| fmt.Printf("[upgrade/config] %s\n", m) | ||
| } | ||
| } | ||
| return nil, fmt.Errorf("[upgrade/config] FATAL. Unable to upgrade a cluster using deprecated featuregates flag. Please see release notes") |
There was a problem hiding this comment.
we can possibly change this to:
Unable to upgrade a cluster using deprecated feature-gate flags. Please see the release notes.
| continue | ||
| } | ||
| if !reflect.DeepEqual(test.expectedMsg, msg) { | ||
| t.Error("Upgrade didn't returned expected message") |
There was a problem hiding this comment.
i would lowercase the above errors, but that's just a nit.
cmd/kubeadm/app/features/features.go
Outdated
| for k := range deprecatedMsg { | ||
| m = append(m, k) | ||
| } | ||
| return m, fmt.Errorf("feature-gates %s are deprecated", strings.Join(deprecatedKey, ",")) |
There was a problem hiding this comment.
i would change this to the following to better handle singular vs plural:
the following feature-gates are deprecated: %s
There was a problem hiding this comment.
This looks awkward in Go code. We return error with a valid slice of deprecation messages. Having deprecation messages does not mean that CheckDeprecatedFlags failed.
The best thing here is to reserve the error only for the case, when we don't recognize the feature gate at all (the case further up the code).
Here we should simply return the slice and no error.
fabriziopandini
force-pushed
the
kubeadm-deprecate-featureflags
branch
from
7cf3df2 to
689417c
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini, timothysc 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 |
|
/test pull-kubernetes-kubemark-e2e-gce-big |
1 similar comment
|
/test pull-kubernetes-kubemark-e2e-gce-big |
|
Automatic merge from submit-queue (batch tested with PRs 67776, 67503, 67679, 67786, 67830). If you want to cherry-pick this change to another branch, please follow the instructions here. |
What this PR does / why we need it:
As for sig discussion (see meeting notes - August 22 - 2018) we are going to block usage of feature gates HighAvailability, SelfHosting, CertsInSecrets for new clusters and block updates to v1.12 of existing clusters using such features.
Which issue(s) this PR fixes *(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)Fixes # kubernetes/kubeadm#1058
Special notes for your reviewer:
I'm going to open issue to track code cleanup in v1.13
Release note:
/sig cluster-lifecycle
/kind feature
/kind cleanup
/assign @timothysc
/cc