Skip to content
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

Add support for disable_dns, dns and ignore on network creation #1104

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rjeffman
Copy link

@rjeffman rjeffman commented Jan 11, 2025

This patch adds options to enhance definition of networks which are not available in the docker-compose spec but are useful for creating temporary clusters for experiments and research.

It adds support for setting DNS entries for the network or disable DNS for the network.

@@ -833,6 +833,12 @@ def get_network_create_args(net_desc, proj_name, net_name):
ipam_config_ls = ipam.get("config", [])
if net_desc.get("enable_ipv6"):
args.append("--ipv6")
if net_desc.get("ignore"):
args.append("--ignore")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you tell the use case for this? It should be easy enough to set the network to external, so there must be something I don't understand.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you run podman-compose up it will create the pod, the containers and the network. If you run podman-compose down it will destroy the pod, the containers, but not the network.

By allowing the use of --ignore one can ensure that multiple up/down will work and not fail because network already exists.

Yes, it is possible (and easy) to use the external network, but this means that the first up is different than another one.

It's most useful while experimenting with service composes rather than in production (which is my primary use for podman-compose).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p12tic I removed the "--ignore" setting from the patch. In the end, using "name: " is enough to re-use an existing network (at least in the case I had).

@@ -833,6 +833,12 @@ def get_network_create_args(net_desc, proj_name, net_name):
ipam_config_ls = ipam.get("config", [])
if net_desc.get("enable_ipv6"):
args.append("--ipv6")
if net_desc.get("ignore"):
args.append("--ignore")
if net_desc.get("disable_dns"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be called x-podman.disable_dns.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

args.append("--ignore")
if net_desc.get("disable_dns"):
args.append("--disable-dns")
if net_desc.get("dns"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be called x-podman.dns.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@p12tic
Copy link
Collaborator

p12tic commented Jan 12, 2025

Thanks a lot for the PR.

disable-dns and dns look good in principle. Please add documentation to https://github.com/containers/podman-compose/blob/main/docs/Extensions.md and a release note to https://github.com/containers/podman-compose/tree/main/newsfragments directory. See the README on how release notes should be written and removed files in 6de335b for examples.

Also, currently all features are in single commit. Could these be split so that each feature is separate at commit level?

@rjeffman
Copy link
Author

@p12tic thank you for the review! I'll update the PR with your suggestions later this week (as other tasks have taken me away from this for a while).

Podman allows to create a network disabling the DNS plugin with
'--disable-dns', but this option is not available in the compose spec.

This patch add 'x-podman.disable-dns' to the podman-compose options,
allowing the creation of a network with the DNS plugin disabled.

Signed-off-by: Rafael Guterres Jeffman <[email protected]>
This patch add 'x-podman.dns' option to the 'network' configuration,
allowing users to set the DNS resolvers for a defined network.

Signed-off-by: Rafael Guterres Jeffman <[email protected]>
@rjeffman
Copy link
Author

@p12tic, I split the PR in two commits, one for '--disable-dns' and one for '--dns'.

Let me know if the text for docs/Extensions.md and newsfragment are ok or should be improved.

@rjeffman rjeffman requested a review from p12tic January 17, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants