diff options
author | Brad Warren <bmw@users.noreply.github.com> | 2022-10-20 23:07:18 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-20 23:07:18 +0300 |
commit | 3a738cadc36d1950b96de915ee6e3e8736ff258e (patch) | |
tree | b02534e1808cdb27d1978622939c4c3b6ea015f7 /certbot-ci | |
parent | 5270c34dd79e2dfc28017afc152d0ec68763fbcf (diff) |
Remove docker-compose dependency (#9436)
This is progress towards https://github.com/certbot/certbot/issues/9370 as discussed at https://github.com/certbot/certbot/pull/9435.
I kept the command using `docker-compose` because `docker compose` doesn't seem that widely recognized yet and https://www.docker.com/blog/announcing-compose-v2-general-availability/ describes aliasing `docker-compose` to `docker compose` on newer systems by default.
* refactor boulder shutdown
* remove docker-compose dep
* Reorder shutdown process
Diffstat (limited to 'certbot-ci')
5 files changed, 34 insertions, 16 deletions
diff --git a/certbot-ci/certbot_integration_tests/nginx_tests/context.py b/certbot-ci/certbot_integration_tests/nginx_tests/context.py index d52501596..9dd633db2 100644 --- a/certbot-ci/certbot_integration_tests/nginx_tests/context.py +++ b/certbot-ci/certbot_integration_tests/nginx_tests/context.py @@ -9,6 +9,7 @@ import pytest from certbot_integration_tests.certbot_tests import context as certbot_context from certbot_integration_tests.nginx_tests import nginx_config as config from certbot_integration_tests.utils import certbot_call +from certbot_integration_tests.utils import constants from certbot_integration_tests.utils import misc @@ -65,4 +66,4 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext): def _stop_nginx(self) -> None: assert self.process.poll() is None self.process.terminate() - self.process.wait() + self.process.wait(constants.MAX_SUBPROCESS_WAIT) diff --git a/certbot-ci/certbot_integration_tests/utils/acme_server.py b/certbot-ci/certbot_integration_tests/utils/acme_server.py index 8994d0ef0..c12057cd4 100755 --- a/certbot-ci/certbot_integration_tests/utils/acme_server.py +++ b/certbot-ci/certbot_integration_tests/utils/acme_server.py @@ -18,6 +18,7 @@ from typing import Dict from typing import List from typing import Mapping from typing import Optional +from typing import Tuple from typing import Type import requests @@ -63,6 +64,7 @@ class ACMEServer: self._stdout = sys.stdout if stdout else open(os.devnull, 'w') # pylint: disable=consider-using-with self._dns_server = dns_server self._http_01_port = http_01_port + self._preterminate_cmds_args: List[Tuple[Tuple[Any, ...], Dict[str, Any]]] = [] if http_01_port != DEFAULT_HTTP_01_PORT: if self._acme_type != 'pebble' or self._proxy: raise ValueError('setting http_01_port is not currently supported ' @@ -85,6 +87,7 @@ class ACMEServer: """Stop the test stack, and clean its resources""" print('=> Tear down the test infrastructure...') try: + self._run_preterminate_cmds() for process in self._processes: try: process.terminate() @@ -94,17 +97,7 @@ class ACMEServer: if e.errno != errno.ESRCH: raise for process in self._processes: - process.wait() - - if os.path.exists(os.path.join(self._workspace, 'boulder')): - # Boulder docker generates build artifacts owned by root with 0o744 permissions. - # If we started the acme server from a normal user that has access to the Docker - # daemon, this user will not be able to delete these artifacts from the host. - # We need to do it through a docker. - process = self._launch_process(['docker', 'run', '--rm', '-v', - '{0}:/workspace'.format(self._workspace), - 'alpine', 'rm', '-rf', '/workspace/boulder']) - process.wait() + process.wait(MAX_SUBPROCESS_WAIT) finally: if os.path.exists(self._workspace): shutil.rmtree(self._workspace) @@ -187,7 +180,7 @@ class ACMEServer: # Load Boulder from git, that includes a docker-compose.yml ready for production. process = self._launch_process(['git', 'clone', 'https://github.com/letsencrypt/boulder', '--single-branch', '--depth=1', instance_path]) - process.wait() + process.wait(MAX_SUBPROCESS_WAIT) # Allow Boulder to ignore usual limit rate policies, useful for tests. os.rename(join(instance_path, 'test/rate-limit-policies-b.yml'), @@ -202,6 +195,17 @@ class ACMEServer: with open(join(instance_path, 'test/config/va{}.json'.format(suffix)), 'w') as f: f.write(json.dumps(config, indent=2, separators=(',', ': '))) + # This command needs to be run before we try and terminate running processes because + # docker-compose up doesn't always respond to SIGTERM. See + # https://github.com/certbot/certbot/pull/9435. + self._register_preterminate_cmd(['docker-compose', 'down'], cwd=instance_path) + # Boulder docker generates build artifacts owned by root with 0o744 permissions. + # If we started the acme server from a normal user that has access to the Docker + # daemon, this user will not be able to delete these artifacts from the host. + # We need to do it through a docker. + self._register_preterminate_cmd(['docker', 'run', '--rm', '-v', + '{0}:/workspace'.format(self._workspace), 'alpine', 'rm', + '-rf', '/workspace/boulder']) try: # Launch the Boulder server self._launch_process(['docker-compose', 'up', '--force-recreate'], cwd=instance_path) @@ -224,7 +228,7 @@ class ACMEServer: process = self._launch_process([ 'docker-compose', 'logs'], cwd=instance_path, force_stderr=True ) - process.wait() + process.wait(MAX_SUBPROCESS_WAIT) raise print('=> Finished boulder instance deployment.') @@ -253,6 +257,17 @@ class ACMEServer: self._processes.append(process) return process + def _register_preterminate_cmd(self, *args: Any, **kwargs: Any) -> None: + self._preterminate_cmds_args.append((args, kwargs)) + + def _run_preterminate_cmds(self) -> None: + for args, kwargs in self._preterminate_cmds_args: + process = self._launch_process(*args, **kwargs) + process.wait(MAX_SUBPROCESS_WAIT) + # It's unlikely to matter, but let's clear the list of cleanup commands + # once they've been run. + self._preterminate_cmds_args.clear() + def main() -> None: # pylint: disable=missing-function-docstring diff --git a/certbot-ci/certbot_integration_tests/utils/constants.py b/certbot-ci/certbot_integration_tests/utils/constants.py index a788881ef..ce0cd91d5 100644 --- a/certbot-ci/certbot_integration_tests/utils/constants.py +++ b/certbot-ci/certbot_integration_tests/utils/constants.py @@ -9,3 +9,4 @@ PEBBLE_MANAGEMENT_URL = 'https://localhost:15000' PEBBLE_CHALLTESTSRV_URL = f'http://localhost:{CHALLTESTSRV_PORT}' MOCK_OCSP_SERVER_PORT = 4002 PEBBLE_ALTERNATE_ROOTS = 2 +MAX_SUBPROCESS_WAIT = 120 diff --git a/certbot-ci/certbot_integration_tests/utils/dns_server.py b/certbot-ci/certbot_integration_tests/utils/dns_server.py index 7dfc9c0b2..ed5b6ccae 100644 --- a/certbot-ci/certbot_integration_tests/utils/dns_server.py +++ b/certbot-ci/certbot_integration_tests/utils/dns_server.py @@ -17,6 +17,8 @@ from typing import Type from pkg_resources import resource_filename +from certbot_integration_tests.utils import constants + BIND_DOCKER_IMAGE = "internetsystemsconsortium/bind9:9.16" BIND_BIND_ADDRESS = ("127.0.0.1", 45953) @@ -67,7 +69,7 @@ class DNSServer: if self.process: try: self.process.terminate() - self.process.wait() + self.process.wait(constants.MAX_SUBPROCESS_WAIT) except BaseException as e: print("BIND9 did not stop cleanly: {}".format(e), file=sys.stderr) diff --git a/certbot-ci/setup.py b/certbot-ci/setup.py index 4341ef266..728c6c707 100644 --- a/certbot-ci/setup.py +++ b/certbot-ci/setup.py @@ -15,7 +15,6 @@ if parse_version(setuptools_version) < parse_version(min_setuptools_version): install_requires = [ 'coverage', 'cryptography', - 'docker-compose', 'pyopenssl', 'pytest', 'pytest-cov', |