containers / aardvark-dns

Authoritative dns server for A/AAAA container records. Forwards other request to host's /etc/resolv.conf
Apache License 2.0
176 stars 31 forks source link

update upsteam resolvers on each refresh #488

Closed Luap99 closed 1 month ago

Luap99 commented 1 month ago

Right now we read resolv.conf once at the start for each network server, this means if resolv.conf on the host changes it keeps using the old nameservers. This commit changes this to pass the same vector around for each thread. Due to rust abstractions this requires the use of a Arc<Mutex<_>> to safly share the vector between threads. Note I am using .expect() here to access the value which causes a panic on error. However if you read the docs on the mutex the only way this errors is when the thread holding the lock panicked in which case it is best for us to abort anyway.

This is still not perfect as we only refresh when a new container is started/stopped but better than not updating it at all. Long term some inotify watcher (or polling) would be better.

openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

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: - ~~[OWNERS](https://github.com/containers/aardvark-dns/blob/main/OWNERS)~~ [Luap99] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
Luap99 commented 1 month ago

@mheon PTAL

packit-as-a-service[bot] commented 1 month ago

Integration tests failed. @containers/packit-build please check.

mheon commented 1 month ago

LGTM

Luap99 commented 1 month ago

@mheon @baude PTAL and merge

mheon commented 1 month ago

/lgtm