netavark ,aardvark: accept and populate custom `dns_servers` for containers #452

flouthoc · 2022-10-17T08:40:21Z

Netavark now accepts dns_servers as part of NetworkOptions which contains list of comma seperated custom DNS servers for containers. Aardvark-dns will use these as resolver for a specific container instead of host's default resolver.

Implements feature here and needs: containers/aardvark-dns#240

openshift-ci · 2022-10-17T08:40:26Z

[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

Netavark now accets `dns_servers` for each container which allows containers to use custom DNS servers as resolvers instead of falling back to host's resolver. Following field allows callers to libnetwork to pass newly added field to `netavark` and `aarvark-dns` Actual feature implemented * containers/aardvark-dns#240 * containers/netavark#452 [NO NEW TESTS NEEDED] [NO TESTS NEEDED] Signed-off-by: Aditya R <[email protected]>

Aardvark-dns and netavark now accepts custom DNS servers for containers via new config field `dns_servers`. New field allows containers to use custom resolvers instead of host's default resolvers. Following commit instruments libpod to pass these custom DNS servers set via `--dns` or central config to the network stack. Depends-on: * Common: containers/common#1189 * Netavark: containers/netavark#452 * Aardvark-dns: containers/aardvark-dns#240 [NO NEW TESTS ADDED] [NO TESTS ADDED] Signed-off-by: Aditya R <[email protected]>

Netavark now accets `dns_servers` for each container which allows containers to use custom DNS servers as resolvers instead of falling back to host's resolver. Following field allows callers to libnetwork to pass newly added field to `netavark` and `aarvark-dns` Actual feature implemented * containers/aardvark-dns#240 * containers/netavark#452 [NO NEW TESTS NEEDED] [NO TESTS NEEDED] Signed-off-by: Aditya R <[email protected]>

src/dns/aardvark.rs

src/commands/teardown.rs

test/100-bridge-iptables.bats

Luap99

Also this should return the passed in the servers in the response when dns is disabled so podman does not need to check for anything and can just write the result to resolv.conf.

flouthoc · 2022-10-17T10:53:23Z

Also this should return the passed in the servers in the response when dns is disabled so podman does not need to check for anything and can just write the result to resolv.conf.

I think what you want is already handled here, podman is already aware before spawning netavark if network has dns_enabled or not ( containers/podman#16175 )

src/dns/aardvark.rs

src/network/bridge.rs

Luap99 · 2022-10-17T10:56:20Z

Also this should return the passed in the servers in the response when dns is disabled so podman does not need to check for anything and can just write the result to resolv.conf.

I think what you want is already handled here, podman is already aware before spawning netavark if network has dns_enabled or not ( containers/podman#16175 )

Exactly podman should never check for dns enabled, netavark should provide the exact servers that end up in resolv.conf. The podman check is much more costly since it requires a network inspect each time with locking,etc...

flouthoc · 2022-10-17T14:54:11Z

Also this should return the passed in the servers in the response when dns is disabled so podman does not need to check for anything and can just write the result to resolv.conf.

I think what you want is already handled here, podman is already aware before spawning netavark if network has dns_enabled or not ( containers/podman#16175 )

Exactly podman should never check for dns enabled, netavark should provide the exact servers that end up in resolv.conf. The podman check is much more costly since it requires a network inspect each time with locking,etc...

This is added to status_block in latest commit.

…ners Netavark now accepts `dns_servers` as part of `NetworkOptions` which contains list of comma seperated custom DNS servers for containers. Aardvark-dns will use these as resolver for a specific container instead of host's default resolver. Implements feature here: containers/aardvark-dns#240 Signed-off-by: Aditya R <[email protected]>

flouthoc · 2022-10-18T03:30:36Z

@Luap99 @baude @mheon Could you PTAL.

mheon · 2022-10-18T13:10:47Z

src/dns/aardvark.rs

        let data = format!(
-            "{} {} {} {}\n",
-            entry.container_id, ipv4s, ipv6s, container_names
+            "{} {} {} {}{}\n",


Where are we prepending a space so that this is separated from the previous field?

@mheon Here https://github.com/containers/netavark/pull/452/files#diff-6307ba066c1817023f2a53b2c7c6a64d35bde6cf7f443cf5b07f34b9f163513bR260 format!(" {}", dns_server_collected)

Luap99

LGTM

Luap99 · 2022-10-18T14:22:56Z

test/100-bridge-iptables.bats

+    # check aardvark config and running
+    run_helper cat "$NETAVARK_TMPDIR/config/aardvark-dns/podman1"
+    assert "${lines[0]}" =~ "10.89.3.1,fd10:88:a::1" "aardvark set to listen to all IPs"
+    assert "${lines[1]}" =~ "^[0-9a-f]{64} 10.89.3.2 fd10:88:a::2 somename 8.8.8.8$" "aardvark config's container"


We should also test at least two custom servers but since we cannot test it right now I fine with keeping like this and add a second test for this once aardvark is ready

mheon · 2022-10-18T14:48:17Z

/lgtm

Aardvark-dns and netavark now accepts custom DNS servers for containers via new config field `dns_servers`. New field allows containers to use custom resolvers instead of host's default resolvers. Following commit instruments libpod to pass these custom DNS servers set via `--dns` or central config to the network stack. Depends-on: * Common: containers/common#1189 * Netavark: containers/netavark#452 * Aardvark-dns: containers/aardvark-dns#240 Signed-off-by: Aditya R <[email protected]>

…server After containers/netavark#452 `netavark` is incharge of deciding `custom_dns_servers` if any so lets honor that and libpod should not set these manually. This also ensures docker parity Podman populates container's `/etc/resolv.conf` with custom DNS servers ( specified via `--dns` or `dns_server` in containers.conf ) even when container is connected to a network where `dns_enabled` is `true`. Current behavior does not matches with docker, hence following commit ensures that podman only populates custom DNS server when container is not connected to any network where DNS is enabled and for the cases where `dns_enabled` is `true` the resolution for custom DNS server will happen via ( `aardvark-dns` or `dnsname` ). Reference: https://docs.docker.com/config/containers/container-networking/#dns-services Closes: containers#16172 Signed-off-by: Aditya R <[email protected]>

openshift-ci bot added the approved label Oct 17, 2022

flouthoc marked this pull request as draft October 17, 2022 08:40

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

flouthoc mentioned this pull request Oct 17, 2022

libnetwork: add DNSServers to NetworkOptions containers/common#1189

Merged

flouthoc mentioned this pull request Oct 17, 2022

podman: relay custom DNS servers to network stack ( netavark/aardvark-dns ) containers/podman#16197

Closed

Luap99 reviewed Oct 17, 2022

View reviewed changes

src/dns/aardvark.rs Outdated Show resolved Hide resolved

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

test/100-bridge-iptables.bats Show resolved Hide resolved

Luap99 reviewed Oct 17, 2022

View reviewed changes

flouthoc force-pushed the container-dns-servers branch from 5b4a3b4 to a590eb8 Compare October 17, 2022 10:49

Luap99 reviewed Oct 17, 2022

View reviewed changes

src/dns/aardvark.rs Outdated Show resolved Hide resolved

Luap99 reviewed Oct 17, 2022

View reviewed changes

src/network/bridge.rs Outdated Show resolved Hide resolved

flouthoc force-pushed the container-dns-servers branch from a590eb8 to 32dbe31 Compare October 17, 2022 11:38

flouthoc mentioned this pull request Oct 17, 2022

aardvark-dns: add support for container's custom dns_servers containers/aardvark-dns#240

Merged

flouthoc force-pushed the container-dns-servers branch from 32dbe31 to 89ecfe3 Compare October 17, 2022 14:53

flouthoc marked this pull request as ready for review October 17, 2022 15:00

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

flouthoc force-pushed the container-dns-servers branch from 89ecfe3 to 6ec2f48 Compare October 17, 2022 16:27

flouthoc force-pushed the container-dns-servers branch from 6ec2f48 to f6e0258 Compare October 17, 2022 16:33

flouthoc requested review from Luap99, mheon and baude October 17, 2022 16:35

mheon reviewed Oct 18, 2022

View reviewed changes

flouthoc requested a review from mheon October 18, 2022 14:06

Luap99 reviewed Oct 18, 2022

View reviewed changes

openshift-ci bot assigned mheon Oct 18, 2022

openshift-ci bot added the lgtm label Oct 18, 2022

openshift-merge-robot merged commit 706601a into containers:main Oct 18, 2022

flouthoc mentioned this pull request Oct 21, 2022

test: unskip exisiting custom_dns_server tests and add a new one. #460

Merged

flouthoc mentioned this pull request Oct 26, 2022

libpod,netavark: correctly set /etc/resolv.conf for custom dns server and make --dns functional containers/podman#16297

Merged

xiaoyar mentioned this pull request Dec 1, 2023

The DNS resolution in container has different behavior with and without "--dns" added in "podman run" command, even the /etc/resolv.conf have the same content in container #855

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

netavark ,aardvark: accept and populate custom `dns_servers` for containers #452

netavark ,aardvark: accept and populate custom `dns_servers` for containers #452

flouthoc commented Oct 17, 2022 •

edited

Loading

openshift-ci bot commented Oct 17, 2022

Luap99 left a comment

flouthoc commented Oct 17, 2022

Luap99 commented Oct 17, 2022

flouthoc commented Oct 17, 2022

flouthoc commented Oct 18, 2022

mheon Oct 18, 2022

flouthoc Oct 18, 2022

Luap99 left a comment

Luap99 Oct 18, 2022

mheon commented Oct 18, 2022

netavark ,aardvark: accept and populate custom dns_servers for containers #452

netavark ,aardvark: accept and populate custom dns_servers for containers #452

Conversation

flouthoc commented Oct 17, 2022 • edited Loading

openshift-ci bot commented Oct 17, 2022

Luap99 left a comment

Choose a reason for hiding this comment

flouthoc commented Oct 17, 2022

Luap99 commented Oct 17, 2022

flouthoc commented Oct 17, 2022

flouthoc commented Oct 18, 2022

mheon Oct 18, 2022

Choose a reason for hiding this comment

flouthoc Oct 18, 2022

Choose a reason for hiding this comment

Luap99 left a comment

Choose a reason for hiding this comment

Luap99 Oct 18, 2022

Choose a reason for hiding this comment

mheon commented Oct 18, 2022

netavark ,aardvark: accept and populate custom `dns_servers` for containers #452

netavark ,aardvark: accept and populate custom `dns_servers` for containers #452

flouthoc commented Oct 17, 2022 •

edited

Loading