diff options
author | Matt Clay <matt@mystile.com> | 2023-05-04 15:41:14 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-04 15:41:14 -0700 |
commit | cdeb607b1d01af4d8dd652bf7117a442fcd9b229 (patch) | |
tree | 495786d3b0603deb3582944bc87817c3ff0bec9d | |
parent | 2b6d5536d7b300bab4a1afd3382536ef178fc432 (diff) | |
download | ansible-cdeb607b1d01af4d8dd652bf7117a442fcd9b229.tar.gz |
ansible-test - Always use unique container names (#80724)
11 files changed, 68 insertions, 120 deletions
diff --git a/changelogs/fragments/ansible-test-unique-container-names.yml b/changelogs/fragments/ansible-test-unique-container-names.yml new file mode 100644 index 0000000000..560090d1aa --- /dev/null +++ b/changelogs/fragments/ansible-test-unique-container-names.yml @@ -0,0 +1,6 @@ +bugfixes: + - ansible-test - All containers created by ansible-test now include the current test session ID in their name. + This avoids conflicts between concurrent ansible-test invocations using the same container host. +breaking_changes: + - ansible-test - Test plugins that rely on containers no longer support reusing running containers. + The previous behavior was an undocumented, untested feature. diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/acme.py b/test/lib/ansible_test/_internal/commands/integration/cloud/acme.py index e8020ca9a9..136c533196 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/acme.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/acme.py @@ -8,7 +8,6 @@ from ....config import ( ) from ....containers import ( - CleanupMode, run_support_container, ) @@ -22,8 +21,6 @@ from . import ( class ACMEProvider(CloudProvider): """ACME plugin. Sets up cloud resources for tests.""" - DOCKER_SIMULATOR_NAME = 'acme-simulator' - def __init__(self, args: IntegrationConfig) -> None: super().__init__(args) @@ -51,17 +48,18 @@ class ACMEProvider(CloudProvider): 14000, # Pebble ACME CA ] - run_support_container( + descriptor = run_support_container( self.args, self.platform, self.image, - self.DOCKER_SIMULATOR_NAME, + 'acme-simulator', ports, - allow_existing=True, - cleanup=CleanupMode.YES, ) - self._set_cloud_config('acme_host', self.DOCKER_SIMULATOR_NAME) + if not descriptor: + return + + self._set_cloud_config('acme_host', descriptor.name) def _setup_static(self) -> None: raise NotImplementedError() diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py b/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py index 0b7ae61d2f..706c8ec19d 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py @@ -21,7 +21,6 @@ from ....docker_util import ( ) from ....containers import ( - CleanupMode, run_support_container, wait_for_file, ) @@ -36,8 +35,6 @@ from . import ( class CsCloudProvider(CloudProvider): """CloudStack cloud provider plugin. Sets up cloud resources before delegation.""" - DOCKER_SIMULATOR_NAME = 'cloudstack-sim' - def __init__(self, args: IntegrationConfig) -> None: super().__init__(args) @@ -96,10 +93,8 @@ class CsCloudProvider(CloudProvider): self.args, self.platform, self.image, - self.DOCKER_SIMULATOR_NAME, + 'cloudstack-sim', ports, - allow_existing=True, - cleanup=CleanupMode.YES, ) if not descriptor: @@ -107,7 +102,7 @@ class CsCloudProvider(CloudProvider): # apply work-around for OverlayFS issue # https://github.com/docker/for-linux/issues/72#issuecomment-319904698 - docker_exec(self.args, self.DOCKER_SIMULATOR_NAME, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';'], capture=True) + docker_exec(self.args, descriptor.name, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';'], capture=True) if self.args.explain: values = dict( @@ -115,10 +110,10 @@ class CsCloudProvider(CloudProvider): PORT=str(self.port), ) else: - credentials = self._get_credentials(self.DOCKER_SIMULATOR_NAME) + credentials = self._get_credentials(descriptor.name) values = dict( - HOST=self.DOCKER_SIMULATOR_NAME, + HOST=descriptor.name, PORT=str(self.port), KEY=credentials['apikey'], SECRET=credentials['secretkey'], diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/galaxy.py b/test/lib/ansible_test/_internal/commands/integration/cloud/galaxy.py index 1391cd8454..ebe8d1ca11 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/galaxy.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/galaxy.py @@ -25,10 +25,10 @@ from . import ( # We add BasicAuthentication, to make the tasks that deal with # direct API access easier to deal with across galaxy_ng and pulp -SETTINGS = b''' -CONTENT_ORIGIN = 'http://ansible-ci-pulp:80' -ANSIBLE_API_HOSTNAME = 'http://ansible-ci-pulp:80' -ANSIBLE_CONTENT_HOSTNAME = 'http://ansible-ci-pulp:80/pulp/content' +SETTINGS = ''' +CONTENT_ORIGIN = 'http://{host}:80' +ANSIBLE_API_HOSTNAME = 'http://{host}:80' +ANSIBLE_CONTENT_HOSTNAME = 'http://{host}:80/pulp/content' TOKEN_AUTH_DISABLED = True GALAXY_REQUIRE_CONTENT_APPROVAL = False GALAXY_AUTHENTICATION_CLASSES = [ @@ -97,7 +97,6 @@ class GalaxyProvider(CloudProvider): super().setup() galaxy_port = 80 - pulp_host = 'ansible-ci-pulp' pulp_port = 24817 ports = [ @@ -110,32 +109,31 @@ class GalaxyProvider(CloudProvider): self.args, self.platform, self.pulp, - pulp_host, + 'galaxy-pulp', ports, start=False, - allow_existing=True, ) if not descriptor: return - if not descriptor.running: - pulp_id = descriptor.container_id + pulp_id = descriptor.container_id - injected_files = { - '/etc/pulp/settings.py': SETTINGS, - '/etc/cont-init.d/111-postgres': SET_ADMIN_PASSWORD, - '/etc/cont-init.d/000-ansible-test-overrides': OVERRIDES, - } - for path, content in injected_files.items(): - with tempfile.NamedTemporaryFile() as temp_fd: - temp_fd.write(content) - temp_fd.flush() - docker_cp_to(self.args, pulp_id, temp_fd.name, path) + injected_files = { + '/etc/pulp/settings.py': SETTINGS.format(host=descriptor.name).encode(), + '/etc/cont-init.d/111-postgres': SET_ADMIN_PASSWORD, + '/etc/cont-init.d/000-ansible-test-overrides': OVERRIDES, + } - descriptor.start(self.args) + for path, content in injected_files.items(): + with tempfile.NamedTemporaryFile() as temp_fd: + temp_fd.write(content) + temp_fd.flush() + docker_cp_to(self.args, pulp_id, temp_fd.name, path) - self._set_cloud_config('PULP_HOST', pulp_host) + descriptor.start(self.args) + + self._set_cloud_config('PULP_HOST', descriptor.name) self._set_cloud_config('PULP_PORT', str(pulp_port)) self._set_cloud_config('GALAXY_PORT', str(galaxy_port)) self._set_cloud_config('PULP_USER', 'admin') diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/httptester.py b/test/lib/ansible_test/_internal/commands/integration/cloud/httptester.py index 85065d6f3b..b3cf2d4905 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/httptester.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/httptester.py @@ -13,7 +13,6 @@ from ....config import ( ) from ....containers import ( - CleanupMode, run_support_container, ) @@ -62,8 +61,6 @@ class HttptesterProvider(CloudProvider): 'http-test-container', ports, aliases=aliases, - allow_existing=True, - cleanup=CleanupMode.YES, env={ KRB5_PASSWORD_ENV: generate_password(), }, diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/nios.py b/test/lib/ansible_test/_internal/commands/integration/cloud/nios.py index a515ae89fa..cc54720836 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/nios.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/nios.py @@ -8,7 +8,6 @@ from ....config import ( ) from ....containers import ( - CleanupMode, run_support_container, ) @@ -22,8 +21,6 @@ from . import ( class NiosProvider(CloudProvider): """Nios plugin. Sets up NIOS mock server for tests.""" - DOCKER_SIMULATOR_NAME = 'nios-simulator' - # Default image to run the nios simulator. # # The simulator must be pinned to a specific version @@ -65,17 +62,18 @@ class NiosProvider(CloudProvider): nios_port, ] - run_support_container( + descriptor = run_support_container( self.args, self.platform, self.image, - self.DOCKER_SIMULATOR_NAME, + 'nios-simulator', ports, - allow_existing=True, - cleanup=CleanupMode.YES, ) - self._set_cloud_config('NIOS_HOST', self.DOCKER_SIMULATOR_NAME) + if not descriptor: + return + + self._set_cloud_config('NIOS_HOST', descriptor.name) def _setup_static(self) -> None: raise NotImplementedError() diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/openshift.py b/test/lib/ansible_test/_internal/commands/integration/cloud/openshift.py index ddd434a817..6e8a5e4fdd 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/openshift.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/openshift.py @@ -16,7 +16,6 @@ from ....config import ( ) from ....containers import ( - CleanupMode, run_support_container, wait_for_file, ) @@ -31,8 +30,6 @@ from . import ( class OpenShiftCloudProvider(CloudProvider): """OpenShift cloud provider plugin. Sets up cloud resources before delegation.""" - DOCKER_CONTAINER_NAME = 'openshift-origin' - def __init__(self, args: IntegrationConfig) -> None: super().__init__(args, config_extension='.kubeconfig') @@ -74,10 +71,8 @@ class OpenShiftCloudProvider(CloudProvider): self.args, self.platform, self.image, - self.DOCKER_CONTAINER_NAME, + 'openshift-origin', ports, - allow_existing=True, - cleanup=CleanupMode.YES, cmd=cmd, ) @@ -87,7 +82,7 @@ class OpenShiftCloudProvider(CloudProvider): if self.args.explain: config = '# Unknown' else: - config = self._get_config(self.DOCKER_CONTAINER_NAME, 'https://%s:%s/' % (self.DOCKER_CONTAINER_NAME, port)) + config = self._get_config(descriptor.name, 'https://%s:%s/' % (descriptor.name, port)) self._write_config(config) diff --git a/test/lib/ansible_test/_internal/containers.py b/test/lib/ansible_test/_internal/containers.py index bfc36434dd..452cbc1ddd 100644 --- a/test/lib/ansible_test/_internal/containers.py +++ b/test/lib/ansible_test/_internal/containers.py @@ -4,7 +4,6 @@ from __future__ import annotations import atexit import collections.abc as c import contextlib -import enum import json import random import time @@ -46,6 +45,7 @@ from .docker_util import ( get_docker_container_id, get_docker_host_ip, get_podman_host_ip, + get_session_container_name, require_docker, detect_host_properties, ) @@ -101,14 +101,6 @@ class HostType: managed = 'managed' -class CleanupMode(enum.Enum): - """How container cleanup should be handled.""" - - YES = enum.auto() - NO = enum.auto() - INFO = enum.auto() - - def run_support_container( args: EnvironmentConfig, context: str, @@ -117,8 +109,7 @@ def run_support_container( ports: list[int], aliases: t.Optional[list[str]] = None, start: bool = True, - allow_existing: bool = False, - cleanup: t.Optional[CleanupMode] = None, + cleanup: bool = True, cmd: t.Optional[list[str]] = None, env: t.Optional[dict[str, str]] = None, options: t.Optional[list[str]] = None, @@ -128,6 +119,8 @@ def run_support_container( Start a container used to support tests, but not run them. Containers created this way will be accessible from tests. """ + name = get_session_container_name(args, name) + if args.prime_containers: docker_pull(args, image) return None @@ -165,46 +158,13 @@ def run_support_container( options.extend(['--ulimit', 'nofile=%s' % max_open_files]) - support_container_id = None - - if allow_existing: - try: - container = docker_inspect(args, name) - except ContainerNotFoundError: - container = None - - if container: - support_container_id = container.id - - if not container.running: - display.info('Ignoring existing "%s" container which is not running.' % name, verbosity=1) - support_container_id = None - elif not container.image: - display.info('Ignoring existing "%s" container which has the wrong image.' % name, verbosity=1) - support_container_id = None - elif publish_ports and not all(port and len(port) == 1 for port in [container.get_tcp_port(port) for port in ports]): - display.info('Ignoring existing "%s" container which does not have the required published ports.' % name, verbosity=1) - support_container_id = None - - if not support_container_id: - docker_rm(args, name) - if args.dev_systemd_debug: options.extend(('--env', 'SYSTEMD_LOG_LEVEL=debug')) - if support_container_id: - display.info('Using existing "%s" container.' % name) - running = True - existing = True - else: - display.info('Starting new "%s" container.' % name) - docker_pull(args, image) - support_container_id = run_container(args, image, name, options, create_only=not start, cmd=cmd) - running = start - existing = False - - if cleanup is None: - cleanup = CleanupMode.INFO if existing else CleanupMode.YES + display.info('Starting new "%s" container.' % name) + docker_pull(args, image) + support_container_id = run_container(args, image, name, options, create_only=not start, cmd=cmd) + running = start descriptor = ContainerDescriptor( image, @@ -215,7 +175,6 @@ def run_support_container( aliases, publish_ports, running, - existing, cleanup, env, ) @@ -694,8 +653,7 @@ class ContainerDescriptor: aliases: list[str], publish_ports: bool, running: bool, - existing: bool, - cleanup: CleanupMode, + cleanup: bool, env: t.Optional[dict[str, str]], ) -> None: self.image = image @@ -706,7 +664,6 @@ class ContainerDescriptor: self.aliases = aliases self.publish_ports = publish_ports self.running = running - self.existing = existing self.cleanup = cleanup self.env = env self.details: t.Optional[SupportContainer] = None @@ -805,10 +762,8 @@ def wait_for_file( def cleanup_containers(args: EnvironmentConfig) -> None: """Clean up containers.""" for container in support_containers.values(): - if container.cleanup == CleanupMode.YES: - docker_rm(args, container.container_id) - elif container.cleanup == CleanupMode.INFO: - display.notice(f'Remember to run `{require_docker().command} rm -f {container.name}` when finished testing.') + if container.cleanup: + docker_rm(args, container.name) def create_hosts_entries(context: dict[str, ContainerAccess]) -> list[str]: diff --git a/test/lib/ansible_test/_internal/docker_util.py b/test/lib/ansible_test/_internal/docker_util.py index 06f383b588..52b9691ed5 100644 --- a/test/lib/ansible_test/_internal/docker_util.py +++ b/test/lib/ansible_test/_internal/docker_util.py @@ -300,7 +300,7 @@ def detect_host_properties(args: CommonConfig) -> ContainerHostProperties: options = ['--volume', '/sys/fs/cgroup:/probe:ro'] cmd = ['sh', '-c', ' && echo "-" && '.join(multi_line_commands)] - stdout = run_utility_container(args, f'ansible-test-probe-{args.session_name}', cmd, options)[0] + stdout = run_utility_container(args, 'ansible-test-probe', cmd, options)[0] if args.explain: return ContainerHostProperties( @@ -336,7 +336,7 @@ def detect_host_properties(args: CommonConfig) -> ContainerHostProperties: cmd = ['sh', '-c', 'ulimit -Hn'] try: - stdout = run_utility_container(args, f'ansible-test-ulimit-{args.session_name}', cmd, options)[0] + stdout = run_utility_container(args, 'ansible-test-ulimit', cmd, options)[0] except SubprocessError as ex: display.warning(str(ex)) else: @@ -402,6 +402,11 @@ def detect_host_properties(args: CommonConfig) -> ContainerHostProperties: return properties +def get_session_container_name(args: CommonConfig, name: str) -> str: + """Return the given container name with the current test session name applied to it.""" + return f'{name}-{args.session_name}' + + def run_utility_container( args: CommonConfig, name: str, @@ -410,6 +415,8 @@ def run_utility_container( data: t.Optional[str] = None, ) -> tuple[t.Optional[str], t.Optional[str]]: """Run the specified command using the ansible-test utility container, returning stdout and stderr.""" + name = get_session_container_name(args, name) + options = options + [ '--name', name, '--rm', diff --git a/test/lib/ansible_test/_internal/host_profiles.py b/test/lib/ansible_test/_internal/host_profiles.py index a51eb69387..33bd6640f5 100644 --- a/test/lib/ansible_test/_internal/host_profiles.py +++ b/test/lib/ansible_test/_internal/host_profiles.py @@ -99,7 +99,6 @@ from .ansible_util import ( ) from .containers import ( - CleanupMode, HostType, get_container_database, run_support_container, @@ -447,7 +446,7 @@ class DockerProfile(ControllerHostProfile[DockerConfig], SshTargetHostProfile[Do @property def label(self) -> str: """Label to apply to resources related to this profile.""" - return f'{"controller" if self.controller else "target"}-{self.args.session_name}' + return f'{"controller" if self.controller else "target"}' def provision(self) -> None: """Provision the host before delegation.""" @@ -462,7 +461,7 @@ class DockerProfile(ControllerHostProfile[DockerConfig], SshTargetHostProfile[Do ports=[22], publish_ports=not self.controller, # connections to the controller over SSH are not required options=init_config.options, - cleanup=CleanupMode.NO, + cleanup=False, cmd=self.build_init_command(init_config, init_probe), ) @@ -838,7 +837,7 @@ class DockerProfile(ControllerHostProfile[DockerConfig], SshTargetHostProfile[Do """Check the cgroup v1 systemd hierarchy to verify it is writeable for our container.""" probe_script = (read_text_file(os.path.join(ANSIBLE_TEST_TARGET_ROOT, 'setup', 'check_systemd_cgroup_v1.sh')) .replace('@MARKER@', self.MARKER) - .replace('@LABEL@', self.label)) + .replace('@LABEL@', f'{self.label}-{self.args.session_name}')) cmd = ['sh'] @@ -853,7 +852,7 @@ class DockerProfile(ControllerHostProfile[DockerConfig], SshTargetHostProfile[Do def create_systemd_cgroup_v1(self) -> str: """Create a unique ansible-test cgroup in the v1 systemd hierarchy and return its path.""" - self.cgroup_path = f'/sys/fs/cgroup/systemd/ansible-test-{self.label}' + self.cgroup_path = f'/sys/fs/cgroup/systemd/ansible-test-{self.label}-{self.args.session_name}' # Privileged mode is required to create the cgroup directories on some hosts, such as Fedora 36 and RHEL 9.0. # The mkdir command will fail with "Permission denied" otherwise. diff --git a/test/lib/ansible_test/_internal/pypi_proxy.py b/test/lib/ansible_test/_internal/pypi_proxy.py index 97663eadd1..c37ef8ec15 100644 --- a/test/lib/ansible_test/_internal/pypi_proxy.py +++ b/test/lib/ansible_test/_internal/pypi_proxy.py @@ -76,7 +76,7 @@ def run_pypi_proxy(args: EnvironmentConfig, targets_use_pypi: bool) -> None: args=args, context='__pypi_proxy__', image=image, - name=f'pypi-test-container-{args.session_name}', + name='pypi-test-container', ports=[port], ) |