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

Method of exposing UDP port changed in 1.16.0 #4667

Closed
KyleAure opened this issue Nov 9, 2021 · 5 comments · May be fixed by #4668
Closed

Method of exposing UDP port changed in 1.16.0 #4667

KyleAure opened this issue Nov 9, 2021 · 5 comments · May be fixed by #4668

Comments

@KyleAure
Copy link
Contributor

KyleAure commented Nov 9, 2021

I am upgrading from:
TestContainers 1.15.3 -> 1.16.2
Docker-Java 3.2.8 -> 3.2.12

In order to expose a UDP port, users are required to access the underlying docker-java command, and modify it to expose the desired port. In my project I am doing this using a custom container class and overwriting the configure() method as follows:

public class KerberosContainer extends GenericContainer<KerberosContainer> {
    @Override
    protected void configure() {
        withExposedPorts(99, 464, 749); //TCP ports
        withNetworkAliases(KRB5_KDC);
        withCreateContainerCmdModifier(cmd -> {
            List<ExposedPort> ports = new ArrayList<>();
            for (ExposedPort p : cmd.getExposedPorts()) {
                ports.add(p);
            }
            ports.add(new ExposedPort(99, InternetProtocol.UDP));
            cmd.withExposedPorts(ports);
	    cmd.withHostName(KRB5_KDC);
        });
    }
}

Once the container is started, I try to extract the mapped port using the InspectContainerResponse object as follows:

    @Override
    protected void containerIsStarted(InspectContainerResponse containerInfo) {
        NetworkSettings networkSettings = containerInfo.getNetworkSettings();
        Ports ports = networkSettings.getPorts();
        Map<ExposedPort, Binding[]> map = ports.getBindings();
        Binding[] bindings = map.get(new ExposedPort(99, InternetProtocol.UDP));
        Binding binding = bindings[0];  //NullPointerException
        String udp99 = binding.getHostPortSpec();

        udp_99 = Integer.valueOf(udp99);
    }

A NullPointerException is thrown because the withCreateContainerCmdModifier was not honored.
This is apparent when inspecting the container created directly with docker:

Screen Shot 2021-11-09 at 1 39 18 PM

And printing out the contents of the bindings map at runtime

{
  749/tcp=[Lcom.github.dockerjava.api.model.Ports$Binding;@3bcb01f9, 
  464/tcp=[Lcom.github.dockerjava.api.model.Ports$Binding;@defb0e3a, 
  99/tcp=[Lcom.github.dockerjava.api.model.Ports$Binding;@15c64f5f, 
  99/udp=null, 
  88/tcp=null
}
@KyleAure
Copy link
Contributor Author

I believe this is also related to issue #4489

@KyleAure
Copy link
Contributor Author

It looks like this broke because createCommand.withPublishAllPorts(true); was removed from the generic container configure step as part of PR #4122
This means that in order to publish a UDP port we need to both expose the port and provide a port binding. Whereas, before if we expose the port, it was bound automatically.

Here is a sample of what I needed to do to work around this:

GenericContainer container = new GenericContainer<>(image)
    .withExposedPorts(8080, 8081)
    .withCreateContainerCmdModifier(cmd -> {
        //Add previously exposed ports and UDP port
        List<ExposedPort> exposedPorts = new ArrayList<>();
        for (ExposedPort p : cmd.getExposedPorts()) {
            exposedPorts.add(p);
        }
        exposedPorts.add(ExposedPort.udp(99));
        cmd.withExposedPorts(exposedPorts);

        //Add previous port bindings and UDP port binding
        Ports ports = cmd.getPortBindings();
        ports.bind(ExposedPort.udp(99), Ports.Binding.empty());
        cmd.withPortBindings(ports);
    }

@bsideup
Copy link
Member

bsideup commented Nov 10, 2021

Putting "wontfix" here because it isn't a breaking change but an assumption that got changed. We never guaranteed that we will bind ports that were added through withCreateContainerCmdModifier and going that route means that now you are dealing with Docker's low level API and need to understand how it behaves.

@KyleAure KyleAure changed the title Method of exposing UDP port broken in 1.16.2 Method of exposing UDP port changed in 1.16.0 Nov 11, 2021
@KyleAure
Copy link
Contributor Author

Closing this issue as no work needs to be done to fix this, just a code change on the user's part. Keeping associated PR open just in case the maintainers want to add this as a test case to prevent future regression testing.

@kiview
Copy link
Member

kiview commented Nov 12, 2021

Thanks @KyleAure, your help is much appreciated 🙌

ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 12, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 13, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 16, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 16, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 16, 2022
ArneLimburg added a commit to ArneLimburg/testflight that referenced this issue Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants