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

Dockerfile: listen on both IPv4 and IPv6 #934

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rissson
Copy link

@rissson rissson commented Feb 5, 2024

Follow up for #805.

Docker indeed doesn't support IPv6 properly, but a lot of other container runtimes do. This change makes routinator running in a container listen on both IPv4 (if available) and IPv6 (if available) by default.

Should I remove the documentation that was added in 603e44d with this change?

Signed-off-by: Marc 'risson' Schmitt <[email protected]>
@ximon18
Copy link
Member

ximon18 commented Feb 6, 2024

Hi @rissson,

Thanks for the contribution, very appreciated! 👏

It seems on the surface like a harmless improvement. However, I'm not (yet) convinced that this is a change that we want to make.

The Docker container runtime is the intended target for the Dockerfile and the images we create and I am concerned that it could be misleading to make it appear to work with IPv6 out of the box given that IPv6 is not supported by Docker by default.

Also, we have no experience with other container runtimes and prefer where possible not to maintain solutions which we do not test and/or are not familiar with ourselves.

One possible way forward could be to extend our automated testing in some way to verify that this does indeed (a) not break with stock Docker and (b) works as intended under some other container runtime, but that would also potentially imply that we support such other runtimes, and at present I don't think we have the resources to test this ourselves or to extend our test / packaging processes in such a way.

@partim? @AlexanderBand? What are your thoughts on this? Perhaps I am being too strict and we should just say LGTM?

@AlexanderBand
Copy link
Member

AlexanderBand commented Feb 6, 2024

If Docker doesn't support IPv6 by default—but it could possibly work—then I'd rather suggest in the documentation that "[::]:3323" may work on certain (or most?) systems, rather than making it the default.

@rissson
Copy link
Author

rissson commented Feb 6, 2024

The Docker container runtime is the intended target for the Dockerfile and the images we create and I am concerned that it could be misleading to make it appear to work with IPv6 out of the box given that IPv6 is not supported by Docker by default.

Also, we have no experience with other container runtimes and prefer where possible not to maintain solutions which we do not test and/or are not familiar with ourselves.

That's fair enough, but one can still use IPv6 with Docker. Either with Docker directly or by using it as a CRI in k8s. I myself have been running Docker with IPv6 turned on, by setting fixed-cidr-v6 and ipv6 in /etc/docker/daemon.json and running the container with --sysctl net.ipv6.conf.all.disable_ipv6=0, and am now in the process of moving stuff over to a dual stack k8s cluster running containerd instead of Docker.

I was banging my head around figuring out why I couldn't reach routinator over IPv6, but could over IPv4, and checking that routinator was listening over IPv6 was the last thing I checked because I assumed that it would. I think most things I run listen on both v4 and v6 if available by default.

On the topic of distributing OCI images, I think it's safe to assume that people will not only run it with the Docker runtime, as they may choose another one because of Docker's (many) flaws.

One possible way forward could be to extend our automated testing in some way to verify that this does indeed (a) not break with stock Docker and (b) works as intended under some other container runtime, but that would also potentially imply that we support such other runtimes, and at present I don't think we have the resources to test this ourselves or to extend our test / packaging processes in such a way.

I have actually tried that change out with Docker and --sysctl net.ipv6.conf.all.disable_ipv6=1 to make sure that it still works in an IPv4-only environment, and it still does indeed.

This change shouldn't break existing installations, and allows for a step less for users. Also, if in the future more arguments are added to the default CMD, they may be missed by users upgrading that are overriding those to enable IPv6 support.

If Docker doesn't support IPv6 by default—but it could possibly work—then I'd rather suggest in the documentation that "[::]:3323" may work on certain (or most?) systems, rather than making it the default.

Just to make sure we're on the same page, [::]:3323 means listen on 0.0.0.0:3323 if IPv4 is enabled on the system and [::]:3323 if IPv6 is enabled on the system. Or rather than system, I should say in the network namespace routinator is running in.

@ximon18
Copy link
Member

ximon18 commented Feb 27, 2024

Apologies for the delayed response, it's a bit busy here at the moment. I'm not comfortable making this change at the moment, I'd want to do some testing and I don't have time unfortunately right now.

Just to make sure we're on the same page, [::]:3323 means listen on 0.0.0.0:3323 if IPv4 is enabled on the system and [::]:3323 if IPv6 is enabled on the system. Or rather than system, I should say in the network namespace routinator is running in.

@AlexanderBand: I think this was directed at you?

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.

3 participants