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

server: improve parent <-> child error handling #481

Closed Luap99 closed 1 month ago

Luap99 commented 1 month ago

The code currently doesn't check for most child errors as it basicaly immediately wrote the ready byte. Also it never considered the fact that the child can exit early and in that case it would have blocked forever because the parent never closed the writing side before call read().

This fixes that part, however it still reports success way to early. It should wait for the first bind but that needs more complicated changes.

serve: fix broken error logging

Turns out the task contains a result in a result and we were only looking at task error caused by tokio and not the actual result returned from start_dns_server(). This caused all bind error to go nowhere and no even logged by in the journal.

Fixes: https://github.com/containers/aardvark-dns/commit/224756d98986355d7f7d71d79c8012f0c7e4c29d ("server: use only one tokio runtime")


Note I still have to do more changes in order to get the bind error back to netavark on first start.

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 another one

mheon commented 1 month ago

Format comment, code LGTM

Luap99 commented 1 month ago

fix the comment but also added a new commit to switch to tokio for the signal handling.

mheon commented 1 month ago

/lgtm