-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
config,coredns: add support for network scoped dns servers
#252
config,coredns: add support for network scoped dns servers
#252
Conversation
Don't think failing test is related to this PR, I have restarted the test just to be sure. Edit: not sure why is this only failing on |
Don't review this yet intentionally breaking tests to see how config looks in failing |
03ebeb6
to
6b06fe8
Compare
Did we already agreed on doing this now? I don't find anything in my mail or jira. |
Wait, why is priority order network servers and then container servers? that doesn't make any sense. Container-specified servers should always have priority. |
We did not ... no decision has been rendered. |
Priority reversed but lets hold this and make consensus on design doc. /hold |
6b06fe8
to
3d85b79
Compare
Aardvark and podman supports `network_dns_servers` which allows aardvark to configure a resolver at network level and all containers attached to a network honors this resolver. Needs: containers/aardvark-dns#252 Signed-off-by: Aditya R <[email protected]>
PTAL this is needed for containers/netavark#497 |
3d85b79
to
ffc64e5
Compare
@Luap99 PTAL again. |
af7645b
to
64fabeb
Compare
64fabeb
to
10aa13a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc, 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:
Approvers can indicate their approval by writing |
src/dns/coredns.rs
Outdated
} | ||
} | ||
// Add network scoped resolvers | ||
if let Some(network_dns_servers) = self.backend.get_network_scoped_resolvers(&src_address.ip()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be done if there were container-configured resolvers. We want container-provided resolvers, or network-provided resolvers. Not both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. My idea was to configure both resolvers but give higher priority to the container ones but I agree with you lets keep only one and we can revisit in future if this is needed otherway around.
config.md
Outdated
The first line in the config must contain a comma separated list of listening ips for this network, usually the bridge ips. | ||
At least one ip must be given. | ||
**Note**: Optional second coloumn allows users to specifify DNS servers at network level (seperated by comman), all the containers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An optional second column of comma delimited domain name servers can be used at the network level. All containers on that network will inherit all the specified name servers instead of using the host's resolver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
LGTM |
@baude PTAL |
/lgtm |
Aardvark and podman supports `network_dns_servers` which allows aardvark to configure a resolver at network level and all containers attached to a network honors this resolver. Needs: containers/aardvark-dns#252 Signed-off-by: Aditya R <[email protected]>
Aardvark and podman supports `network_dns_servers` which allows aardvark to configure a resolver at network level and all containers attached to a network honors this resolver. Needs: containers/aardvark-dns#252 Signed-off-by: Aditya R <[email protected]>
Aardvark and podman supports `network_dns_servers` which allows aardvark to configure a resolver at network level and all containers attached to a network honors this resolver. Needs: containers/aardvark-dns#252 Signed-off-by: Aditya R <[email protected]>
* Add support for `podman network update <>` ```console network update Description: update networks for containers and pods Usage: podman network update [options] NAME Examples: podman network update podman1 Options: --dns-add stringArray add network level nameservers --dns-drop stringArray remove network level nameservers ``` * Add support for `--network-dns-server` to `podman network create` Extends podman to support recently added features in `netavark` and `aardvark-dns` * containers/netavark#497 * containers/aardvark-dns#252 * containers/netavark#503 [NO NEW TESTS NEEDED] [NO TESTS NEEDED] Signed-off-by: Aditya R <[email protected]>
Aardvark-dns users must be able to specify
dns_servers
for containers at network level as well, so all the containers must inherit custom dns servers from their network if specified.What happens if both container's dns server and network's dns server are specified ?
network dns servers
and then container'scustom dns servers
.By default if nothing is specified resolution will happen from host's resolver