Skip to content

feat(network): Add network context manager #367

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

Merged
merged 23 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9c3ecc8
Add network context manager
mloesch Jul 5, 2023
97ad2b8
Merge branch 'main' into network
mloesch Nov 23, 2023
c10223f
Use random names for networks
mloesch Nov 27, 2023
7542743
Merge branch 'main' into network
kiview Nov 28, 2023
e4d9c19
Add explicit API DockerContainer.with_network(Network)
mloesch Nov 28, 2023
c757142
Add support for network aliases
mloesch Nov 28, 2023
b076309
Merge remote-tracking branch 'origin/main' into network
mloesch Nov 29, 2023
8c2f8b9
Merge branch 'main' into network
mloesch Feb 6, 2024
b523301
Merge remote-tracking branch 'origin/main' into network
mloesch Mar 12, 2024
9093c6b
Merge remote-tracking branch 'origin/main' into network
mloesch Apr 4, 2024
1c8ebdc
chore: change return type to Self
mloesch Apr 4, 2024
cac19d3
chore: further specify type
mloesch Apr 8, 2024
b392ca6
chore: clarify Network init signature
mloesch Apr 8, 2024
8c8b6e2
chore: remove __del__hook
mloesch Apr 8, 2024
a64625c
chore(codestyle): switch to `ruff` from `black` for code formatting (…
max-pfeiffer Apr 5, 2024
a00d8dd
fix(vault): add support for HashiCorp Vault container (#366)
f4z3r Apr 5, 2024
065f707
fix(core): make config editable to avoid monkeypatching.1 (#532)
alexanderankin Apr 8, 2024
ba33506
chore(main): release testcontainers 4.3.2 (#530)
github-actions[bot] Apr 8, 2024
9db1080
chore: pin nginx image
mloesch Apr 8, 2024
9426254
fix class style
mloesch Apr 8, 2024
3fc6bcf
Merge remote-tracking branch 'origin/main' into network
mloesch Apr 8, 2024
66e2584
Merge branch 'main' into network
santi Apr 15, 2024
2e50cb3
chore: reformat
mloesch Apr 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions core/testcontainers/core/network.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import uuid
from typing import Optional

from testcontainers.core.docker_client import DockerClient


class Network(object):
"""
Network context manager for programmatically connecting containers.
"""

def __init__(self, docker_client_kw: Optional[dict] = None, **kwargs) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

From an API users' point of view, it is not really clear what the kwargs arguments are meant for. Could you update the signature to be def __init__(self, docker_client_kw: Optional[dict] = None, docker_network_kw: Optional[dict] = None) -> None:?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in b392ca6

self.name = str(uuid.uuid4())
self._docker = DockerClient(**(docker_client_kw or {}))
self._kwargs = kwargs

def remove(self) -> None:
self._network.remove()

def __enter__(self) -> 'Network':
self._network = self._docker.client.networks.create(self.name, **self._kwargs)
self.id = self._network.id
return self

def __exit__(self, exc_type, exc_val, exc_tb) -> None:
self.remove()

def __del__(self) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We've had some bad experiences with external resource cleanup through the __del__ method, so we have decided to move away from this. Having the context manager and the remove method provides sufficient means to cleanup the resource. __del__ hook can be removed 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 8c8b6e2

if self._network is not None:
try:
self.remove()
except: # noqa: E722
pass
27 changes: 27 additions & 0 deletions core/tests/test_network.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from testcontainers.core.container import DockerContainer
from testcontainers.core.docker_client import DockerClient
from testcontainers.core.network import Network


def test_network_gets_created_and_cleaned_up():
with Network() as network:
docker = DockerClient()
networks_list = docker.client.networks.list(network.name)
assert networks_list[0].name == network.name
assert networks_list[0].id == network.id
assert not docker.client.networks.list(network.name)


def test_containers_can_communicate_over_network():
with Network() as network:
with DockerContainer("nginx:alpine-slim").with_name(
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using with_name (which can lead to naming conflicts when parallelizing tests or accidentally reusing names), can we add an API with_network_aliases(string[]), similar to tc-java, that uses Docker's --network-alias mechanism?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kiview thanks for the pointer! Added this in c757142

"alpine1").with_kwargs(network=network.name) as alpine1:
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to use with_kwargs here and not have an explicit API with_network(Network)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 added in e4d9c19

with DockerContainer("nginx:alpine-slim").with_name(
"alpine2").with_kwargs(network=network.name) as alpine2:
status, output = alpine1.exec("ping -c 1 alpine2")
assert status == 0
assert "64 bytes" in str(output)

status, output = alpine2.exec("ping -c 1 alpine1")
assert status == 0
assert "64 bytes" in str(output)