dns: wait for aardvark server just after spawning aardvark-dns.#300
dns: wait for aardvark server just after spawning aardvark-dns.#300flouthoc wants to merge 1 commit intocontainers:mainfrom
aardvark-dns.#300Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
34d100e to
76045b7
Compare
Since starting DNS server is the last task for netavark's `network setup` so netavark must verify at least a few times if aardvark-dns is in a state where it can serve. Signed-off-by: Aditya R <arajan@redhat.com>
76045b7 to
7b394da
Compare
|
I don't think this is the right way. There is still no guarantee that this waits for aardvark to be ready. |
|
@Luap99 Yes as of now it only waits for while and then proceeds. I am just afraid that if we wait for infinite duration and aardvark fails this netavark would end up in blocking state. I can think of two options. Do you have any suggestion other than these.
|
| while retry_count > 0 { | ||
| let aardvark_pid = self.get_aardvark_pid(); | ||
| if aardvark_pid != -1 | ||
| && signal::kill(Pid::from_raw(aardvark_pid), Signal::SIGHUP).is_ok() |
There was a problem hiding this comment.
Avoid SIGHUP, send signal 0 if possible - it takes no action on the target process, just returns if the process is alive.
|
Waiting for a basic DNS request to process (with a time limit) seems like the best course of action. |
| { | ||
| break; | ||
| } | ||
| let duration_millis = time::Duration::from_millis(500); |
There was a problem hiding this comment.
500ms is a little coarse. Recommend adjusting down to 250ms and doubling retries.
|
I think the best thing to do is do a fork on aardvark side. Basically netavark spawns aardvark and then waits for exit. aardvark then sets up everything and once it is ready if forks itself and the parent exits so netavark know it is ready. This will also fix issues with the error reporting since we can keep the stderr open until the fork. This however does not fix the issue when adding new entries. In this case we still would need a bidirectional way to update so aardvark could signal us that it is ready. |
|
I think we could this i am trying out a spike here: containers/aardvark-dns#148 and we might need few changes on netavark end as well. |
|
we went with #308 so I am closing this one |
Since starting DNS server is the last task for netavark's
network setupso netavark must verify at least a few times if aardvark-dns isin a state where it can serve request before passing context back to
container manager.
Potential fix: containers/podman#14173