Kubeadm add v1beta1 config #69289

fabriziopandini · 2018-10-01T15:43:20Z

What this PR does / why we need it:
This PR adds v1beta1 version of kubeadm config API.

Which issue(s) this PR fixes:
Ref: kubernetes/kubeadm#911

Special notes for your reviewer:
Sorry about the size of this PR 😧

Depends on:

Kubeadm cleanup v1alpha3 api conversion and restore fuzzer #69058

Please consider latest 4 commits:

Add a duplicated v1beta1 API
Automated bump from v1alpha3 references to v1beta1
fix tests
(autogenerated)

Release note:

kubeadm: Add a `v1beta1` API.

@kubernetes/sig-cluster-lifecycle-pr-reviews
/sig cluster-lifecycle
/kind api-change
/area kubeadm

/cc @timothysc @luxas @rosti

rosti

Thank you @fabriziopandini ! At a quick first pass this looks OK. I'll do another, more thorough review tomorrow.

rosti · 2018-10-01T16:03:02Z

cmd/kubeadm/app/apis/kubeadm/fuzzer/fuzzer_test.go

-// v1alpha2 -> v1alpha3 migration. As the ComponentConfigs were embedded in the structs
-// earlier now have moved out it's not possible to do a lossless roundtrip "the normal way"
-// When we support v1alpha3 and higher only, we can reenable this
-


This probably needs to be moved to #69058

You are right, but I missed that train, so I'm fixing it here

neolit123

LGTM, thanks a lot!

neolit123 · 2018-10-01T16:04:59Z

cmd/kubeadm/app/util/config/cluster_test.go

+			},
+		},
+		{
+			name: "invalid v1beta1 - No CLusterStatus in kubeadm-config ConfigMap",


super minor nit:
CLusterStatus -> ClusterStatus
(appears more than once in this file)

neolit123 · 2018-10-01T16:07:31Z

cmd/kubeadm/app/util/config/cluster_test.go

+		{
+			name: "invalid v1beta1 - No CLusterStatus in kubeadm-config ConfigMap",
+			configMap: fakeConfigMap{
+				name: kubeadmconstants.InitConfigurationConfigMap, // ClusterConfiguration from kubeadm-config.


the typecheck CI complains about this const missing.

The merger of #69275 renamed InitConfigurationConfigMap to KubeadmConfigConfigMap, thus this test file won't compile.

rosti · 2018-10-02T09:15:33Z

cmd/kubeadm/app/util/config/cluster_test.go

+		{
+			name: "invalid v1beta1 - No CLusterStatus in kubeadm-config ConfigMap",
+			configMap: fakeConfigMap{
+				name: kubeadmconstants.InitConfigurationConfigMap, // ClusterConfiguration from kubeadm-config.


The merger of #69275 renamed InitConfigurationConfigMap to KubeadmConfigConfigMap, thus this test file won't compile.

rosti · 2018-10-02T10:21:05Z

cmd/kubeadm/app/util/config/testdata/defaulting/master/defaulted.yaml

@@ -115,7 +115,6 @@ evictionHard:
  imagefs.available: 15%
  memory.available: 100Mi
  nodefs.available: 10%
-  nodefs.inodesFree: 5%


Test failure here:

--- FAIL: TestConfigFileAndDefaultsToInternalConfig (0.94s) --- FAIL: TestConfigFileAndDefaultsToInternalConfig/incompleteYAMLToDefaultedv1beta1 (0.00s) masterconfig_test.go:123: the expected and actual output differs. in: testdata/defaulting/master/incomplete.yaml out: testdata/defaulting/master/defaulted.yaml groupversion: kubeadm.k8s.io/v1beta1 diff: --- expected +++ actual @@ -115,6 +115,7 @@ imagefs.available: 15%! (MISSING) memory.available: 100Mi nodefs.available: 10%! (MISSING)+ nodefs.inodesFree: 5%! (MISSING) evictionPressureTransitionPeriod: 5m0s failSwapOn: true fileCheckFrequency: 20s FAIL FAIL k8s.io/kubernetes/cmd/kubeadm/app/util/config 1.007s

There is probably some change in the KubeletConfig here. So whenever a change is made there we need to update this. We may want to just check if we have the right kind here. Yet, this is a job for another PR.

timothysc

Minor comments otherwise LGTM
/approve

timothysc · 2018-10-02T21:57:04Z

cmd/kubeadm/app/apis/kubeadm/v1beta1/bootstraptokenstring.go

+// for both validation of the practically of the API server from a joining node's point
+// of view and as an authentication method for the node in the bootstrap phase of
+// "kubeadm join". This token is and should be short-lived
+type BootstrapTokenString struct {


So with bootstrap tokens finding a landing home this cycle do we still need this? We've been carrying this for a while now.

If you don't mind I will iterate on this in a separated PR trying to get rid of the code for bootstrap token management that now is duplicated for each API version

timothysc · 2018-10-02T22:04:59Z

cmd/kubeadm/app/apis/kubeadm/v1beta1/doc.go

+limitations under the License.
+*/
+
+// +k8s:defaulter-gen=TypeMeta


You may need to pull in @neolit123 's recent changes here.

timothysc · 2018-10-02T22:09:17Z

cmd/kubeadm/app/cmd/config.go

@@ -228,6 +229,7 @@ func NewCmdConfigMigrate(out io.Writer) *cobra.Command {
 			locally in the CLI tool without ever touching anything in the cluster.
 			In this version of kubeadm, the following API versions are supported:
 			- %s
+			- %s


This gives some info about api version supported by kubeadm command migrate...

timothysc · 2018-10-02T22:11:33Z

cmd/kubeadm/app/cmd/config_test.go

@@ -231,32 +232,28 @@ func TestImagesPull(t *testing.T) {
 }

 func TestMigrate(t *testing.T) {
-	/*
-		TODO: refactor this to test v1alpha3 --> v1beta1 after introducing v1beta1
+	cfg := []byte(dedent.Dedent(`


Fine for now, but next time we can break this out as a follow-up PR.

fabriziopandini · 2018-10-04T13:46:00Z

/test pull-kubernetes-integration

timothysc

/lgtm

k8s-ci-robot · 2018-10-04T21:49:26Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

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:

~~cmd/kubeadm/OWNERS~~ [fabriziopandini,timothysc]
hack/OWNERS

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 added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 1, 2018

k8s-ci-robot requested a review from luxas October 1, 2018 15:43

k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Oct 1, 2018

k8s-ci-robot requested review from rosti and timothysc October 1, 2018 15:43

rosti reviewed Oct 1, 2018

View reviewed changes

neolit123 reviewed Oct 1, 2018

View reviewed changes

rosti reviewed Oct 2, 2018

View reviewed changes

fabriziopandini force-pushed the kubeadm-add-v1beta1 branch from e6798c0 to 1e5e7e0 Compare October 2, 2018 11:04

timothysc reviewed Oct 2, 2018

View reviewed changes

fabriziopandini added 2 commits October 4, 2018 12:02

Add a duplicated v1beta1 API

8ae5179

Automated bump from v1alpha3 references to v1beta1

488ac68

fabriziopandini force-pushed the kubeadm-add-v1beta1 branch from 1e5e7e0 to b99deb4 Compare October 4, 2018 11:04

fabriziopandini added 2 commits October 4, 2018 13:20

Fix tests

9aec633

autogenerated

b4092ac

fabriziopandini force-pushed the kubeadm-add-v1beta1 branch from b99deb4 to b4092ac Compare October 4, 2018 11:20

k8s-ci-robot assigned timothysc Oct 4, 2018

timothysc approved these changes Oct 4, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2018

timothysc added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2018

k8s-ci-robot merged commit 5e3dc7d into kubernetes:master Oct 4, 2018

fabriziopandini deleted the kubeadm-add-v1beta1 branch October 8, 2018 11:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubeadm add v1beta1 config #69289

Kubeadm add v1beta1 config #69289

fabriziopandini commented Oct 1, 2018 •

edited

rosti left a comment

rosti Oct 1, 2018

fabriziopandini Oct 2, 2018

neolit123 left a comment

neolit123 Oct 1, 2018

neolit123 Oct 1, 2018

rosti Oct 2, 2018

rosti Oct 2, 2018

rosti Oct 2, 2018

timothysc left a comment

timothysc Oct 2, 2018

fabriziopandini Oct 4, 2018

timothysc Oct 2, 2018

fabriziopandini Oct 4, 2018

timothysc Oct 2, 2018

fabriziopandini Oct 4, 2018

timothysc Oct 2, 2018

fabriziopandini commented Oct 4, 2018

timothysc left a comment

k8s-ci-robot commented Oct 4, 2018

Kubeadm add v1beta1 config #69289

Kubeadm add v1beta1 config #69289

Conversation

fabriziopandini commented Oct 1, 2018 • edited

rosti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neolit123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timothysc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabriziopandini commented Oct 4, 2018

timothysc left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 4, 2018

fabriziopandini commented Oct 1, 2018 •

edited