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

Bug: port mapping not working with with_network (with_exposed_ports) #645

Closed
fabricebaranski opened this issue Jul 11, 2024 · 7 comments · Fixed by #678
Closed

Bug: port mapping not working with with_network (with_exposed_ports) #645

fabricebaranski opened this issue Jul 11, 2024 · 7 comments · Fixed by #678

Comments

@fabricebaranski
Copy link

Hi all,
I would like to have two containers connected each others, with a code similar to

test_network = Network()
test_network.create()
container1 = DockerContainer("myimage1", hostname="myimage1") \
        .with_network(test_network).with_network_aliases("myalias1").with_exposed_ports(9098)
container2 = DockerContainer("myimage1", hostname="myimage2") \
        .with_network(test_network).with_network_aliases("myalias2").with_exposed_ports(9099)
container1.start()
container2.start()

I regularly have no port mapping for my containers. Do you have any ideas?

@fabricebaranski fabricebaranski changed the title Pb with port mapping (with_exposed_ports) Bug: Pb with port mapping (with_exposed_ports) Jul 11, 2024
@alexanderankin alexanderankin changed the title Bug: Pb with port mapping (with_exposed_ports) Bug: port mapping not working with with_network (with_exposed_ports) Jul 11, 2024
@fabricebaranski
Copy link
Author

Moreover, I have some difficulties to create communication between these two containers on gitlab ci.

@mgorsk1
Copy link
Contributor

mgorsk1 commented Jul 15, 2024

I also experience some difficulties with advanced network setup and I came to the conclusion it's due to the way it's implemented in TC:

  • when new container is created, self._container = docker_client.run is executed without network kwarg, this means container has default network assigned by default:
        if "network" not in kwargs and not get_docker_host():
            # Otherwise we'll try to find the docker host for dind usage.
            host_network = self.find_host_network()
            if host_network:
                kwargs["network"] = host_network
  • later, if self._network is not empty (after adding .with_network to container definition), container is connected to it (so it now is connected to two networks):
        if self._network:
            self._network.connect(self._container.id, self._network_aliases)

the solution could be to pass network kwarg to container constructor and omit .with_network completely but then we cannot assign network aliases. The workaround for this scenario is to use .with_name (it will be used as network alias as well) - I usually generate it with some random characters inside to avoid name clashes. Example:

network = Network()
network.create()

MYSQL_CONTAINER_NAME = "mysql-" + generate_random_string_of_length(8)

mysql_container = (
    MySqlContainer(
        f"mysql:{MYSQL_IMAGE_TAG}",
        username=MYSQL_USER,
        password=MYSQL_PASSWORD,
        root_password=MYSQL_ROOT_PASSWORD,
        dbname=MYSQL_DB,
        network=network.name
    )
    .with_name(MYSQL_CONTAINER_NAME)
    .with_command("--sort_buffer_size=10M")
)

@fabricebaranski
Copy link
Author

It seems to work when I add .with_bind_ports(9099, 0)

@champialex
Copy link

champialex commented Aug 12, 2024

Confirming this does not work, for good reasons:

When calling docker_client.run in container.py#start, it doesn't pass the network through.
If you look at how the method behaves, it will then resort to host networking which is not meant to support port bindings. : if you run docker port on the created container, no port will show up.
By the time the network is connected (L102), it is too late.

Instead, this is what start should look like:

....
        self._container = docker_client.run(
            self.image,
            command=self._command,
            detach=True,
            environment=self.env,
            ports=self.ports,
            name=self._name,
            volumes=self.volumes,
            # New params
            network=network.name, 
            networking_config={network.name: EndpointConfig(version, aliases=self._network_aliases, **self._network_endpoint_configs) )
            **self._kwargs,
        )

In the meantime a workaround is:

    # replace:
    # ctr.with_network(network)
    # ctr.with_network_aliases("network_alias")
   # with:
    ctr.with_kwargs(
        network=network.name, networking_config={network.name: EndpointConfig("1.33", aliases=["network_alias"])}
    )

Also adding a quick reproducer for testing:

def kafka(network: Network):
    ctr = KafkaContainer(image=f"confluentinc/cp-kafka:7.6.1")
    ctr.with_network(network)
    ctr.with_network_aliases("kafka")
    # uncomment to fix
    # ctr.with_kwargs(network=network.name, networking_config={network.name: EndpointConfig("1.33", aliases=["kafka"])})
    with ctr:
        assert re.match("[^/:]+:[0-9]{4,5}", ctr.get_bootstrap_server()).group()

@alexanderankin
Copy link
Collaborator

this is what start should look like

open to a PR for this

@Lenormju
Copy link

I struggled for hours with the same problem, and got this to work (in Pytest's fixtures) :

MQTT_BROKER_NETWORK_ALIAS = "mqtt_broker"

@pytest.fixture()
def network_for_test() -> Generator[Network, None, None]:
    test_network = Network()
    with test_network:
        yield test_network


@pytest.fixture()
def mqtt_broker(network_for_test: Network) -> Generator[MosquittoContainer, None, None]:
    mqtt_broker = MosquittoContainer() \
        .with_network(network_for_test) \
        .with_network_aliases(MQTT_BROKER_NETWORK_ALIAS)
    with mqtt_broker:
        yield mqtt_broker


@pytest.fixture()
def web_server(mqtt_broker: MosquittoContainer,
               network_for_test: Network) -> Generator[ServerContainer, None, None]:
    # [...]
    web_server = ServerContainer(
        image="something", port=80
    ).with_network(network_for_test)
    # [...]
    with web_server:
        yield web_server

This way, I can make the web server connect to the MQTT broker (using the MQTT_BROKER_NETWORK_ALIAS).

@mgorsk1
Copy link
Contributor

mgorsk1 commented Aug 13, 2024

I've combined @champialex and mine findings into #678

alexanderankin pushed a commit that referenced this issue Aug 13, 2024
fixes #645
- network should be attached as the container is started, not as a
post-start action. This will make sure port binding and exposing works
correctly.

---------

Signed-off-by: mgorsk1 <[email protected]>
alexanderankin pushed a commit that referenced this issue Aug 13, 2024
fixes #645
- network should be attached as the container is started, not as a
post-start action. This will make sure port binding and exposing works
correctly.

---------

Signed-off-by: mgorsk1 <[email protected]>
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 a pull request may close this issue.

5 participants