main: run aardvark as a `daemon` via `forking` and maintain its partial `daemon` like nature. #148

flouthoc · 2022-06-14T09:49:50Z

This is one the redesign PR for aardvark-dns and netavark. Design
proposes that forking will happen at aardvark-end instead of
netavark and aarvark verifies validity of server before returning
control to aardvark.

Redesign proposal

Aardvark will invoke server on the child process by double-forking.
Parent waits for child to show up and verify against a dummy DNS
query to check if server is running.
Exit parent on success and deatch child.
Calling process will wait for aardvark's parent process to return.
On successful return from parent it will be assumed that aardvark is
running properly

One new design is implemented and merged and netavark starts using this
it should close

openshift-ci · 2022-06-14T09:49:55Z

[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

Needs approval from an approver in each of these files:

~~OWNERS~~ [flouthoc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

src/commands/run.rs

Luap99

It looks like you are misunderstanding what I want.
The goal is to daemonize ourself, see daemon(3). exec() is not required.

src/commands/run.rs

src/main.rs

flouthoc · 2022-06-14T13:18:59Z

It looks like you are misunderstanding what I want. The goal is to daemonize ourself, see daemon(3). exec() is not required.

Yeah can do that well, exec might not be needed. We can just continue with actual serve logic in the child fork.

Netavark now does not double-forks aardvark-dns's server instead it waits for aardvark-process to return back and success return means aardvark-dns is ready to serve requests and now forking happens at aardvark end. This needs: containers/aardvark-dns#148 Signed-off-by: Aditya R <[email protected]>

Luap99 · 2022-06-15T11:32:32Z

src/commands/run.rs

+            Ok(ForkResult::Parent { child, .. }) => {
+                log::debug!("starting aardvark on a child with pid {}", child);
+                // verify aardvark here and block till all the ip are ready
+                match config::parse_configs(&input_dir) {


I don't think we should try to query the server.

If we want to sync with the child we could create a pipe and make a blocking read in the parent and a write in the child directly before the child starts listening on the socket.

The problem is again that it does not ensures e2e testing on the actual interface and does not ensures if something is taking time after it performs listening. We can use pipe but current check ensure that we actually create a DNS client and check against it, is there any problem if we use this approach for verification ?

It just doesn't feel right to ensure this by making a query. First this adds a hardcoded value with a special meaning which leads to unexpected behaviour when a user for some reason ends up using this name. It also adds a lot of overhead.

@Luap99 I see but one issue is that we have different servers running in different threads and will have to ensure that each one of them is running. My intention is to only return from parent when event loop for all the servers have already started which happens after we start listening and its a blocking call so we cannot notify after that with unix pipe approach we will end up notifying parent before even event loop starts but i doubt that this delay would matter so I don't have a strict opinion here and we could switch to unix pipe approach if everyone is fine with that so no issues. @mheon Do you have any opinion here ?

Switched this a unix pipe blocking approach.

src/main.rs

src/commands/run.rs

Netavark now does not double-forks aardvark-dns's server instead it waits for aardvark-process to return back and success return means aardvark-dns is ready to serve requests and now forking happens at aardvark end. This needs: containers/aardvark-dns#148 Signed-off-by: Aditya R <[email protected]>

src/commands/run.rs

edsantiago · 2022-06-17T19:12:43Z

Since this seems to be the clearinghouse for all the different Aardvark CI flakes, here are the ones seen in the last fifteen days in podman:

Podman run networking [It] Aardvark Test 2: Two containers, same subnet

fedora-36 : int podman fedora-36 root host
- PR #14608
- PR #14533
- PR #14478
  - 06-03 10:40
  - 06-03 10:40
- PR #14477
  - 06-03 09:58
fedora-36 : int remote fedora-36 root host [remote]
- PR #14608
  - 06-16 09:26
  - 06-16 09:26
- PR #14570

Podman run networking [It] Aardvark Test 5: Two containers on their own subnets, one container on both

fedora-36 : int podman fedora-36 root host
- PR #14608
- PR #14533
- PR #14477
fedora-36 : int remote fedora-36 root host [remote]
- PR #14570
  - 06-14 19:24
  - 06-14 19:24

Podman run networking [It] Aardvark Test 3: Two containers, same subnet w/aliases

fedora-36 : int podman fedora-36 root host
- PR #14608
  - 06-16 09:09
- PR #14605
  - 06-15 14:47
- PR #14533
- PR #14478
fedora-36 : int remote fedora-36 root host [remote]
- PR #14570
  - 06-14 19:24
  - 06-14 19:24

Podman run networking [It] podman run check dnsname plugin with Netavark

fedora-36 : int podman fedora-36 root host
- PR #14605

Podman run networking [It] Aardvark Test 6: Three subnets, first container on 1/2 and second on 2/3, w/ network aliases

fedora-36 : int podman fedora-36 root host
- PR #14533
fedora-36 : int remote fedora-36 root host [remote]
- PR #14570

Podman run networking [It] Aardvark Test 1: One container

fedora-36 : int podman fedora-36 root host
- PR #14477

Luap99

One nit otherwise LGTM
We should definitely put this and the netavark change in the podman CI to make sure everything still works before merging either PR.

@cevich Is it possible to use netavark/aardvark build from this PR in the podman CI?

Luap99 · 2022-07-12T14:08:06Z

src/commands/run.rs

+        // fork and verify if server is running
+        // and exit parent
+        // setsid() ensures that there is no controlling terminal on the child process
+        match unsafe { fork() } {


Why is this unsafe?

nix's fork is returns unsafe most likely because it invokes fork( directly in such cases rust compiler will bark however our use-case is okay here so all such functions can be invoked under unsafe. See https://docs.rs/nix/latest/nix/unistd/fn.fork.html and this https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html

But invocation is fine for our use-case.

cevich · 2022-07-12T16:31:48Z

@cevich Is it possible to use netavark/aardvark build from this PR in the podman CI?

Yes, but only with some manual intervention[*]. Here's what I would suggest (in podman):

Check out main
Edit .cirrus.yml and comment out the podman_machine task (workaround a bug: fix expected in a day or less).
Use hack/get_ci_vm.sh int podman fedora-36 root host (or one of the other tasks if you prefer).
Do an rpm -e --nodeps aardvark-dns netavark to strip out the distro-provided files w/o also removing any dependencies.
Manually download the latest netavark and aardvark builds:
1. curl -o /tmp/netavark.zip https://api.cirrus-ci.com/v1/artifact/github/containers/netavark/success/binary.zip
2. curl -o /tmp/aardvark-dns.zip https://api.cirrus-ci.com/v1/artifact/github/containers/aardvark-dns/success/binary.zip
Unzip those files and manually copy the binaries into place.
Execute the podman tests (if desired) cd $GOSRC; ./contrib/cirrus/runner.sh (should "just work").

[*]Note: If you expect to be doing this regularly, this semi-manual method will quickly become a PITA. In this case, I would propose new CI work for podman. For example, we make setup_environment.sh sensitive to a PR label, and if present perform the steps above automatically.

Luap99 · 2022-07-12T16:36:10Z

Manually download the latest netavark and aardvark builds:

I want the one from this PR and containers/netavark#300, we should verify that this works before merging.

I don't plan on doing this regular, I just want to be sure this passes podman CI

cevich · 2022-07-12T17:44:54Z

I want the one from this PR and...

Ahh okay, this can be done in a similar way, the URL is simply different:

Click on the 'success' for each, hit the 'view more details in Cirrus-CI' link.
Look at the URL, it will have a "task ID" (number) at the end. Copy this.
In your VM, use a URL of the form: https://api.cirrus-ci.com/v1/artifact/task/<CIRRUS_TASK_ID>/binary.zip
Do the same thing to get the URL for the other repo's CI run.

Does that make sense?

(Let me know if it doesn't work, I didn't actually try it this time).

Luap99 · 2022-07-12T17:59:45Z

Makes sense I will try it tomorrow.

This is one the redesign PR for aardvark-dns and netavark. Design proposes that forking will happen at aardvark-end instead of netavark and aardvark will verify if servers are up before parent goes away. Redesign proposal * Aardvark will invoke server on the child process by forking. * Parent waits for child to show up and verify against a dummy DNS query to check if server is running. * Exit parent on success and deatch child. * Calling process will wait for aardvark's parent process to return. * On successful return from parent it will be assumed that aardvark is running properly One new design is implemented and merged and netavark starts using this it should close * containers/podman#14173 * containers/podman#14171 Signed-off-by: Aditya R <[email protected]>

flouthoc · 2022-07-13T06:19:45Z

Rebased.

Netavark now does not double-forks aardvark-dns's server instead it waits for aardvark-process to return back and success return means aardvark-dns is ready to serve requests and now forking happens at aardvark end. This needs: containers/aardvark-dns#148 Signed-off-by: Aditya R <[email protected]>

Luap99 · 2022-07-13T13:52:53Z

OK I checked both the podman integration and system test with this and they passed, LGTM

@baude @mheon PTAL

baude · 2022-07-14T14:16:56Z

/lgtm

Netavark now does not double-forks aardvark-dns's server instead it waits for aardvark-process to return back and success return means aardvark-dns is ready to serve requests and now forking happens at aardvark end. This needs: containers/aardvark-dns#148 Signed-off-by: Aditya R <[email protected]>

openshift-ci bot added the do-not-merge/work-in-progress label Jun 14, 2022

openshift-ci bot added the approved label Jun 14, 2022

flouthoc marked this pull request as draft June 14, 2022 09:49

This was referenced Jun 14, 2022

dns: wait for aardvark server just after spawning aardvark-dns. containers/netavark#300

Closed

Aardvark/Netavark flakes containers/podman#14173

Closed

Aardvark Test 5: Two containers on their own subnets, one container on both containers/podman#14171

Closed

flouthoc commented Jun 14, 2022

View reviewed changes

src/commands/run.rs Outdated Show resolved Hide resolved

flouthoc force-pushed the support-double-forking branch from 780f281 to 29eac3c Compare June 14, 2022 10:04

Luap99 reviewed Jun 14, 2022

View reviewed changes

src/commands/run.rs Outdated Show resolved Hide resolved

src/commands/run.rs Outdated Show resolved Hide resolved

src/commands/run.rs Outdated Show resolved Hide resolved

src/main.rs Outdated Show resolved Hide resolved

flouthoc force-pushed the support-double-forking branch from 29eac3c to aa61a9c Compare June 15, 2022 10:15

flouthoc changed the title ~~[WIP] main: run aardvark as a daemon via double-forking and maintain its partial daemon like nature.~~ [WIP] main: run aardvark as a daemon via forking and maintain its partial daemon like nature. Jun 15, 2022

flouthoc marked this pull request as ready for review June 15, 2022 10:17

flouthoc changed the title ~~[WIP] main: run aardvark as a daemon via forking and maintain its partial daemon like nature.~~ main: run aardvark as a daemon via forking and maintain its partial daemon like nature. Jun 15, 2022

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 15, 2022

flouthoc mentioned this pull request Jun 15, 2022

netavark, dns: don't double-fork aardvark instead wait for the aardvark process to return. containers/netavark#308

Merged

flouthoc force-pushed the support-double-forking branch 2 times, most recently from f3d9392 to b08879c Compare June 15, 2022 10:46

Luap99 reviewed Jun 15, 2022

View reviewed changes

flouthoc force-pushed the support-double-forking branch from b08879c to 7b8dd89 Compare June 15, 2022 13:01

flouthoc force-pushed the support-double-forking branch from 7b8dd89 to 0441806 Compare June 15, 2022 13:22

Luap99 reviewed Jun 15, 2022

View reviewed changes

src/commands/run.rs Outdated Show resolved Hide resolved

Luap99 reviewed Jun 15, 2022

View reviewed changes

src/commands/run.rs Outdated Show resolved Hide resolved

flouthoc force-pushed the support-double-forking branch 3 times, most recently from 4107b83 to 383e425 Compare June 16, 2022 07:50

flouthoc requested a review from Luap99 June 16, 2022 07:52

Luap99 reviewed Jul 12, 2022

View reviewed changes

flouthoc force-pushed the support-double-forking branch from 383e425 to 631a2b6 Compare July 13, 2022 06:19

openshift-ci bot assigned baude Jul 14, 2022

openshift-ci bot added the lgtm label Jul 14, 2022

openshift-ci bot merged commit 6ba6818 into containers:main Jul 14, 2022

flouthoc mentioned this pull request Aug 18, 2022

Newly started container sometimes fails to resolve hostname of another container on the same podman net containers/podman#13983

Closed

flouthoc mentioned this pull request Sep 28, 2022

Resolving peer pod DNS names often hangs for a long time in user mode containers/podman#15972

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

main: run aardvark as a `daemon` via `forking` and maintain its partial `daemon` like nature. #148

main: run aardvark as a `daemon` via `forking` and maintain its partial `daemon` like nature. #148

flouthoc commented Jun 14, 2022 •

edited

Loading

openshift-ci bot commented Jun 14, 2022

Luap99 left a comment

flouthoc commented Jun 14, 2022

Luap99 Jun 15, 2022

flouthoc Jun 15, 2022 •

edited

Loading

Luap99 Jun 15, 2022

flouthoc Jun 15, 2022

flouthoc Jun 16, 2022 •

edited

Loading

edsantiago commented Jun 17, 2022

Luap99 left a comment

Luap99 Jul 12, 2022

flouthoc Jul 13, 2022

cevich commented Jul 12, 2022

Luap99 commented Jul 12, 2022

cevich commented Jul 12, 2022

Luap99 commented Jul 12, 2022

flouthoc commented Jul 13, 2022

Luap99 commented Jul 13, 2022

baude commented Jul 14, 2022

main: run aardvark as a daemon via forking and maintain its partial daemon like nature. #148

main: run aardvark as a daemon via forking and maintain its partial daemon like nature. #148

Conversation

flouthoc commented Jun 14, 2022 • edited Loading

openshift-ci bot commented Jun 14, 2022

Luap99 left a comment

Choose a reason for hiding this comment

flouthoc commented Jun 14, 2022

Luap99 Jun 15, 2022

Choose a reason for hiding this comment

flouthoc Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

Luap99 Jun 15, 2022

Choose a reason for hiding this comment

flouthoc Jun 15, 2022

Choose a reason for hiding this comment

flouthoc Jun 16, 2022 • edited Loading

Choose a reason for hiding this comment

edsantiago commented Jun 17, 2022

Podman run networking [It] Aardvark Test 2: Two containers, same subnet

Podman run networking [It] Aardvark Test 5: Two containers on their own subnets, one container on both

Podman run networking [It] Aardvark Test 3: Two containers, same subnet w/aliases

Podman run networking [It] podman run check dnsname plugin with Netavark

Podman run networking [It] Aardvark Test 6: Three subnets, first container on 1/2 and second on 2/3, w/ network aliases

Podman run networking [It] Aardvark Test 1: One container

Luap99 left a comment

Choose a reason for hiding this comment

Luap99 Jul 12, 2022

Choose a reason for hiding this comment

flouthoc Jul 13, 2022

Choose a reason for hiding this comment

cevich commented Jul 12, 2022

Luap99 commented Jul 12, 2022

cevich commented Jul 12, 2022

Luap99 commented Jul 12, 2022

flouthoc commented Jul 13, 2022

Luap99 commented Jul 13, 2022

baude commented Jul 14, 2022

main: run aardvark as a `daemon` via `forking` and maintain its partial `daemon` like nature. #148

main: run aardvark as a `daemon` via `forking` and maintain its partial `daemon` like nature. #148

flouthoc commented Jun 14, 2022 •

edited

Loading

flouthoc Jun 15, 2022 •

edited

Loading

flouthoc Jun 16, 2022 •

edited

Loading