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 enable_ipv6 #749

Merged
merged 2 commits into from
Mar 9, 2024
Merged

Add support for enable_ipv6 #749

merged 2 commits into from
Mar 9, 2024

Conversation

maxi0604
Copy link
Contributor

@maxi0604 maxi0604 commented Aug 23, 2023

Fixes #751

This pull request adds support for the enable_ipv6 network option when creating the network. This is supported by docker compose and is contained in the compose-spec [1]. The change simply exposes the --ipv6 option of podman network create

PS: One could argue that this should be on by default and only disabled when enable_ipv6: false is set, but doing something like that should - if we even choose to do it at all - be delayed at the very least until containers/podman#15850 is fixed.

[1] https://github.com/compose-spec/compose-spec/blob/master/06-networks.md#enable_ipv6

@sgryphon
Copy link

This is important. Please merge.

@pini-gh
Copy link

pini-gh commented Jan 8, 2024

Anything preventing this to be merged?

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

The PR looks great, thank you!

We need unit tests for it.

This part https://github.com/containers/podman-compose/blob/main/podman_compose.py#L713C1-L749C34 could be extracted to a separate function. Then a unit tests could be written just like for container_to_args in pytests/test_container_to_args.py

@p12tic
Copy link
Collaborator

p12tic commented Mar 8, 2024

@maxi0604 Just a note that podman-compose has switched to main as the primary branch. You probably rebased against devel.

@p12tic
Copy link
Collaborator

p12tic commented Mar 8, 2024

Also probably a nitpick, but it would be great if extraction of get_network_create_args would be the first commit. This way the amount of changes to code not under unit tests would be minimized.

@p12tic
Copy link
Collaborator

p12tic commented Mar 9, 2024

Rebased and reordered commits.

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Looks great!

@p12tic p12tic merged commit 121b5f6 into containers:main Mar 9, 2024
4 checks passed
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.

Support enable_ipv6 for networks
4 participants