-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dns: Don't use dnsmasq service as container #3950
Conversation
/hold Need to test this with crc-org/snc#829 one. |
0c13ee7
to
611ceef
Compare
publicDNSQueryURI = "quay.io" | ||
crcDnsmasqService = "crc-dnsmasq.service" | ||
dnsmasqService = "dnsmasq.service" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This service is already installed/available in the VM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it is already installed and available in the VM
@@ -50,7 +47,7 @@ func createDnsmasqDNSConfig(serviceConfig services.ServicePostStartConfig) error | |||
return err | |||
} | |||
|
|||
return serviceConfig.SSHRunner.CopyDataPrivileged([]byte(dnsConfig), "/var/srv/dnsmasq.conf", 0644) | |||
return serviceConfig.SSHRunner.CopyDataPrivileged([]byte(dnsConfig), "/etc/dnsmasq.d/crc-dnsmasq.conf", 0644) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MCO won't complain about this file being created behind its back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MCO is not tracking this file so it is not complaining,
dnsmasqConfTemplate = `user=root | ||
port= {{ .Port }} | ||
bind-interfaces | ||
dnsmasqConfTemplate = `listen-address={{ .IP }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be localhost instead of the external IP?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't, because this goes to network pod and the localhost is not resolved so we need the external IP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"need" the external IP is probably a bit strong, there are most likely some 10.x address we can use that would be reachable by all pods but not exposed externally. (not asking for changes, just nuancing your answer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunno if listen-address is mandatory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunno if listen-address is mandatory?
If we not specify then it will also listen to loopback interface.
Listen on the given IP address(es). Both --interface and --listen-address options may be given, in which case the set of both interfaces and addresses is used. Note that if no --interface option is given, but --listen-address is,
dnsmasq will not automatically listen on the loopback interface. To achieve this, its IP address, 127.0.0.1, must be explicitly given as a --listen-address option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are most likely some 10.x address we can use that would be reachable by all pods but not exposed externally.
May be we can attach to a dummy interface but didn't try it since it is only available for openshift bundle right now and not with microshift one.
611ceef
to
00d2a2c
Compare
/retest |
/unhold |
This PR is going to use systemd dnsmasq service instead running it as part of container and then consuming it. It should work with current bundles and also updated bundle which doesn't have dnsmasq container cached. We are doing it because in future we want to use OVN-Kubernetes as network plugin for OCP/OKD and with our current solution it is not able to resolve the IP of the dnsmasq container so everything around dns is broken which this PR fixes. ``` === using openshift-sdn === $ oc rsh busybox-sleep-pod sh-5.1# ping 10.88.0.8 PING 10.88.0.8 (10.88.0.8) 56(84) bytes of data. 64 bytes from 10.88.0.8: icmp_seq=1 ttl=63 time=0.878 ms 64 bytes from 10.88.0.8: icmp_seq=2 ttl=63 time=0.068 ms === using ovn-k === sh-5.1# ping 10.88.0.8 PING 10.88.0.8 (10.88.0.8) 56(84) bytes of data. ^C --- 10.88.0.8 ping statistics --- 15 packets transmitted, 0 received, 100% packet loss, time 14368ms ```
00d2a2c
to
1eeff8f
Compare
dnsmasqConfTemplate = `user=root | ||
port= {{ .Port }} | ||
bind-interfaces | ||
dnsmasqConfTemplate = `listen-address={{ .IP }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"need" the external IP is probably a bit strong, there are most likely some 10.x address we can use that would be reachable by all pods but not exposed externally. (not asking for changes, just nuancing your answer)
dnsmasqConfTemplate = `user=root | ||
port= {{ .Port }} | ||
bind-interfaces | ||
dnsmasqConfTemplate = `listen-address={{ .IP }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dunno if listen-address is mandatory?
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfergeau The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR is going to use systemd dnsmasq service instead running it as part of container and then consuming it. It should work with current bundles and also updated bundle which doesn't have dnsmasq container cached.
We are doing it because in future we want to use OVN-Kubernetes as network plugin for OCP/OKD and with our current solution it is not able to resolve the IP of the dnsmasq container so everything around dns is broken which this PR fixes.