Move api-int record from coredns to /etc/hosts #2236
Conversation
This is to enable more flexibility about when the networking services are deployed. With the api-int record in /etc/hosts we don't need coredns to be running for a node to connect to the cluster.
openshift-ci-robot
requested review from
kikisdeliveryservice and
yuqi-zhang
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cybertron The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/test e2e-openstack |
| @@ -0,0 +1,7 @@ | |||
| mode: 0644 | |||
| path: "/etc/hosts" | |||
There was a problem hiding this comment.
People may have legitimately added custom content in here before, I don't think we can unilaterally own this file - particularly on upgrades.
I approve the general idea (this also seems related to #2190 right?).
It's too bad there's not a simple /etc/hosts.d but I guess everyone who wants this ends up using a local resolver.
So that's one option...we could switch to on-host dnsmasq or resolved.
Or we could try to hack in some sort of Ansible-like "lineinfile" support to the MCO just for this.
There was a problem hiding this comment.
This is primarily targeted at enabling openshift/enhancements#524
The problem with using dnsmasq for this is that we already run a local DNS server for the other records. It's possible we could make it work, but it gets tricky and makes the DNS flow even more complicated. Right now we configure a local server that forwards to the previously configured servers in resolv.conf. This would add another dnsmasq somewhere in that flow. I guess if there's no other way to do it then we'll have to eat the complexity, but it's not ideal either.
I also did look at using the append option in ignition and had that working. It's a weird fit for MCO though and I couldn't get it to work so I switched to just templating the whole file. It's another thing I could investigate further though.
There was a problem hiding this comment.
OK I see. Hum...do we have any offhand guesses around whether my concern about customers injecting /etc/hosts via Ignition is a real concern? It may not happen. If it does though, what would we tell them for configuring static resolution?
Perhaps we can implement our own "merge /etc/hosts" logic by having code in the MCO that on startup takes the current content, appends this data to it into e.g. /run/hosts and then does mount --bind /run/hosts /etc/hosts or so.
That said it feels like what we really want to achieve here is switch over the host's resolver to coredns but only if coredns is active and working? Basically if coredns is up, update /etc/resolv.conf to use 127.0.0.1:53. It's a bit like where we ended up with #2011 - we have a systemd unit which monitors a pod, and if the pod is down takes action.
There was a problem hiding this comment.
People may have legitimately added custom content in here before, I don't think we can unilaterally own this file - particularly on upgrades.
The DNS operator adds an entry for the cluster image registry to /etc/hosts so that the container runtime can pull from the registry: https://github.com/openshift/cluster-dns-operator/blob/0d46f0303675473ce5d022904ddaff23132bac97/assets/dns/daemonset.yaml#L90-L146
Would MCO overwrite /etc/hosts only on upgrades, every time the DNS operator modified it (the DNS operator checks minutely for the entry that it adds to /etc/hosts and adds it if it is missing or outdated), or what?
It's too bad there's not a simple
/etc/hosts.dbut I guess everyone who wants this ends up using a local resolver.
Yeah, something similar to /etc/NetworkManager/dnsmasq.d/. A /etc/hosts.d/ or similar would be useful to the DNS operator.
That said it feels like what we really want to achieve here is switch over the host's resolver to coredns but only if coredns is active and working? Basically if coredns is up, update
/etc/resolv.confto use127.0.0.1:53. It's a bit like where we ended up with #2011 - we have a systemd unit which monitors a pod, and if the pod is down takes action.
That would be terrific— it would obviate the need for the DNS operator to update /etc/hosts and would solve several related longstanding problems.
There was a problem hiding this comment.
Oops, I missed these comments. We did discuss the possibility of dynamically switching between dnsmasq and coredns for local resolution. It's still a bit complicated, but certainly an option.
I just pushed an alternate approach that uses a simple service to do the appending. It's fairly primitive, but it does what we need and shouldn't mess with any other customizations to /etc/hosts: #2258
|
cc: @patrickdillon |
|
looks good to me (from ovirt) but need to check the @cgwalters comment. |
|
@cybertron: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
| path: "/etc/hosts" | ||
| contents: | ||
| inline: | | ||
| 127.0.0.1 localhost localhost.localdomain localhost4 localhost4.localdomain4 |
There was a problem hiding this comment.
I guess we should use $cluster_name and $domain_name instead of 'ostest.test.metalkube.org' , right?
|
this is superceded by: #2258 no? |
|
/close |
|
@celebdor: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
10 participants
This is to enable more flexibility about when the networking services
are deployed. With the api-int record in /etc/hosts we don't need
coredns to be running for a node to connect to the cluster.