testcontainers-java icon indicating copy to clipboard operation
testcontainers-java copied to clipboard

[Enhancement]: Throw exception if user tries to set network mode "host"

Open aidando73 opened this issue 3 years ago • 3 comments

Module

Core

Proposal

Discussion spawned off of https://github.com/testcontainers/testcontainers-java/issues/5151

As per the docker docs:

The host networking driver only works on Linux hosts, and is not supported on Docker Desktop for Mac, Docker Desktop for Windows, or Docker EE for Windows Server.

I think we should throw an exception if a user tries to set network mode "host" if they aren't on linux. E.g.

    @Override
    public SELF withNetworkMode(String networkMode) {
        if (networkMode.equals("host") && !SystemUtils.IS_OS_LINUX) {
            throw new IllegalArgumentException(
                "You are using the 'host' network mode, which is not supported on non-Linux platforms."
            );
        }
        this.networkMode = networkMode;
        return self();
    }

Our users will experience a lot of weirdness otherwise.

  • The container will not be be reachable via any port.
  • If a container has any exposed ports, the container won't be able to start up (as we check for the tcp port to open up but it never does in HostPortWaitStrategy).
  • HttpWaitStrategy won't work

I think we can prevent headache if we did this.

aidando73 avatar Sep 17 '22 06:09 aidando73

Hey @REslim30 I am interested in this issue, Can you assign this to me? It seems pretty simple as you explained it clearly. Do we need to change documentation as well?

null-sys avatar Sep 19 '22 03:09 null-sys

@null-sys Thanks, we would love to get your help! No need to assign yourself haha (I don't have permissions to assign). Just submit a PR and you should be good to go. As for documentation I think updating the javadoc should be sufficient. I can give you a bit of guidance through the process.

But before that, I wanted to discuss with @kiview or @eddumelendez whether this issue is a good idea or not as there might be implications I might not have considered. And if we're good to go then you can go forward with a PR.

aidando73 avatar Sep 19 '22 04:09 aidando73

Hey @REslim30, in the light of closing #5852, I think it would be a good idea to make this explicit through throwing an exception with a clear error message, independent of the system being Linux or not.

kiview avatar Nov 17 '22 15:11 kiview