kubeadm: Remove feature gates from JoinConfiguration #70755
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
area/kubeadm
sig/cluster-lifecycle
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
/assign @fabriziopandini @luxas @timothysc |
There was a problem hiding this comment.
looks good. @timothysc suggestion is good to have the extra test.
timothysc
added
the
priority/important-soon
k8s-ci-robot
removed
the
needs-priority
|
I will add the conversion test tomorrow. |
There was a problem hiding this comment.
@ereslibre Thanks for this PR! Well done!
@luxas @rosti I was expecting fuzzer test or custom roundtrip tests to fail after removing a field in the API, but it doesn't seems the case... what I'm missing?
|
/this-is-fine |
|
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. |
|
/test pull-kubernetes-e2e-gce-100-performance |
|
@fabriziopandini My educated guess is that the fuzzer works with internal types to start with. Therefore if we have removed this field and its conversion code from an internal type, the fuzzer won't fail (it has nothing to fuzz) even though the field is still present in some of the versioned types. |
|
/test pull-kubernetes-e2e-gce-100-performance |
There was a problem hiding this comment.
given bazel and verify tests are passing.
/lgtm
thanks @ereslibre
k8s-ci-robot
added
the
lgtm
k8s-ci-robot
added
size/L
k8s-ci-robot
added
size/M
There was a problem hiding this comment.
Thanks @ereslibre !
/approve
I left final lgtm to @timothysc
k8s-ci-robot
added
the
approved
Relay on the feature gates provided by the ClusterConfiguration when downloaded from the cluster during the join process.
| @@ -542,7 +533,7 @@ func (j *Join) BootstrapKubelet(tlsBootstrapCfg *clientcmdapi.Config) error { | |||
| } | |||
|
|
|||
| // This feature is disabled by default in kubeadm | |||
| if features.Enabled(j.cfg.FeatureGates, features.DynamicKubeletConfig) { | |||
| if features.Enabled(j.initCfg.FeatureGates, features.DynamicKubeletConfig) { | |||
There was a problem hiding this comment.
nit: in the future this should be use/be called ClusterConfiguration, because the thing returned from the cluster configmap at this stage is a clusterconfig, not an initconfig
This is not specific to this PR though
There was a problem hiding this comment.
Yes, I'll file a bug for that later, thank you.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ereslibre, fabriziopandini, luxas 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 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Relay on the feature gates provided by the ClusterConfiguration
when downloaded from the cluster during the join process.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes kubernetes/kubeadm#1088
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?: