Skip to content
This repository was archived by the owner on Jun 29, 2022. It is now read-only.

bootkube: use host's /etc/hosts entries#409

Merged
invidian merged 1 commit intomasterfrom
invidian/bootkube-rkt-host-etc-hosts
Aug 18, 2020
Merged

bootkube: use host's /etc/hosts entries#409
invidian merged 1 commit intomasterfrom
invidian/bootkube-rkt-host-etc-hosts

Conversation

@invidian
Copy link
Member

Currently, bare metal environment requires resolvable DNS names for API
server and etcd servers. For user without running DNS server locally or
for testing, it would be nice to be able have some workaround for it.
One would be to populate /etc/hosts of nodes with required entries.

This is almost possible, using CLC snippets for controller nodes, except
the bootkube, which runs in rkt container and have it's own /etc/hosts
file, so health check on static kube-apiserver pod never succeeds. To
make it work, rkt parameter --hosts-entry=host needs to be added.

I suggest we add --hosts-entry=host to bootkube, as potential breakage
impact is minimal and this will save as from exposing general
bootkube_rkt_extra_args in the Terraform module/lokocfg.

By the issue, it is not required to be applied on all platforms, but
again, the breakage chance seems minimal and generally we should be
heading towards having quite unified configurations across all the
platforms, so this commit adds it to all of them.

Closes #408

Signed-off-by: Mateusz Gozdek mateusz@kinvolk.io

@invidian invidian force-pushed the invidian/bootkube-rkt-host-etc-hosts branch 2 times, most recently from 191810d to 8b0547d Compare May 11, 2020 09:59
quay.io/kinvolk/bootkube:v0.14.0-helm-amd64 \
--net=host \
--dns=host \
--hosts-entry=host \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we verified this doesn't have side effects? Also, I believe you're proposing to add this flag to all platforms for the sake of uniformity, however does it make sense to do that on platforms which don't need this functionality? I think it might be better to only add the flag where it's needed, to minimize potential "surprises" in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we verified this doesn't have side effects?

I cannot come up with any possible side effects of that beside benefits. This potentially allows to also use CLC snippets for /etc/hosts for names resolving for other platforms too (if using only Terraform code, which I know we don't support).

@iaguis
Copy link
Contributor

iaguis commented Jun 25, 2020

The bootkube container is running with host networking so it makes sense it also uses the host's /etc/hosts. So I'm fine with this change, can you rebase?

@invidian invidian force-pushed the invidian/bootkube-rkt-host-etc-hosts branch 2 times, most recently from 77f9a87 to a71cbcd Compare June 25, 2020 09:56
@invidian invidian force-pushed the invidian/bootkube-rkt-host-etc-hosts branch from a71cbcd to 358210a Compare August 3, 2020 09:38
@invidian
Copy link
Member Author

invidian commented Aug 3, 2020

The bootkube container is running with host networking so it makes sense it also uses the host's /etc/hosts. So I'm fine with this change, can you rebase?

Rebased. Please have a look.

@iaguis iaguis force-pushed the invidian/bootkube-rkt-host-etc-hosts branch from 358210a to fd1fb9f Compare August 17, 2020 14:15
Currently, bare metal environment requires resolvable DNS names for API
server and etcd servers. For user without running DNS server locally or
for testing, it would be nice to be able have some workaround for it.
One would be to populate /etc/hosts of nodes with required entries.

This is almost possible, using CLC snippets for controller nodes, except
the bootkube, which runs in rkt container and have it's own /etc/hosts
file, so health check on static kube-apiserver pod never succeeds. To
make it work, rkt parameter --hosts-entry=host needs to be added.

I suggest we add --hosts-entry=host to bootkube, as potential breakage
impact is minimal and this will save as from exposing general
bootkube_rkt_extra_args in the Terraform module/lokocfg.

By the issue, it is not required to be applied on all platforms, but
again, the breakage chance seems minimal and generally we should be
heading towards having quite unified configurations across all the
platforms, so this commit adds it to all of them.

Closes #408

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@iaguis iaguis force-pushed the invidian/bootkube-rkt-host-etc-hosts branch from fd1fb9f to 7c46641 Compare August 18, 2020 07:54
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased this again and it looks good to me.

@invidian
Copy link
Member Author

Thanks @iaguis. Will merge once CI passes.

@invidian invidian merged commit 524a81d into master Aug 18, 2020
@invidian invidian deleted the invidian/bootkube-rkt-host-etc-hosts branch August 18, 2020 09:34
@invidian invidian mentioned this pull request Jan 22, 2021
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change bootkube rkt container to use host's /etc/hosts entries

3 participants