From 93c04dce8df3025f77912c5d93d99e619172266c Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Fri, 31 Dec 2021 20:43:02 +1300 Subject: [PATCH 1/7] fix: Properly detect pytest running via container The original `/.dockerenv` approach is no longer valid, and context wise we're only using this for the test suite, so using an ENV in that container is a better solution. --- test/conftest.py | 10 +++++----- test/requirements/Dockerfile-nginx-proxy-tester | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 3e0f3af..438210e 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -23,7 +23,7 @@ logging.getLogger('DNS').setLevel(logging.DEBUG) logging.getLogger('requests.packages.urllib3.connectionpool').setLevel(logging.WARN) CA_ROOT_CERTIFICATE = os.path.join(os.path.dirname(__file__), 'certs/ca-root.crt') -I_AM_RUNNING_INSIDE_A_DOCKER_CONTAINER = os.path.isfile("/.dockerenv") +PYTEST_RUNNING_IN_CONTAINER = os.environ.get('PYTEST_RUNNING_IN_CONTAINER') == "1" FORCE_CONTAINER_IPV6 = False # ugly global state to consider containers' IPv6 address instead of IPv4 @@ -259,7 +259,7 @@ def restore_urllib_dns_resolver(getaddrinfo_func): def remove_all_containers(): for container in docker_client.containers.list(all=True): - if I_AM_RUNNING_INSIDE_A_DOCKER_CONTAINER and container.id.startswith(socket.gethostname()): + if PYTEST_RUNNING_IN_CONTAINER and container.id.startswith(socket.gethostname()): continue # pytest is running within a Docker container, so we do not want to remove that particular container logging.info(f"removing container {container.name}") container.remove(v=True, force=True) @@ -349,7 +349,7 @@ def connect_to_network(network): :return: the name of the network we were connected to, or None """ - if I_AM_RUNNING_INSIDE_A_DOCKER_CONTAINER: + if PYTEST_RUNNING_IN_CONTAINER: try: my_container = docker_client.containers.get(socket.gethostname()) except docker.errors.NotFound: @@ -372,7 +372,7 @@ def disconnect_from_network(network=None): :param network: name of a docker network to disconnect from """ - if I_AM_RUNNING_INSIDE_A_DOCKER_CONTAINER and network is not None: + if PYTEST_RUNNING_IN_CONTAINER and network is not None: try: my_container = docker_client.containers.get(socket.gethostname()) except docker.errors.NotFound: @@ -394,7 +394,7 @@ def connect_to_all_networks(): :return: a list of networks we connected to """ - if not I_AM_RUNNING_INSIDE_A_DOCKER_CONTAINER: + if not PYTEST_RUNNING_IN_CONTAINER: return [] else: # find the list of docker networks diff --git a/test/requirements/Dockerfile-nginx-proxy-tester b/test/requirements/Dockerfile-nginx-proxy-tester index 3c25c0c..36984fe 100644 --- a/test/requirements/Dockerfile-nginx-proxy-tester +++ b/test/requirements/Dockerfile-nginx-proxy-tester @@ -1,5 +1,7 @@ FROM python:3.9 +ENV PYTEST_RUNNING_IN_CONTAINER=1 + COPY python-requirements.txt /requirements.txt RUN pip install -r /requirements.txt From e748d53a1f614b5210143e00202daf6c1219e127 Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Fri, 31 Dec 2021 21:51:40 +1300 Subject: [PATCH 2/7] chore: Extract hostname access to a var DRY and clearer that we're referring to the pytest container. --- test/conftest.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 438210e..2212ba4 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -28,6 +28,7 @@ FORCE_CONTAINER_IPV6 = False # ugly global state to consider containers' IPv6 a docker_client = docker.from_env() +test_container = socket.gethostname() ############################################################################### @@ -259,7 +260,7 @@ def restore_urllib_dns_resolver(getaddrinfo_func): def remove_all_containers(): for container in docker_client.containers.list(all=True): - if PYTEST_RUNNING_IN_CONTAINER and container.id.startswith(socket.gethostname()): + if PYTEST_RUNNING_IN_CONTAINER and container.id.startswith(test_container): continue # pytest is running within a Docker container, so we do not want to remove that particular container logging.info(f"removing container {container.name}") container.remove(v=True, force=True) @@ -351,9 +352,9 @@ def connect_to_network(network): """ if PYTEST_RUNNING_IN_CONTAINER: try: - my_container = docker_client.containers.get(socket.gethostname()) + my_container = docker_client.containers.get(test_container) except docker.errors.NotFound: - logging.warn(f"container {socket.gethostname()!r} not found") + logging.warn(f"container {test_container} not found") return # figure out our container networks @@ -374,9 +375,9 @@ def disconnect_from_network(network=None): """ if PYTEST_RUNNING_IN_CONTAINER and network is not None: try: - my_container = docker_client.containers.get(socket.gethostname()) + my_container = docker_client.containers.get(test_container) except docker.errors.NotFound: - logging.warn(f"container {socket.gethostname()!r} not found") + logging.warn(f"container {test_container} not found") return # figure out our container networks From b2b4c7199730baa86996fececaedeb70d4618198 Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Fri, 31 Dec 2021 22:12:25 +1300 Subject: [PATCH 3/7] fix: Don't remove pytest container when running with host network mode When the container runs with host networking instead of the default bridge, the `$HOSTNAME` / `/etc/hostname` reflects that of the host instead of the container ID , which causes the pytest container to get removed accidentally. Using a container name instead we can more reliably target the container to avoid removing it, should we need to run with host networking instead. --- test/conftest.py | 6 ++++-- test/pytest.sh | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 2212ba4..b8db538 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -28,7 +28,9 @@ FORCE_CONTAINER_IPV6 = False # ugly global state to consider containers' IPv6 a docker_client = docker.from_env() -test_container = socket.gethostname() + +# Name of pytest container to reference if it's being used for running tests +test_container = 'nginx-proxy-pytest' ############################################################################### @@ -260,7 +262,7 @@ def restore_urllib_dns_resolver(getaddrinfo_func): def remove_all_containers(): for container in docker_client.containers.list(all=True): - if PYTEST_RUNNING_IN_CONTAINER and container.id.startswith(test_container): + if PYTEST_RUNNING_IN_CONTAINER and container.name == test_container: continue # pytest is running within a Docker container, so we do not want to remove that particular container logging.info(f"removing container {container.name}") container.remove(v=True, force=True) diff --git a/test/pytest.sh b/test/pytest.sh index 99c054c..28275e5 100755 --- a/test/pytest.sh +++ b/test/pytest.sh @@ -18,8 +18,8 @@ docker build -t nginx-proxy-tester -f "${DIR}/requirements/Dockerfile-nginx-prox # run the nginx-proxy-tester container setting the correct value for the working dir in order for # docker-compose to work properly when run from within that container. -exec docker run --rm -it \ ---volume /var/run/docker.sock:/var/run/docker.sock \ ---volume "${DIR}:${DIR}" \ ---workdir "${DIR}" \ -nginx-proxy-tester "${ARGS[@]}" \ No newline at end of file +exec docker run --rm -it --name "nginx-proxy-pytest" \ + --volume "/var/run/docker.sock:/var/run/docker.sock" \ + --volume "${DIR}:${DIR}" \ + --workdir "${DIR}" \ + nginx-proxy-tester "${ARGS[@]}" \ No newline at end of file From 0e5d97a268a7dac6151631ce664cadf201b84fa9 Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Fri, 31 Dec 2021 22:14:26 +1300 Subject: [PATCH 4/7] fix: Don't connect pytest container to networks when using host network This is not compatible or required, since host networking is no longer isolated to container networks only. --- test/conftest.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/conftest.py b/test/conftest.py index b8db538..3f5f04c 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -362,6 +362,10 @@ def connect_to_network(network): # figure out our container networks my_networks = list(my_container.attrs["NetworkSettings"]["Networks"].keys()) + # If the pytest container is using host networking, it cannot connect to container networks (not required with host network) + if 'host' in my_networks: + return None + # make sure our container is connected to the nginx-proxy's network if network not in my_networks: logging.info(f"Connecting to docker network: {network.name}") From 04b0181980c0ec51accede0618a53788959e3d95 Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Fri, 31 Dec 2021 22:30:49 +1300 Subject: [PATCH 5/7] fix: Ensure networks are actually connected to pytest container The `network` object would never be in a list of network names (strings), and without `greedy=True` arg as the `docker-py` API docs note, the containers will not be part of the results, thus always returning an empty list which was not intended.. Now the network will properly match the current networks for pytest container, avoiding duplicate connect attempts, and the network list result will actually have containers to count when filtering by length. --- test/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 3f5f04c..fce4cde 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -367,7 +367,7 @@ def connect_to_network(network): return None # make sure our container is connected to the nginx-proxy's network - if network not in my_networks: + if network.name not in my_networks: logging.info(f"Connecting to docker network: {network.name}") network.connect(my_container) return network @@ -405,7 +405,7 @@ def connect_to_all_networks(): return [] else: # find the list of docker networks - networks = [network for network in docker_client.networks.list() if len(network.containers) > 0 and network.name != 'bridge'] + networks = [network for network in docker_client.networks.list(greedy=True) if len(network.containers) > 0 and network.name != 'bridge'] return [connect_to_network(network) for network in networks] From 115461744b931f81f175b5f2ed3b17e74179e1f8 Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Sat, 1 Jan 2022 01:38:13 +1300 Subject: [PATCH 6/7] fix: Skip IPv6 when forced but not available + avoid `none` network A test on raw IP addresses doesn't reach the existing IPv6 skip logic, added that to avoid a test failing when only IPv4 is available (eg: standard docker container networks). Additionally some other tests set the `none` network and connecting to this fails as it's not allowed? Preventing that from happening resolves the final failing tests within containerized pytest. --- test/conftest.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index fce4cde..dda8379 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -239,6 +239,11 @@ def monkey_patch_urllib_dns_resolver(): logging.getLogger('DNS').debug(f"resolving domain name {repr(args)}") _args = list(args) + # Fail early when querying IP directly and it is forced ipv6 when not supported, + # Otherwise a pytest container not using the host network fails to pass `test_raw-ip-vhost`. + if FORCE_CONTAINER_IPV6 and not HAS_IPV6: + pytest.skip("This system does not support IPv6") + # custom DNS resolvers ip = nginx_proxy_dns_resolver(args[0]) if ip is None: @@ -366,8 +371,9 @@ def connect_to_network(network): if 'host' in my_networks: return None - # make sure our container is connected to the nginx-proxy's network - if network.name not in my_networks: + # Make sure our container is connected to the nginx-proxy's network, + # but avoid connecting to `none` network (not valid) with `test_server-down` tests + if network.name not in my_networks and network.name != 'none': logging.info(f"Connecting to docker network: {network.name}") network.connect(my_container) return network From 6b3ee66783889599f96c43807792e15c2b01bb0d Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Sat, 1 Jan 2022 01:39:51 +1300 Subject: [PATCH 7/7] chore: white-space housekeeping Noticed some trailing white-space. Removed for consistency with the rest of the file. --- test/conftest.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index dda8379..cf26b30 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -34,9 +34,9 @@ test_container = 'nginx-proxy-pytest' ############################################################################### -# +# # utilities -# +# ############################################################################### @contextlib.contextmanager @@ -60,7 +60,7 @@ def ipv6(force_ipv6=True): class requests_for_docker(object): """ - Proxy for calling methods of the requests module. + Proxy for calling methods of the requests module. When a HTTP response failed due to HTTP Error 404 or 502, retry a few times. Provides method `get_conf` to extract the nginx-proxy configuration content. """ @@ -224,7 +224,7 @@ def docker_container_dns_resolver(domain_name): ip = container_ip(container) log.info(f"resolving domain name {domain_name!r} as IP address {ip} of container {container.name}") - return ip + return ip def monkey_patch_urllib_dns_resolver(): @@ -306,7 +306,7 @@ def docker_compose_down(compose_file='docker-compose.yml'): def wait_for_nginxproxy_to_be_ready(): """ - If one (and only one) container started from image nginxproxy/nginx-proxy:test is found, + If one (and only one) container started from image nginxproxy/nginx-proxy:test is found, wait for its log to contain substring "Watching docker events" """ containers = docker_client.containers.list(filters={"ancestor": "nginxproxy/nginx-proxy:test"}) @@ -416,18 +416,18 @@ def connect_to_all_networks(): ############################################################################### -# +# # Py.test fixtures -# +# ############################################################################### @pytest.fixture(scope="module") def docker_compose(request): """ pytest fixture providing containers described in a docker compose file. After the tests, remove the created containers - + A custom docker compose file name can be defined in a variable named `docker_compose_file`. - + Also, in the case where pytest is running from a docker container, this fixture makes sure our container will be attached to all the docker networks. """ @@ -463,9 +463,9 @@ def nginxproxy(): ############################################################################### -# +# # Py.test hooks -# +# ############################################################################### # pytest hook to display additionnal stuff in test report @@ -492,9 +492,9 @@ def pytest_runtest_setup(item): pytest.xfail(f"previous test failed ({previousfailed.name})") ############################################################################### -# +# # Check requirements -# +# ############################################################################### try: