Fix `kubeadm init --token-ttl=0`/config `tokenTTL: "0"`. #54640

mattmoyer · 2017-10-26T16:34:27Z

What this PR does / why we need it:
This PR fixes a bug that was introduced in #48783 when we reduced the default token TTL from infinite to 24 hours.

This change worked for kubeadm token create but was broken for kubeadm init because the API machinery defaulting mechanism couldn't differentiate between an unset value (which should default to 24 hours) and a value explicitly set to 0 (which should mean infinite).

The fix is to change TokenTTL from a metav1.Duration to *metav1.Duration so that nil can represent the unspecified value.

This bug was introduced in #48783.

Which issue this PR fixes: fixes kubernetes/kubeadm#509

Special notes for your reviewer:
This should be a cherry pick candidate for 1.8.x.

Here is some example output for each case:

Unspecified TTL using the `kubeadm init --token-ttl [...]` flag

root@master:/vagrant# ./kubeadm reset >/dev/null && ./kubeadm init >/dev/null && ./kubeadm token list
TOKEN                     TTL       EXPIRES                USAGES                   DESCRIPTION                                                EXTRA GROUPS
abbd6a.389f44259787c5cb   23h       2017-10-27T16:00:32Z   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token

1 hour TTL using the `kubeadm init --token-ttl [...]` flag

root@master:/vagrant# ./kubeadm reset >/dev/null && ./kubeadm init --token-ttl 1h >/dev/null && ./kubeadm token list
TOKEN                     TTL       EXPIRES                USAGES                   DESCRIPTION                                                EXTRA GROUPS
cb3368.9087edaf99b28c32   59m       2017-10-26T17:01:33Z   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token

Infinite TTL using the `kubeadm init --token-ttl [...]` flag

root@master:/vagrant# ./kubeadm reset >/dev/null && ./kubeadm init --token-ttl 0 >/dev/null && ./kubeadm token list
TOKEN                     TTL         EXPIRES   USAGES                   DESCRIPTION                                                EXTRA GROUPS
2f709c.972cf789c277671a   <forever>   <never>   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token

Unspecified TTL using the `tokenTTL` configuration value

root@master:/vagrant# cat kubeadm-default.yaml
---
apiVersion: kubeadm.k8s.io/v1alpha1
kind: MasterConfiguration
root@master:/vagrant# ./kubeadm reset >/dev/null && ./kubeadm init --config ./kubeadm-default.yaml >/dev/null && ./kubeadm token list
TOKEN                     TTL       EXPIRES                USAGES                   DESCRIPTION                                                EXTRA GROUPS
e4171c.74a2582c4df88c6e   23h       2017-10-27T16:03:39Z   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token

1 hour TTL using the `tokenTTL` configuration value

root@master:/vagrant# cat kubeadm-1h.yaml
---
apiVersion: kubeadm.k8s.io/v1alpha1
kind: MasterConfiguration
tokenTTL: "1h"
root@master:/vagrant# ./kubeadm reset >/dev/null && ./kubeadm init --config ./kubeadm-1h.yaml >/dev/null && ./kubeadm token list
TOKEN                     TTL       EXPIRES                USAGES                   DESCRIPTION                                                EXTRA GROUPS
8f8722.ef80f0370dfd866b   59m       2017-10-26T17:04:42Z   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token

Infinite TTL using the `tokenTTL` configuration value

root@master:/vagrant# cat kubeadm-infinite.yaml
---
apiVersion: kubeadm.k8s.io/v1alpha1
kind: MasterConfiguration
tokenTTL: "0"
root@master:/vagrant# ./kubeadm reset >/dev/null && ./kubeadm init --config ./kubeadm-infinite.yaml >/dev/null && ./kubeadm token list
TOKEN                     TTL         EXPIRES   USAGES                   DESCRIPTION                                                EXTRA GROUPS
6e6f32.833465c25c175c38   <forever>   <never>   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token

Release note:

kubeadm init: fix a bug that prevented the --token-ttl flag and tokenTTL configuration value from working as expected for infinite (0) values.

/kind bug
cc @kubernetes/sig-cluster-lifecycle-bugs @luxas @jbeda @wackxu

mattmoyer · 2017-10-26T19:43:08Z

Cherry pick for release-1.8: #54649

dmmcquay · 2017-10-26T21:12:47Z

/lgtm

luxas

Thanks @mattmoyer!

/lgtm

k8s-github-robot · 2017-10-27T18:02:49Z

/test all [submit-queue is verifying that this PR is safe to merge]

mattmoyer · 2017-10-27T18:53:36Z

/retest

luxas · 2017-10-27T19:17:03Z

/retest

luxas · 2017-10-27T19:46:16Z

@mattmoyer this actually seems to be a real failure:

W1027 19:11:48.687] panic: runtime error: invalid memory address or nil pointer dereference
W1027 19:11:48.687] [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x2e0a396]
W1027 19:11:48.688] 
W1027 19:11:48.688] goroutine 1 [running]:
W1027 19:11:48.688] k8s.io/kubernetes/cmd/kubeadm/app/cmd.AddInitConfigFlags(0xc4206df5f0, 0xc42009dba0, 0xc4206e2970)
W1027 19:11:48.688] 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/init.go:183 +0x3e6
W1027 19:11:48.688] k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdConfigUploadFromFlags(0x5f331a0, 0xc4200a6008, 0xc4206e28f0, 0x1)
W1027 19:11:48.688] 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/config.go:166 +0x268
W1027 19:11:48.689] k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdConfigUpload(0x5f331a0, 0xc4200a6008, 0xc4206e28f0, 0xa)
W1027 19:11:48.689] 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/config.go:77 +0x13c
W1027 19:11:48.689] k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdConfig(0x5f331a0, 0xc4200a6008, 0x1)
W1027 19:11:48.689] 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/config.go:62 +0x258
W1027 19:11:48.689] k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewKubeadmCommand(0x5f33160, 0xc4200a6000, 0x5f331a0, 0xc4200a6008, 0x5f331a0, 0xc4200a6010, 0x5f37ba0)
W1027 19:11:48.690] 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/cmd.go:73 +0x192
W1027 19:11:48.690] main.main()
W1027 19:11:48.690] 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/genkubedocs/gen_kube_docs.go:79 +0x327

mattmoyer · 2017-10-27T20:55:26Z

[...] this actually seems to be a real failure:

Thanks, I'll take a look. I assumed it was a flake because it passed once.

luxas · 2017-10-27T21:13:01Z

it passed once.

I think it started failing now as an other, related PR merged (the kubeadm man page PR) recently

fejta-bot · 2017-11-01T15:47:24Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

mattmoyer · 2017-11-01T18:24:02Z

/lgtm cancel

luxas · 2017-11-01T18:24:47Z

/approve cancel

k8s-github-robot · 2017-11-01T18:25:22Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dmmcquay, mattmoyer
We suggest the following additional approver: luxas

Assign the PR to them by writing /assign @luxas in a comment when ready.

Associated issue: 509

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

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

cmd/kubeadm/OWNERS

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

timothysc · 2017-11-06T22:56:11Z

/cc @jpbetz this needs to make the 1.8.3 release-train.

mattmoyer · 2017-11-06T22:59:06Z

I'm taking another look now to see if I can figure out the pull-kubernetes-verify error. Would love help if anyone else has time to take a look.

fejta-bot · 2017-11-07T01:11:31Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-11-07T04:20:31Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

jpbetz · 2017-11-07T06:30:46Z

@mattmoyer the stack trace in the test failure:

panic: runtime error: invalid memory address or nil pointer dereference
 [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x2e90eb6]
 
 goroutine 1 [running]:
 k8s.io/kubernetes/cmd/kubeadm/app/cmd.AddInitConfigFlags(0xc42076def0, 0xc420ac0000, 0xc42021adc0)
 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/init.go:183 +0x3e6
 k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdConfigUploadFromFlags(0x6060b20, 0xc42000e018, 0xc42021ad40, 0x1)
 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/config.go:166 +0x268
 k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdConfigUpload(0x6060b20, 0xc42000e018, 0xc42021ad40, 0xa)
 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/config.go:77 +0x13c
 k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdConfig(0x6060b20, 0xc42000e018, 0x1)
 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/config.go:62 +0x258
 k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewKubeadmCommand(0x6060ae0, 0xc42000e010, 0x6060b20, 0xc42000e018, 0x6060b20, 0xc42000e020, 0x6065560)
 	/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/cmd.go:73 +0x192
 main.main()

Suggests that the nil pointer dereference happens here in init.go:

flagSet.DurationVar(
		&cfg.TokenTTL.Duration, "token-ttl", cfg.TokenTTL.Duration,
		"The duration before the bootstrap token is automatically deleted. 0 means 'never expires'.",
	)

There also appears to be a i.cfg.TokenTTL.Duration deference in the Run function further down in the same file:

if err := nodebootstraptokenphase.UpdateOrCreateToken(client, i.cfg.Token, false, i.cfg.TokenTTL.Duration, kubeadmconstants.DefaultTokenUsages, []string{kubeadmconstants.NodeBootstrapTokenAuthGroup}, tokenDescription); err != nil {
...

fejta-bot · 2017-11-07T08:36:33Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-11-07T11:24:32Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

k8s-github-robot · 2017-11-07T11:58:08Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @dmmcquay @luxas @mattmoyer @timothysc

mattmoyer · 2017-11-07T12:00:11Z

@jpbetz I think found the problem, we were missing an import _ "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/install" that we needed to make the kubeadm CLI build trigger registering the MasterConfiguration/NodeConfiguration defaulters into the global registry.

I added the import in cmd/kubeadm/app/cmd/cmd.go and it resolved the verify problem for me locally.

This was broken because the API machinery defaulting mechanism couldn't differentiate between an unset value (which should default to 24 hours) and a value explicitly set to 0 (which should mean infinite). The fix is to change `TokenTTL` from a `metav1.Duration` to `*metav1.Duration` so that `nil` can represent the unspecified value. This bug was introduced in kubernetes#48783.

jpbetz · 2017-11-07T14:34:01Z

@mattmoyer Great, merged.

k8s-github-robot assigned timothysc and dmmcquay Oct 26, 2017

mattmoyer mentioned this pull request Oct 26, 2017

make --token-ttl 0 effect #53805

Closed

mattmoyer force-pushed the kubeadm-fix-init-token-ttl branch 3 times, most recently from 8af195a to 6c6f421 Compare October 26, 2017 18:47

mattmoyer mentioned this pull request Oct 26, 2017

Automated cherry pick of #54640 #54649

Merged

dmmcquay approved these changes Oct 26, 2017

View reviewed changes

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

k8s-ci-robot assigned luxas Oct 27, 2017

luxas approved these changes Oct 27, 2017

View reviewed changes

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

luxas added cherrypick-candidate and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 27, 2017

luxas added this to the v1.8 milestone Oct 27, 2017

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

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 1, 2017

k8s-github-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 1, 2017

jpbetz added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. labels Nov 6, 2017

jpbetz mentioned this pull request Nov 7, 2017

Fix nil pointer dereference in #54640 #55220

Closed

mattmoyer force-pushed the kubeadm-fix-init-token-ttl branch from 6c6f421 to c65dd85 Compare November 7, 2017 11:57

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2017

mattmoyer force-pushed the kubeadm-fix-init-token-ttl branch 2 times, most recently from 16626da to 6d49976 Compare November 7, 2017 12:35

mattmoyer force-pushed the kubeadm-fix-init-token-ttl branch from 6d49976 to 8ab898f Compare November 7, 2017 13:25

jpbetz merged commit a00340a into kubernetes:master Nov 7, 2017

mattmoyer deleted the kubeadm-fix-init-token-ttl branch November 7, 2017 14:34

vhosakot mentioned this pull request Dec 20, 2017

Fixed kubelet RPM and kubeadm token expiry issue contiv/install#327

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `kubeadm init --token-ttl=0`/config `tokenTTL: "0"`. #54640

Fix `kubeadm init --token-ttl=0`/config `tokenTTL: "0"`. #54640

mattmoyer commented Oct 26, 2017 •

edited

Loading

mattmoyer commented Oct 26, 2017

dmmcquay commented Oct 26, 2017

luxas left a comment

k8s-github-robot commented Oct 27, 2017

mattmoyer commented Oct 27, 2017

luxas commented Oct 27, 2017

luxas commented Oct 27, 2017

mattmoyer commented Oct 27, 2017 •

edited

Loading

luxas commented Oct 27, 2017

fejta-bot commented Nov 1, 2017

mattmoyer commented Nov 1, 2017

luxas commented Nov 1, 2017

k8s-github-robot commented Nov 1, 2017

timothysc commented Nov 6, 2017

mattmoyer commented Nov 6, 2017

fejta-bot commented Nov 7, 2017

fejta-bot commented Nov 7, 2017

jpbetz commented Nov 7, 2017 •

edited

Loading

fejta-bot commented Nov 7, 2017

fejta-bot commented Nov 7, 2017

k8s-github-robot commented Nov 7, 2017

mattmoyer commented Nov 7, 2017 •

edited

Loading

jpbetz commented Nov 7, 2017

Fix kubeadm init --token-ttl=0/config tokenTTL: "0". #54640

Fix kubeadm init --token-ttl=0/config tokenTTL: "0". #54640

Conversation

mattmoyer commented Oct 26, 2017 • edited Loading

mattmoyer commented Oct 26, 2017

dmmcquay commented Oct 26, 2017

luxas left a comment

Choose a reason for hiding this comment

k8s-github-robot commented Oct 27, 2017

mattmoyer commented Oct 27, 2017

luxas commented Oct 27, 2017

luxas commented Oct 27, 2017

mattmoyer commented Oct 27, 2017 • edited Loading

luxas commented Oct 27, 2017

fejta-bot commented Nov 1, 2017

mattmoyer commented Nov 1, 2017

luxas commented Nov 1, 2017

k8s-github-robot commented Nov 1, 2017

timothysc commented Nov 6, 2017

mattmoyer commented Nov 6, 2017

fejta-bot commented Nov 7, 2017

fejta-bot commented Nov 7, 2017

jpbetz commented Nov 7, 2017 • edited Loading

fejta-bot commented Nov 7, 2017

fejta-bot commented Nov 7, 2017

k8s-github-robot commented Nov 7, 2017

mattmoyer commented Nov 7, 2017 • edited Loading

jpbetz commented Nov 7, 2017

Fix `kubeadm init --token-ttl=0`/config `tokenTTL: "0"`. #54640

Fix `kubeadm init --token-ttl=0`/config `tokenTTL: "0"`. #54640

mattmoyer commented Oct 26, 2017 •

edited

Loading

mattmoyer commented Oct 27, 2017 •

edited

Loading

jpbetz commented Nov 7, 2017 •

edited

Loading

mattmoyer commented Nov 7, 2017 •

edited

Loading