modules/ignition: unify kubelet/docker bootstrap via tectonic-torcx #1710

lucab · 2017-08-17T10:22:30Z

This PR unifies the bootstrapping process of new nodes, taking care of both kubelet (now) and docker (future CL releases, via torcx).
It introduces a new k8s-node-bootstrap.service, replacing and augmenting kubelet-env.service.
The full bootstrap flow is now documented at node-bootstrap-flow.md (see rendered version).

Summary of the changes:

drop kubelet-env.service and kubelet.env
introduce k8s-node-bootstrap.service
introduce /etc/kubernetes/installer/kubelet.env

/cc @squeed

alekssaul · 2017-08-24T15:02:20Z

platforms/vmware/main.tf

  private_key             = "${var.tectonic_vmware_ssh_private_key_path}"
  image_re                = "${var.tectonic_image_re}"
+
+  ign_installer_kubelet_env_id = "${module.ignition.installer_kubelet_env_id}"


I believe this line is supposed to go under module "masters" instead of etcd?

It is already under "masters", github folding is tricking you into believing it's under "etcd" instead ;)

darn you github!

cpanato · 2017-09-15T09:11:09Z

close/open to refresh the labels

sudhaponnaganti · 2017-09-15T19:26:54Z

This is a release blocker

lucab · 2017-09-18T13:09:24Z

This is mostly green (I think Azure failure is a flake in the cloud API) and ready for review.

s-urbaniak · 2017-09-18T14:55:40Z

modules/ignition/assets.tf

+    kubeconfig_fetch_cmd     = "${var.kubeconfig_fetch_cmd != "" ? "ExecStartPre=${var.kubeconfig_fetch_cmd}" : ""}"
+    tectonic_torcx_image_url = "${replace(var.container_images["tectonic_torcx"],var.image_re,"$1")}"
+    tectonic_torcx_image_tag = "${replace(var.container_images["tectonic_torcx"],var.image_re,"$2")}"
+    torcx_skip_setup         = "${var.tectonic_vanilla_k8s == 1 ? "true" : "false" }"


${var.tectonic_vanilla_k8s ? "true" : "false" }

s-urbaniak · 2017-09-18T14:56:51Z

modules/ignition/variables.tf

+}
+
+variable "tectonic_vanilla_k8s" {
+  default = false


let's remove default here to enforce variable declaration.

s-urbaniak

two nits, else LGTM once green, thanks! That's a big step :-)

s-urbaniak · 2017-09-18T14:59:42Z

config.tf

    tectonic_etcd_operator       = "quay.io/coreos/tectonic-etcd-operator:v0.0.2"
    tectonic_prometheus_operator = "quay.io/coreos/tectonic-prometheus-operator:v1.6.0"
    tectonic_cluo_operator       = "quay.io/coreos/tectonic-cluo-operator:v0.2.0"
+    tectonic_torcx               = "quay.io/coreos/tectonic-torcx:installer-latest"


installer-latest: do we want this as a final value?

Yes, this is a mutable tag dedicated to the installer. We need this here for forward compatibility until some higher level component takes care of the kubernetes->docker mapping.
@robszumski has a tracker item for this somewhere.

lucab · 2017-09-18T15:27:26Z

@s-urbaniak thanks for the review, nits addressed, CI ongoing.

Quentin-M · 2017-09-18T20:43:24Z

This cannot be merged as is because all tests are red.

aaronlevy · 2017-09-18T20:54:02Z

cc @yifan-gu

lucab · 2017-09-18T22:17:44Z

@Quentin-M sorry, it was fine before and I mistakenly broke it when addressing review comments. Amended.

Quentin-M · 2017-09-18T22:18:24Z

Thank you so much!

diegs · 2017-09-18T23:00:03Z

Documentation/dev/node-bootstrap-flow.md

+
+Additionally, only on one of the master node the following kubernetes bootstrapping happens:
+
+* `bootkube.service` deploys the initial bootstrapping control-plane. It is started only after ``kubelet.service` _is started_. It is a oneshot unit and cannot crash, and it runs only during bootstrap


nit: double back-tick on kubelet.service

diegs · 2017-09-18T23:00:20Z

Documentation/dev/node-bootstrap-flow.md

+* `k8s-node-bootstrap.service` ensures node and assets freshness. It is automatically started on boot, can crash-loop, and it runs only during bootstrap
+* `kubelet.service` is the main kubelet deamon. It is automatically started on boot, it is crash-looping until `kubelet.env` is populated, and it runs on each boot
+
+Additionally, only on one of the master node the following kubernetes bootstrapping happens:


nit: one of the master nodes

Quentin-M · 2017-09-18T23:08:26Z

Pushed to fix conflict in config.tf.. but looks like there are some new comments anyways. We can wait until tomorrow then.

diegs · 2017-09-18T23:10:07Z

@Quentin-M mine are just nits, they aren't release blocking and can be fixed in a follow-up PR.

yifan-gu · 2017-09-18T23:28:45Z

modules/ignition/resources/services/k8s-node-bootstrap.service

@@ -0,0 +1,36 @@
+[Unit]
+Description=Determine the Kubelet Image Version


nit: It does more than that now right? @lucab

This introduces a new k8s-node-bootstrap.service, replacing and augmenting kubelet-env.service. The full bootstrap flow is now documented at [Documentation/dev/node-bootstrap-flow.md] Summary of the changes: * drop kubelet-env.service and kubelet.env * introduce k8s-node-bootstrap.service * introduce /etc/kubernetes/installer/kubelet.env

lucab · 2017-09-19T12:24:21Z

@diegs thanks for proofreading!

@yifan-gu yes, it does docker setup (via torcx) and kubelet.env setup (similar to previous kube-version). I renamed the service but forgot to update the description.

@Quentin-M thanks for the push, it hit some flakes anyway. I rebased once more for the remaining nits, no flakes this time.

lucab force-pushed the ups/torcx-bootstrapper branch from 7297eaa to b202b42 Compare August 18, 2017 16:52

alekssaul reviewed Aug 24, 2017

View reviewed changes

lucab force-pushed the ups/torcx-bootstrapper branch 5 times, most recently from c178b50 to 16ea9e6 Compare August 30, 2017 09:24

lucab mentioned this pull request Aug 30, 2017

tf/common: write an installer-dedicated kubelet.env #1659

Closed

lucab force-pushed the ups/torcx-bootstrapper branch 7 times, most recently from 8c0a567 to 1d3daf7 Compare September 7, 2017 12:21

lucab force-pushed the ups/torcx-bootstrapper branch from 1d3daf7 to 02546e9 Compare September 7, 2017 13:58

lucab force-pushed the ups/torcx-bootstrapper branch 2 times, most recently from 403447a to f6b286c Compare September 15, 2017 07:26

cpanato added the run-smoke-tests label Sep 15, 2017

cpanato closed this Sep 15, 2017

cpanato reopened this Sep 15, 2017

lucab force-pushed the ups/torcx-bootstrapper branch 3 times, most recently from 1f92f9b to 582ad93 Compare September 15, 2017 15:44

lucab changed the title ~~[WIP] tf/common: unify kubelet/docker bootstrap via tectonic_torcx~~ modules/ignition: unify kubelet/docker bootstrap via tectonic-torcx Sep 15, 2017

lucab force-pushed the ups/torcx-bootstrapper branch from 582ad93 to 375fc03 Compare September 15, 2017 18:04

sudhaponnaganti added the release-blocker label Sep 15, 2017

lucab force-pushed the ups/torcx-bootstrapper branch from 375fc03 to f2b8a8e Compare September 17, 2017 11:50

lucab force-pushed the ups/torcx-bootstrapper branch 2 times, most recently from d26ee0b to c2d9cfb Compare September 18, 2017 11:43

s-urbaniak reviewed Sep 18, 2017

View reviewed changes

s-urbaniak previously approved these changes Sep 18, 2017

View reviewed changes

s-urbaniak reviewed Sep 18, 2017

View reviewed changes

lucab dismissed s-urbaniak’s stale review via de03697 September 18, 2017 15:26

lucab force-pushed the ups/torcx-bootstrapper branch from c2d9cfb to de03697 Compare September 18, 2017 15:26

lucab force-pushed the ups/torcx-bootstrapper branch from de03697 to b32d36b Compare September 18, 2017 21:57

diegs reviewed Sep 18, 2017

View reviewed changes

Quentin-M force-pushed the ups/torcx-bootstrapper branch from b32d36b to eb88abd Compare September 18, 2017 23:05

yifan-gu reviewed Sep 18, 2017

View reviewed changes

docs/dev: document new bootstrap flow

3988fa1

lucab force-pushed the ups/torcx-bootstrapper branch from eb88abd to 1e7b1ca Compare September 19, 2017 07:54

lucab force-pushed the ups/torcx-bootstrapper branch from 1e7b1ca to 3fc7170 Compare September 19, 2017 10:58

alexsomesan approved these changes Sep 19, 2017

View reviewed changes

s-urbaniak approved these changes Sep 19, 2017

View reviewed changes

s-urbaniak merged commit 0cf916e into coreos:master Sep 19, 2017


		Additionally, only on one of the master node the following kubernetes bootstrapping happens:

		* `bootkube.service` deploys the initial bootstrapping control-plane. It is started only after ``kubelet.service` _is started_. It is a oneshot unit and cannot crash, and it runs only during bootstrap

		@@ -0,0 +1,36 @@
		[Unit]
		Description=Determine the Kubelet Image Version

modules/ignition: unify kubelet/docker bootstrap via tectonic-torcx #1710

modules/ignition: unify kubelet/docker bootstrap via tectonic-torcx #1710

Uh oh!

Conversation

lucab commented Aug 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpanato commented Sep 15, 2017

Uh oh!

sudhaponnaganti commented Sep 15, 2017

Uh oh!

lucab commented Sep 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

s-urbaniak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucab commented Sep 18, 2017

Uh oh!

Quentin-M commented Sep 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronlevy commented Sep 18, 2017

Uh oh!

lucab commented Sep 18, 2017

Uh oh!

Quentin-M commented Sep 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Quentin-M commented Sep 18, 2017

Uh oh!

diegs commented Sep 18, 2017

Uh oh!

yifan-gu Sep 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucab commented Sep 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

lucab commented Aug 17, 2017 •

edited

Loading

Quentin-M commented Sep 18, 2017 •

edited

Loading

yifan-gu Sep 18, 2017 •

edited

Loading