Kubelet tls bootstrap #2809

abhinavdahiya · 2018-01-24T01:36:35Z

Enable kubelet auto TLS bootstrapping.

Generates kubeconfig-kublet from a bootstrapping token for kubelet to generate CSRs.
Current implementation uses auto-approve all nodes.
Requires ca.crt and ca.key to sign CSRs generated by nodes. (handled by kube-controller-mananger)

(kubernetes-retired/bootkube#663)
(https://kubernetes.io/docs/admin/kubelet-tls-bootstrapping/)

cc @aaronlevy @diegs

coreosbot · 2018-01-24T01:36:37Z

Can one of the admins verify this patch?

diegs

Amazing work!

diegs · 2018-01-24T01:38:05Z

config.tf

-    bootkube                     = "quay.io/coreos/bootkube:v0.8.1"
-    etcd                         = "quay.io/coreos/etcd:v3.2.14"
+    bootkube                     = "quay.io/coreos/bootkube:v0.10.0"
+    etcd                         = "quay.io/coreos/etcd:v3.1.8"


why the lower etcd version?

oh, fixed that.

diegs · 2018-01-24T01:38:41Z

contrib/user-provided-certs/kube/self-signed/admin.tf

@@ -0,0 +1,43 @@
+# admin
+# Admin (generated/tls/{admin.key,admin.crt})
+# Used to create kubeconfig (generated/auth/kubeconfig) with admin level priviledges.


spelling: privileges

diegs · 2018-01-24T01:40:58Z

modules/tls/kube/self-signed/admin.tf

-# Kubelet
-resource "tls_private_key" "kubelet" {
+# Admin (generated/tls/{admin.key,admin.crt})
+# Used to create kubeconfig (generated/auth/kubeconfig) with admin level priviledges.


diegs · 2018-01-24T01:42:14Z

modules/bootkube/assets.tf

@@ -45,11 +61,31 @@ resource "local_file" "kubeconfig" {
  filename = "./generated/auth/kubeconfig"


I would consider renaming this one too (to kubeconfig-admin) to make it really clear that you shouldn't use it for anything in the cluster. Others might have different feelings though.

bootkube start expects the admin kubeconfig to exist at this path.
https://github.com/kubernetes-incubator/bootkube/blob/master/pkg/bootkube/bootstrap.go#L38-L41

Heh, look at the fool who left that TODO there :-/

may be just add a comment on the kubeconfig file if that does not break any consumer?

diegs · 2018-01-24T01:43:31Z

cc @ericchiang

coreosbot · 2018-01-24T02:15:42Z

Can one of the admins verify this patch?

coreosbot · 2018-01-24T02:15:58Z

Can one of the admins verify this patch?

diegs · 2018-01-24T02:16:09Z

ok to test

diegs · 2018-01-24T02:16:34Z

closed and reopened to trigger smoke tests.

enxebre · 2018-01-24T09:21:08Z

modules/ignition/resources/services/kubelet.service

+  --cni-bin-dir=/var/lib/cni/bin \
+  --cni-conf-dir=/etc/kubernetes/cni/net.d \
+  --exit-on-lock-contention \
+  --kubeconfig=/etc/kubernetes/kubeconfig \


Why not?:

--kubeconfig=/path/to/to-be-generated-kubeconfig --require-kubeconfig --bootstrap-kubeconfig="/path/to/bootstrap/kubeconfig"

I believe he alphabetized them.

I meant what's the reasoning to not use the --bootstrap-kubeconfig flag and then letting the kubelet write the new config on approval in the path pointed by --kubeconfig as suggested here https://kubernetes.io/docs/admin/kubelet-tls-bootstrapping/#kubelet-configuration.

kubernetes-retired/bootkube#663 (comment)

enxebre · 2018-01-24T11:21:35Z

Hey @abhinavdahiya After a quick run, secrets check-pointing seems to be failing:

 Failed to checkpoint secrets for pod kube-system/kube-apiserver-z7d2m: failed to checkpoint secret for pod kube-system/kube-apiserver-z7d2m: failed to retrieve secret kube-system/kube-apiserver: secrets "kube-apiserver" is forbidden: User "system:bootstrap:1xombd" cannot get secrets in the namespace "kube-system"

Sound like the checkpointer using the kubelet bootstrapping kubeconfig?
If so it seems potentially related to https://github.com/coreos/tectonic-installer/pull/2809/files#r163490474 if we use those flags the checkpointer could pick the valid kubeconfig.
Following a different path it seems we are just missing the role/service-account for the checkpointer on the kube_core_renderer, this should fix it https://github.com/coreos-inc/tectonic-operators/pull/227 cc @diegs

enxebre · 2018-01-24T15:24:37Z

modules/tls/kube/self-signed/outputs.tf

@@ -33,8 +33,8 @@ output "id" {
    local_file.apiserver_crt.id,
    local_file.kube_ca_key.id,
    local_file.kube_ca_crt.id,
-    local_file.kubelet_key.id,


This needs (along with everything else) to be kept in sync within contrib/user-provided-certs.
@abhinavdahiya We are in the process of decoupling the tls generation from the installer. Eventually we'll rely on modules/tls/user-provided to just wire the previously generated certificates into the installer.
As a transitional step for automation purposes we plan to rely on contrib/user-provided-certs to pre-generate the certificates before moving to a more suitable tool.

@enxebre I think i have synced all these changes to contrib/user-provided-certs in this PR iteself .
(771525b#diff-1e46a06517303b3aa2caeddc2865efb5)
(771525b#diff-266340c2b198d2324636d1e007ce3a66)
(771525b#diff-a870b0054dd4ceba57305ced8ff8b116)

Is anything else required?

coreosbot · 2018-01-25T01:05:40Z

Can one of the admins verify this patch?

cpanato · 2018-01-26T09:07:33Z

retest this please. govcloud only

cpanato · 2018-01-26T10:02:29Z

retest this please. aws only

cpanato · 2018-01-26T10:55:31Z

retest this please. metal only

cpanato · 2018-01-26T12:21:56Z

all tests are green. aws/azure/metal/govcloud.

alexsomesan · 2018-01-29T16:58:04Z

config.tf

    etcd                         = "quay.io/coreos/etcd:v3.2.14"
    etcd_operator                = "quay.io/coreos/etcd-operator:v0.5.0"
    hyperkube                    = "quay.io/coreos/hyperkube:v1.9.1_coreos.0"
-    kube_core_renderer           = "quay.io/coreos/kube-core-renderer:beryllium-m1"
+    kube_core_renderer           = "quay.io/coreos/kube-core-renderer-dev:6c49ce4da9fc36966812381891b4f558aa53097b"


I guess there will be a release of kube-core-renderer before this change needs to merge, so that we don't have to carry the dev tag into installer's master?

//cc: @diegs

Is there a serious problem with using dev images on master? Then we cut over to release images for release branches?

I don't have any concerns myself as long as we track the follow-up to catch up with the future release.

Let's go forward with that for now then.

abhinavdahiya · 2018-02-02T17:12:02Z

friendly ping :)

cpanato · 2018-02-02T17:30:29Z

@abhinavdahiya can you please rebase? then I can check the tests. thanks!

enxebre · 2018-02-02T17:32:35Z

retest this please

abhinavdahiya · 2018-02-02T18:42:47Z

@cpanato rebased against master

abhinavdahiya · 2018-02-06T23:59:39Z

@cpanato rebased against master as after #2850

abhinavdahiya · 2018-02-08T22:51:58Z

any updates on this. :)

diegs · 2018-02-08T22:59:13Z

retest this please. Needs to re-run after rebase with all platforms.

abhinavdahiya · 2018-02-09T17:27:11Z

@diegs all green

alexsomesan

Looks good to me.
All issues raised in conversations seem to have satisfactory solutions / explanations.

abhinavdahiya requested review from diegs, sym3tri and aaronlevy January 24, 2018 01:36

diegs previously approved these changes Jan 24, 2018

View reviewed changes

abhinavdahiya dismissed diegs’s stale review via 74190e9 January 24, 2018 01:51

abhinavdahiya force-pushed the kubelet-tls-bootstrap branch from ef28504 to 74190e9 Compare January 24, 2018 01:51

diegs added platform/aws platform/bare-metal run-smoke-tests labels Jan 24, 2018

diegs closed this Jan 24, 2018

diegs reopened this Jan 24, 2018

diegs closed this Jan 24, 2018

diegs reopened this Jan 24, 2018

enxebre reviewed Jan 24, 2018

View reviewed changes

abhinavdahiya force-pushed the kubelet-tls-bootstrap branch 4 times, most recently from a76dc9b to 004fef4 Compare January 24, 2018 22:47

abhinavdahiya added the platform/azure label Jan 24, 2018

abhinavdahiya closed this Jan 25, 2018

abhinavdahiya reopened this Jan 25, 2018

cpanato added platform/aws and removed platform/govcloud labels Jan 26, 2018

cpanato added platform/bare-metal and removed platform/aws labels Jan 26, 2018

abhinavdahiya force-pushed the kubelet-tls-bootstrap branch from 500a60c to d08c244 Compare January 26, 2018 18:07

sym3tri assigned alexsomesan and squat Jan 29, 2018

alexsomesan reviewed Jan 29, 2018

View reviewed changes

abhinavdahiya force-pushed the kubelet-tls-bootstrap branch from d08c244 to bf78ad0 Compare February 2, 2018 18:19

abhinavdahiya force-pushed the kubelet-tls-bootstrap branch 2 times, most recently from 0190675 to f4c51c6 Compare February 6, 2018 23:59

kubelet tls bootstrap

6d2c21a

abhinavdahiya force-pushed the kubelet-tls-bootstrap branch from f4c51c6 to 6d2c21a Compare February 7, 2018 19:39

diegs added platform/azure platform/aws platform/govcloud labels Feb 8, 2018

alexsomesan approved these changes Feb 12, 2018

View reviewed changes

alexsomesan merged commit 41126a3 into coreos:master Feb 12, 2018

		@@ -45,11 +61,31 @@ resource "local_file" "kubeconfig" {
		filename = "./generated/auth/kubeconfig"

Kubelet tls bootstrap #2809

Kubelet tls bootstrap #2809

Conversation

abhinavdahiya commented Jan 24, 2018 • edited Loading

coreosbot commented Jan 24, 2018

diegs 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enxebre Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

diegs commented Jan 24, 2018

coreosbot commented Jan 24, 2018

coreosbot commented Jan 24, 2018

diegs commented Jan 24, 2018

diegs commented Jan 24, 2018

enxebre Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enxebre Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enxebre commented Jan 24, 2018 • edited Loading

enxebre Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

abhinavdahiya Jan 24, 2018 • edited Loading

Choose a reason for hiding this comment

coreosbot commented Jan 25, 2018

cpanato commented Jan 26, 2018

cpanato commented Jan 26, 2018

cpanato commented Jan 26, 2018

cpanato commented Jan 26, 2018

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

abhinavdahiya commented Feb 2, 2018

cpanato commented Feb 2, 2018

enxebre commented Feb 2, 2018

abhinavdahiya commented Feb 2, 2018 • edited Loading

abhinavdahiya commented Feb 6, 2018

abhinavdahiya commented Feb 8, 2018

diegs commented Feb 8, 2018

abhinavdahiya commented Feb 9, 2018

alexsomesan left a comment

Choose a reason for hiding this comment

abhinavdahiya commented Jan 24, 2018 •

edited

Loading

enxebre Jan 24, 2018 •

edited

Loading

enxebre Jan 24, 2018 •

edited

Loading

enxebre Jan 24, 2018 •

edited

Loading

enxebre commented Jan 24, 2018 •

edited

Loading

enxebre Jan 24, 2018 •

edited

Loading

abhinavdahiya Jan 24, 2018 •

edited

Loading

abhinavdahiya commented Feb 2, 2018 •

edited

Loading