Skip to content
This repository was archived by the owner on Feb 5, 2020. It is now read-only.

Conversation

@lucab
Copy link
Contributor

@lucab lucab commented Aug 11, 2017

This updates all platforms to write the initial kubelet.env under
an installer-dedicated directory /etc/kubernetes/installer/, in
order to differentiate between the file dropped by terraform/ignition
and the one written by a bootstrap agent.

It is a prerequisite for OST-23.

@lucab lucab force-pushed the ups/installer-kubelet-env branch from 893b7e3 to be4ca90 Compare August 11, 2017 12:01
@lucab lucab force-pushed the ups/installer-kubelet-env branch 6 times, most recently from 07815e9 to ac452a5 Compare August 11, 2017 14:51
@squat
Copy link
Contributor

squat commented Aug 16, 2017

@lucab do you need to update the kubelet.service units to load this new envfile [0]? Otherwise, I think this change will make no difference in booted clusters.

[0] https://github.com/coreos/tectonic-installer/blob/master/modules/aws/ignition/resources/services/kubelet.service#L5

@lucab
Copy link
Contributor Author

lucab commented Aug 16, 2017

@squat no, that will be done by a dedicate bootstrapper. See the TODO line in this PR and the linked Jira ticket.

@squat
Copy link
Contributor

squat commented Aug 16, 2017

@lucab got it. looks good

squat
squat previously approved these changes Aug 16, 2017
@lucab
Copy link
Contributor Author

lucab commented Aug 17, 2017

@s-urbaniak would prefer to hold this for one more iteration, in order to try avoiding introducing more assets duplication.

@alexsomesan
Copy link
Contributor

Looks really cool. I like the approach!
Spoke to @s-urbaniak off-line about keeping this open for further testing.

@lucab lucab force-pushed the ups/installer-kubelet-env branch from 1101ebe to 3b077ec Compare August 18, 2017 12:44
@s-urbaniak
Copy link
Contributor

retest this

@s-urbaniak
Copy link
Contributor

ok to test

@s-urbaniak s-urbaniak closed this Aug 18, 2017
@s-urbaniak s-urbaniak reopened this Aug 18, 2017
@lucab lucab force-pushed the ups/installer-kubelet-env branch from 3b077ec to 339a8fb Compare August 18, 2017 14:22
@Quentin-M
Copy link
Contributor

Except for the bootkube master node, this file must not exist so that kubelet.service is blocked until another service queries an available API server for the version to use (e.g. the bootstrapping API server).

The process of launching bootkube+writing the env file on bootkube master, or querying the bootstraping API server+writing the env file is generic to all platforms. We worked a lot on simplifying the node states, the amount of configuration/files/services needed on each of them and the bootstrapping flow. I'd advocate for continuing to keep this as simple as possible and unifying the ignition files for all platforms.

Adding an extra file /etc/kubernetes/installer/kubelet.env on every nodes, that is then moved/copied/used conditionally does not sound too appealing. I'd rather have the actual env file always written by ignition - but overwrote by the kubelet service if an API server is available to give us the data. Worse case scenario, if we consider that no bootkube master election is available, there could be a time-out on the process of retrieving the version to use. The fact that all kubelet services will fail to retrieve the version to use on first cluster boot is acceptable as we already know that a cluster we are just bootstrapping will already have the appropriate kubelet version. For new nodes, the query will go through and the file will be updated before kubelet starts.

However, note that the interaction with the node agent is sensitive too and would have to be taken into consideration.

/cc @yifan-gu (who worked on the initial solution with me)

@lucab lucab force-pushed the ups/installer-kubelet-env branch 2 times, most recently from f7d4abe to e1ea1e2 Compare August 23, 2017 12:45
@s-urbaniak
Copy link
Contributor

ok to test

@lucab lucab force-pushed the ups/installer-kubelet-env branch 3 times, most recently from e8baea2 to cfc8183 Compare August 23, 2017 16:28
@lucab
Copy link
Contributor Author

lucab commented Aug 24, 2017

The module/ignition refactoring is now happening in #1743. I'll keep rebasing this until the other lands.

@lucab lucab force-pushed the ups/installer-kubelet-env branch 6 times, most recently from a15eaa3 to f03d13c Compare August 28, 2017 12:44
This updates all platforms to write the initial `kubelet.env` under
an installer-dedicated directory `/etc/kubernetes/installer/`, in
order to differentiate between the file dropped by terraform/ignition
and the one written by a bootstrap agent.

It is a prerequisite for OST-23.
@lucab lucab force-pushed the ups/installer-kubelet-env branch from f03d13c to b7ebf38 Compare August 29, 2017 08:12
@lucab
Copy link
Contributor Author

lucab commented Aug 30, 2017

Most of the pre-requisite work here landed in #1743. Closing this, I'll keep torcx changes in #1710.

@lucab lucab closed this Aug 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants