From b25e1b1e952e0901d9ef19da1673b60e674ad4e3 Mon Sep 17 00:00:00 2001 From: ckunki Date: Mon, 4 Dec 2023 08:17:21 +0100 Subject: [PATCH 1/9] Supported pushing Docker image to a Docker registry --- doc/changes/changes_0.1.0.md | 1 + exasol/ds/sandbox/cli/cli.py | 9 ++ .../cli/commands/create_docker_image.py | 35 +++-- exasol/ds/sandbox/lib/dss_docker/__init__.py | 2 +- .../ds/sandbox/lib/dss_docker/create_image.py | 17 ++- .../ds/sandbox/lib/dss_docker/push_image.py | 79 ++++++++++ .../runtime/ansible/dss_docker_playbook.yml | 4 +- test/integration/conftest.py | 35 +++++ .../test_create_dss_docker_image.py | 26 ---- test/integration/test_push_docker_image.py | 136 ++++++++++++++++++ test/ports.py | 30 ++++ 11 files changed, 330 insertions(+), 44 deletions(-) create mode 100644 exasol/ds/sandbox/lib/dss_docker/push_image.py create mode 100644 test/integration/test_push_docker_image.py create mode 100644 test/ports.py diff --git a/doc/changes/changes_0.1.0.md b/doc/changes/changes_0.1.0.md index b805652b..73611329 100644 --- a/doc/changes/changes_0.1.0.md +++ b/doc/changes/changes_0.1.0.md @@ -24,6 +24,7 @@ Version: 0.1.0 * #23: Fixed AWS Code build * #69: Added entry point to start Jupyter server * #84: Fixed retrieval and display of jupyter password +* #36: Supported pushing Docker image to a Docker registry ## Bug Fixes diff --git a/exasol/ds/sandbox/cli/cli.py b/exasol/ds/sandbox/cli/cli.py index 21966a2e..8f5d9e1e 100644 --- a/exasol/ds/sandbox/cli/cli.py +++ b/exasol/ds/sandbox/cli/cli.py @@ -1,6 +1,15 @@ import click +import os @click.group() def cli(): pass + + +def option_with_env_default(envvar: str, *args, **kwargs): + kwargs["help"] = f"{kwargs['help']} [defaults to environment variable '{envvar}']" + return click.option( + *args, **kwargs, + default=lambda: os.environ.get(envvar, ""), + ) diff --git a/exasol/ds/sandbox/cli/commands/create_docker_image.py b/exasol/ds/sandbox/cli/commands/create_docker_image.py index 16ffb587..d63c118b 100644 --- a/exasol/ds/sandbox/cli/commands/create_docker_image.py +++ b/exasol/ds/sandbox/cli/commands/create_docker_image.py @@ -1,9 +1,9 @@ import click -from exasol.ds.sandbox.cli.cli import cli +from exasol.ds.sandbox.cli.cli import cli, option_with_env_default from exasol.ds.sandbox.cli.options.logging import logging_options from exasol.ds.sandbox.cli.common import add_options -from exasol.ds.sandbox.lib.dss_docker import DssDockerImage +from exasol.ds.sandbox.lib.dss_docker import DssDockerImage, DockerRegistry from exasol.ds.sandbox.lib.logging import SUPPORTED_LOG_LEVELS from exasol.ds.sandbox.lib.logging import set_log_level @@ -14,10 +14,15 @@ '--repository', type=str, metavar="ORG/REPO", show_default=True, default="exasol/data-science-sandbox", help="Organization and repository on hub.docker.com to publish the docker image to"), - click.option('--version', type=str, help="Docker image version tag"), click.option( - '--publish', type=bool, is_flag=True, - help="Whether to publish the created Docker image"), + '--version', type=str, metavar="VERSION", + help="Docker image version tag"), + option_with_env_default("DOCKER_REGISTRY_USER", + '--registry-user', type=str, metavar="USER", + help="Username for publication to Docker registry"), + option_with_env_default("DOCKER_REGISTRY_PASSWORD", + '--registry-password', type=str, metavar="PASSWORD", + help="Password for publication to Docker registry"), click.option( '--keep-container', type=bool, is_flag=True, help="""Keep the Docker Container running after creating the image. @@ -27,18 +32,26 @@ def create_docker_image( repository: str, version: str, - publish: bool, + registry_user: str, + registry_password: str, keep_container: bool, log_level: str, ): """ - Create a Docker image for data-science-sandbox and deploy - it to a Docker repository. + Create a Docker image for data-science-sandbox. If username and password + for the Docker registry are specified then deploy the image to the registry. """ set_log_level(log_level) - DssDockerImage( + creator = DssDockerImage( repository=repository, version=version, - publish=publish, keep_container=keep_container, - ).create() + ) + if registry_user and registry_password: + creator.registry = DockerRegistry( + creator.repository, + registry_user, + registry_password, + ) + print(f'user {creator.registry.username} password {registry_password}') + # creator.create() diff --git a/exasol/ds/sandbox/lib/dss_docker/__init__.py b/exasol/ds/sandbox/lib/dss_docker/__init__.py index f62d46be..a9d32e27 100644 --- a/exasol/ds/sandbox/lib/dss_docker/__init__.py +++ b/exasol/ds/sandbox/lib/dss_docker/__init__.py @@ -1 +1 @@ -from .create_image import DssDockerImage +from .create_image import DssDockerImage, DockerRegistry diff --git a/exasol/ds/sandbox/lib/dss_docker/create_image.py b/exasol/ds/sandbox/lib/dss_docker/create_image.py index fba83070..fe7bb797 100644 --- a/exasol/ds/sandbox/lib/dss_docker/create_image.py +++ b/exasol/ds/sandbox/lib/dss_docker/create_image.py @@ -19,7 +19,7 @@ from exasol.ds.sandbox.lib.ansible.ansible_access import AnsibleAccess, AnsibleFacts from exasol.ds.sandbox.lib.setup_ec2.run_install_dependencies import run_install_dependencies from exasol.ds.sandbox.lib.setup_ec2.host_info import HostInfo - +from exasol.ds.sandbox.lib.dss_docker.push_image import DockerRegistry DSS_VERSION = version("exasol-data-science-sandbox") _logger = get_status_logger(LogType.DOCKER_IMAGE) @@ -63,15 +63,19 @@ def __init__( self, repository: str, version: str = None, - publish: bool = False, keep_container: bool = False, ): version = version if version else DSS_VERSION self.container_name = f"ds-sandbox-{DssDockerImage.timestamp()}" - self.image_name = f"{repository}:{version}" - self.publish = publish + self.repository = repository + self.version = version self.keep_container = keep_container self._start = None + self.registry = None + + @property + def image_name(self): + return f"{self.repository}:{self.version}" def _ansible_run_context(self) -> AnsibleRunContext: extra_vars = { @@ -150,11 +154,16 @@ def _cleanup(self, container: DockerContainer): _logger.info("Removing container") container.remove() + def _push(self): + if self.registry is not None: + self.registry.push(self.version) + def create(self): try: container = self._start_container() facts = self._install_dependencies() image = self._commit_container(container, facts) + self._push() except Exception as ex: raise ex finally: diff --git a/exasol/ds/sandbox/lib/dss_docker/push_image.py b/exasol/ds/sandbox/lib/dss_docker/push_image.py new file mode 100644 index 00000000..0f776412 --- /dev/null +++ b/exasol/ds/sandbox/lib/dss_docker/push_image.py @@ -0,0 +1,79 @@ +import docker +import json +import logging +import requests + +from docker.client import DockerClient + +from typing import Callable, Dict, Optional +from exasol.ds.sandbox.lib.logging import get_status_logger, LogType + + +_logger = get_status_logger(LogType.DOCKER_IMAGE) + + +def get_from_dict(d: Dict[str, any], *keys: str) -> str: + for key in keys: + if not key in d: + return None + d = d[key] + return d + + +class ProgressReporter: + def __init__(self, verbose: bool): + self.last_status = None + self.verbose = verbose + self.need_linefeed = False + + def _report(self, printer: Callable, msg: Optional[str], **kwargs): + if msg is not None: + printer(msg, **kwargs) + + def _linefeed(self): + if self.need_linefeed: + self.need_linefeed = False + print() + + def report(self, status: Optional[str], progress: Optional[str]): + if not self.verbose: + return + if status == self.last_status: + self._report(print, progress, end="\r") + self.need_linefeed = progress + else: + self.last_status = status + self._linefeed() + self._report(_logger.info, status) + + +class DockerRegistry: + def __init__(self, repository: str, username: str, password: str): + self.repository = repository + self.username = username + self.password = password + self._client = None + + def client(self): + if self._client is None: + self._client = docker.from_env() + return self._client + + def push(self, tag: str): + auth_config = { + "username": self.username, + "password": self.password, + } + resp = self.client().images.push( + repository=self.repository, + tag=tag, + auth_config=auth_config, + stream=True, + decode=True, + ) + reporter = ProgressReporter(_logger.isEnabledFor(logging.INFO)) + for el in resp: + reporter.report( + el.get("status", None), + el.get("progress", None), + ) diff --git a/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml b/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml index 008c3c04..7c70227e 100644 --- a/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml +++ b/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml @@ -21,5 +21,5 @@ need_sudo: false docker_integration_test: true tasks: - - import_tasks: general_setup_tasks.yml - - import_tasks: cleanup_tasks.yml +# - import_tasks: general_setup_tasks.yml +# - import_tasks: cleanup_tasks.yml diff --git a/test/integration/conftest.py b/test/integration/conftest.py index 628acf87..6726e9af 100644 --- a/test/integration/conftest.py +++ b/test/integration/conftest.py @@ -1,6 +1,41 @@ +import docker +import pytest + +from exasol.ds.sandbox.lib.dss_docker import DssDockerImage + def pytest_addoption(parser): parser.addoption( "--dss-docker-image", default=None, help="Name and version of existing Docker image to use for tests", ) + parser.addoption( + "--docker-registry", default=None, metavar="HOST:PORT", + help="Docker registry for pushing Docker images to", + ) + + +@pytest.fixture(scope="session") +def dss_docker_image(request): + """ + If dss_docker_image_name is provided then don't create an image but + reuse the existing image as specified by cli option + --ds-docker-image-name. + """ + existing = request.config.getoption("--dss-docker-image") + if existing and ":" in existing: + name, version = existing.split(":") + yield DssDockerImage(name, version) + return + + testee = DssDockerImage( + "my-repo/dss-test-image", + version=f"{DssDockerImage.timestamp()}", + publish=False, + keep_container=False, + ) + testee.create() + try: + yield testee + finally: + docker.from_env().images.remove(testee.image_name) diff --git a/test/integration/test_create_dss_docker_image.py b/test/integration/test_create_dss_docker_image.py index c0f16d04..ee2c60f9 100644 --- a/test/integration/test_create_dss_docker_image.py +++ b/test/integration/test_create_dss_docker_image.py @@ -18,32 +18,6 @@ from exasol.ds.sandbox.lib import pretty_print -@pytest.fixture(scope="session") -def dss_docker_image(request): - """ - If dss_docker_image_name is provided then don't create an image but - reuse the existing image as specified by cli option - --ds-docker-image-name, see file conftest.py. - """ - existing = request.config.getoption("--dss-docker-image") - if existing and ":" in existing: - name, version = existing.split(":") - yield DssDockerImage(name, version) - return - - testee = DssDockerImage( - "my-repo/dss-test-image", - version=f"{DssDockerImage.timestamp()}", - publish=False, - keep_container=False, - ) - testee.create() - try: - yield testee - finally: - docker.from_env().images.remove(testee.image_name) - - @pytest.fixture def dss_docker_container(dss_docker_image): client = docker.from_env() diff --git a/test/integration/test_push_docker_image.py b/test/integration/test_push_docker_image.py new file mode 100644 index 00000000..3c59f1b8 --- /dev/null +++ b/test/integration/test_push_docker_image.py @@ -0,0 +1,136 @@ +import contextlib +import docker +import json +import logging +import pytest +import re +import requests +import time + +from docker.client import DockerClient +from typing import Callable, Optional + +from test.ports import find_free_port +from exasol.ds.sandbox.lib.dss_docker.push_image import DockerRegistry +from exasol.ds.sandbox.lib.dss_docker import DssDockerImage + +_logger = logging.getLogger(__name__) +_logger.setLevel(logging.DEBUG) + + +def normalize_request_name(name: str): + name = re.sub(r"[\[\]._]+", "_", name) + return re.sub(r"^_+|_+$", "", name) + + +class LocalDockerRegistry(DockerRegistry): + """ + Simulate Docker registry by running a local Docker container and using + the registry inside. + + Please note for pushing images to a Docker registry with host or port + differing from the official address requires to tag images in advance. + + So host and port must be prepended to property ``repository`` of the + image. + """ + def __init__(self, host_and_port: str, repo_name: str): + super().__init__( + f'{host_and_port}/{repo_name}', + username=None, + password=None, + ) + self.host_and_port = host_and_port + self.repo_name = repo_name + + @property + def url(self): + return f'http://{self.host_and_port}' + + @property + def images(self): + url = f"{self.url}/v2/{self.repo_name}/tags/list" + result = requests.request("GET", url) + images = json.loads(result.content.decode("UTF-8")) + return images + + @property + def repositories(self): + url = f"{self.url}/v2/_catalog/" + result = requests.request("GET", url) + repos = json.loads(result.content.decode("UTF-8"))["repositories"] + return repos + + def tag_image(self, image_name: str, tag: str): + image = self.client().images.get(image_name) + _logger.info(f'tagging image to {self.repository}:{tag}') + image.tag(self.repository, tag) + + +@pytest.fixture(scope="session") +def docker_registry(request): + """ + Provide a context for creating a LocalDockerRegistry accepting + parameter ``repository``. + + You can provide cli option ``--docker-registry HOST:PORT`` to pytest in + order reuse an already running Docker container as registry. + """ + def make_context(host_and_port: str) -> Callable[[str], LocalDockerRegistry]: + @contextlib.contextmanager + def context(repository: str) -> DockerRegistry: + yield LocalDockerRegistry(host_and_port, repository) + return context + + existing = request.config.getoption("--docker-registry") + if existing is not None: + yield make_context(existing) + return + + test_name = normalize_request_name(request.node.name) + container_name = f"{test_name}_registry" + port = find_free_port() + + _logger.debug("Pulling Docker image with Docker registry") + client = docker.from_env() + client.images.pull(repository="registry", tag="2") + _logger.debug(f"Start container of {container_name}") + try: + client.containers.get(container_name).remove(force=True) + except: + pass + container = client.containers.run( + image="registry:2", + name=container_name, + ports={5000: port}, + detach=True + ) + time.sleep(10) + _logger.debug(f"Finished start container of {container_name}") + try: + yield make_context(f"localhost:{port}") + finally: + _logger.debug("Stopping container") + container.stop() + _logger.debug("Removing container") + container.remove() + + +def test_push(dss_docker_image, docker_registry): + tag = "999.9.9" + with docker_registry(dss_docker_image.repository) as registry: + registry.tag_image(dss_docker_image.image_name, tag) + registry.push(tag) + assert dss_docker_image.repository in registry.repositories + assert tag in registry.images["tags"] + + +def test_push_via_image(dss_docker_image, docker_registry): + repo = dss_docker_image.repository + tag = dss_docker_image.version + with docker_registry(repo) as registry: + dss_docker_image.registry = registry + registry.tag_image(dss_docker_image.image_name, tag) + dss_docker_image._push() + assert repo in registry.repositories + assert tag in registry.images["tags"] diff --git a/test/ports.py b/test/ports.py new file mode 100644 index 00000000..26bc33f2 --- /dev/null +++ b/test/ports.py @@ -0,0 +1,30 @@ +import socket +from contextlib import ExitStack +from typing import List + + +# copied from https://github.com/exasol/integration-test-docker-environment +def find_free_ports(num_ports: int) -> List[int]: + def new_socket(): + return socket.socket(socket.AF_INET, socket.SOCK_STREAM) + def bind(sock: socket.socket, port: int): + sock.bind(('', port)) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + def acquire_port_numbers(num_ports: int) -> List[int]: + with ExitStack() as stack: + sockets = [stack.enter_context(new_socket()) for dummy in range(num_ports)] + for sock in sockets: + bind(sock, 0) + yield sock.getsockname()[1] + def check_port_numbers(ports): + with ExitStack() as stack: + sockets_and_ports = [(stack.enter_context(new_socket()), port) for port in ports] + for sock, port in sockets_and_ports: + bind(sock, port) + ports = list(acquire_port_numbers(num_ports)) + check_port_numbers(ports) + return ports + + +def find_free_port(): + return find_free_ports(1)[0] From 55c3542ace470d2e0c7115589f3e2863a334cbe2 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 5 Dec 2023 08:18:20 +0100 Subject: [PATCH 2/9] Fixed review findings --- .../cli/commands/create_docker_image.py | 59 +++++---- .../ds/sandbox/lib/dss_docker/create_image.py | 23 ++-- .../ds/sandbox/lib/dss_docker/push_image.py | 26 +--- .../runtime/ansible/dss_docker_playbook.yml | 4 +- pyproject.toml | 5 + test/integration/local_docker_registry.py | 47 ++++++++ test/integration/test_push_docker_image.py | 113 +++++++----------- test/unit/test_dss_docker_image.py | 19 ++- 8 files changed, 167 insertions(+), 129 deletions(-) create mode 100644 test/integration/local_docker_registry.py diff --git a/exasol/ds/sandbox/cli/commands/create_docker_image.py b/exasol/ds/sandbox/cli/commands/create_docker_image.py index d63c118b..dedac8e4 100644 --- a/exasol/ds/sandbox/cli/commands/create_docker_image.py +++ b/exasol/ds/sandbox/cli/commands/create_docker_image.py @@ -1,4 +1,5 @@ import click +import os from exasol.ds.sandbox.cli.cli import cli, option_with_env_default from exasol.ds.sandbox.cli.options.logging import logging_options @@ -8,23 +9,37 @@ from exasol.ds.sandbox.lib.logging import set_log_level +USER_ENV = "DOCKER_REGISTRY_USER" +PASSWORD_ENV = "DOCKER_REGISTRY_PASSWORD" + + @cli.command() @add_options([ click.option( - '--repository', type=str, metavar="ORG/REPO", show_default=True, + "--repository", type=str, metavar="[HOST[:PORT]/]ORG/REPO", + show_default=True, default="exasol/data-science-sandbox", - help="Organization and repository on hub.docker.com to publish the docker image to"), + help=""" + Organization and repository on hub.docker.com to publish the + docker image to see https://docs.docker.com/engine/reference/commandline/tag. + """), click.option( - '--version', type=str, metavar="VERSION", + "--version", type=str, metavar="VERSION", help="Docker image version tag"), - option_with_env_default("DOCKER_REGISTRY_USER", - '--registry-user', type=str, metavar="USER", - help="Username for publication to Docker registry"), - option_with_env_default("DOCKER_REGISTRY_PASSWORD", - '--registry-password', type=str, metavar="PASSWORD", - help="Password for publication to Docker registry"), click.option( - '--keep-container', type=bool, is_flag=True, + "--publish", type=bool, is_flag=True, + help="Whether to publish the created Docker image"), + click.option( + "--registry-user", type=str, metavar="USER", + default=lambda: os.environ.get(USER_ENV, None), + help=f""" + Username for Docker registry [defaults to environment + variable {USER_ENV}]. Password is always read + from environment variable {PASSWORD_ENV}. + """ + ), + click.option( + "--keep-container", type=bool, is_flag=True, help="""Keep the Docker Container running after creating the image. Otherwise stop and remove the container."""), ]) @@ -32,8 +47,8 @@ def create_docker_image( repository: str, version: str, + publish: bool, registry_user: str, - registry_password: str, keep_container: bool, log_level: str, ): @@ -41,17 +56,13 @@ def create_docker_image( Create a Docker image for data-science-sandbox. If username and password for the Docker registry are specified then deploy the image to the registry. """ + def registry_password(): + if registry_user is None: + return None + return os.environ.get(PASSWORD_ENV, None) + set_log_level(log_level) - creator = DssDockerImage( - repository=repository, - version=version, - keep_container=keep_container, - ) - if registry_user and registry_password: - creator.registry = DockerRegistry( - creator.repository, - registry_user, - registry_password, - ) - print(f'user {creator.registry.username} password {registry_password}') - # creator.create() + creator = DssDockerImage(repository, version, keep_container) + if publish: + creator.registry = DockerRegistry(registry_user, registry_password()) + creator.create() diff --git a/exasol/ds/sandbox/lib/dss_docker/create_image.py b/exasol/ds/sandbox/lib/dss_docker/create_image.py index fe7bb797..d57f7289 100644 --- a/exasol/ds/sandbox/lib/dss_docker/create_image.py +++ b/exasol/ds/sandbox/lib/dss_docker/create_image.py @@ -2,12 +2,13 @@ import humanfriendly import importlib_resources +from functools import reduce from datetime import datetime from docker.types import Mount from exasol.ds.sandbox.lib import pretty_print from importlib_metadata import version from pathlib import Path -from typing import List +from typing import Dict, List, Optional from docker.models.containers import Container as DockerContainer from docker.models.images import Image as DockerImage @@ -25,14 +26,16 @@ _logger = get_status_logger(LogType.DOCKER_IMAGE) -def get_fact(facts: AnsibleFacts, *keys: str) -> str: - keys = list(keys) - keys.insert(0, "dss_facts") - for key in keys: - if not key in facts: - return None - facts = facts[key] - return facts +def get_fact(facts: AnsibleFacts, *keys: str) -> Optional[str]: + return get_nested_value(facts, "dss_facts", *keys) + + +def get_nested_value(mapping: Dict[str, any], *keys: str) -> Optional[str]: + def nested_item(current, key): + valid = current is not None and key in current + return current[key] if valid else None + + return reduce(nested_item, keys, mapping) def entrypoint(facts: AnsibleFacts) -> List[str]: @@ -156,7 +159,7 @@ def _cleanup(self, container: DockerContainer): def _push(self): if self.registry is not None: - self.registry.push(self.version) + self.registry.push(self.repository, self.version) def create(self): try: diff --git a/exasol/ds/sandbox/lib/dss_docker/push_image.py b/exasol/ds/sandbox/lib/dss_docker/push_image.py index 0f776412..5061ed4c 100644 --- a/exasol/ds/sandbox/lib/dss_docker/push_image.py +++ b/exasol/ds/sandbox/lib/dss_docker/push_image.py @@ -5,21 +5,13 @@ from docker.client import DockerClient -from typing import Callable, Dict, Optional +from typing import Callable, Optional from exasol.ds.sandbox.lib.logging import get_status_logger, LogType _logger = get_status_logger(LogType.DOCKER_IMAGE) -def get_from_dict(d: Dict[str, any], *keys: str) -> str: - for key in keys: - if not key in d: - return None - d = d[key] - return d - - class ProgressReporter: def __init__(self, verbose: bool): self.last_status = None @@ -48,24 +40,18 @@ def report(self, status: Optional[str], progress: Optional[str]): class DockerRegistry: - def __init__(self, repository: str, username: str, password: str): - self.repository = repository + def __init__(self, username: str, password: str): self.username = username self.password = password - self._client = None - - def client(self): - if self._client is None: - self._client = docker.from_env() - return self._client - def push(self, tag: str): + def push(self, repository: str, tag: str): auth_config = { "username": self.username, "password": self.password, } - resp = self.client().images.push( - repository=self.repository, + client = docker.from_env() + resp = client.images.push( + repository=repository, tag=tag, auth_config=auth_config, stream=True, diff --git a/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml b/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml index 7c70227e..008c3c04 100644 --- a/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml +++ b/exasol/ds/sandbox/runtime/ansible/dss_docker_playbook.yml @@ -21,5 +21,5 @@ need_sudo: false docker_integration_test: true tasks: -# - import_tasks: general_setup_tasks.yml -# - import_tasks: cleanup_tasks.yml + - import_tasks: general_setup_tasks.yml + - import_tasks: cleanup_tasks.yml diff --git a/pyproject.toml b/pyproject.toml index 5dc17a2c..058608ff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,3 +61,8 @@ testpaths = [ "test" ] +# ignore warnings caused by docker lib +filterwarnings = [ + "ignore:the imp module is deprecated:DeprecationWarning", + "ignore:distutils Version classes are deprecated:DeprecationWarning", +] diff --git a/test/integration/local_docker_registry.py b/test/integration/local_docker_registry.py new file mode 100644 index 00000000..285385be --- /dev/null +++ b/test/integration/local_docker_registry.py @@ -0,0 +1,47 @@ +import docker +import contextlib +import json +import logging +import requests +import time + +from typing import Dict, List +from test.ports import find_free_port +from exasol.ds.sandbox.lib.dss_docker import DssDockerImage, DockerRegistry + + +_logger = logging.getLogger(__name__) +_logger.setLevel(logging.DEBUG) + + +class LocalDockerRegistry(DockerRegistry): + """ + Simulate Docker registry by running a local Docker container and using + the registry inside. + + Please note for pushing images to a Docker registry with host or port + differing from the official address requires to tag images in advance. + + So host and port must be prepended to property ``repository`` of the + image. + """ + def __init__(self, host_and_port: str): + super().__init__(username=None, password=None) + self.host_and_port = host_and_port + + @property + def url(self): + return f'http://{self.host_and_port}' + + def images(self, repo_name: str) -> Dict[str, any]: + url = f"{self.url}/v2/{repo_name}/tags/list" + result = requests.request("GET", url) + images = json.loads(result.content.decode("UTF-8")) + return images + + @property + def repositories(self) -> List[str]: + url = f"{self.url}/v2/_catalog/" + result = requests.request("GET", url) + repos = json.loads(result.content.decode("UTF-8"))["repositories"] + return repos diff --git a/test/integration/test_push_docker_image.py b/test/integration/test_push_docker_image.py index 3c59f1b8..6154b6b3 100644 --- a/test/integration/test_push_docker_image.py +++ b/test/integration/test_push_docker_image.py @@ -7,13 +7,14 @@ import requests import time -from docker.client import DockerClient -from typing import Callable, Optional - -from test.ports import find_free_port -from exasol.ds.sandbox.lib.dss_docker.push_image import DockerRegistry from exasol.ds.sandbox.lib.dss_docker import DssDockerImage +from test.integration.local_docker_registry import ( + custom_docker_registry_context, + LocalDockerRegistry, + local_docker_registry_context, +) + _logger = logging.getLogger(__name__) _logger.setLevel(logging.DEBUG) @@ -23,48 +24,28 @@ def normalize_request_name(name: str): return re.sub(r"^_+|_+$", "", name) -class LocalDockerRegistry(DockerRegistry): +@contextlib.contextmanager +def tagged_image( + dss_docker_image: DssDockerImage, + registry: LocalDockerRegistry, + tag: str = None, +): """ - Simulate Docker registry by running a local Docker container and using - the registry inside. - - Please note for pushing images to a Docker registry with host or port - differing from the official address requires to tag images in advance. - - So host and port must be prepended to property ``repository`` of the - image. + Prepend host and port of LocalDockerRegistry to the repository name of the + DssDockerImage to enable pushing the image to the local registry. """ - def __init__(self, host_and_port: str, repo_name: str): - super().__init__( - f'{host_and_port}/{repo_name}', - username=None, - password=None, - ) - self.host_and_port = host_and_port - self.repo_name = repo_name - - @property - def url(self): - return f'http://{self.host_and_port}' - - @property - def images(self): - url = f"{self.url}/v2/{self.repo_name}/tags/list" - result = requests.request("GET", url) - images = json.loads(result.content.decode("UTF-8")) - return images - - @property - def repositories(self): - url = f"{self.url}/v2/_catalog/" - result = requests.request("GET", url) - repos = json.loads(result.content.decode("UTF-8"))["repositories"] - return repos - - def tag_image(self, image_name: str, tag: str): - image = self.client().images.get(image_name) - _logger.info(f'tagging image to {self.repository}:{tag}') - image.tag(self.repository, tag) + new_repo = f"{registry.host_and_port}/{dss_docker_image.repository}" + client = docker.from_env() + tag = tag or dss_docker_image.version + _logger.info(f'tagging image to {new_repo}:{tag}') + image = client.images.get(dss_docker_image.image_name) + image.tag(new_repo, tag) + tagged = DssDockerImage(new_repo, tag) + tagged.registry = registry + try: + yield tagged + finally: + docker.from_env().images.remove(tagged.image_name) @pytest.fixture(scope="session") @@ -76,25 +57,19 @@ def docker_registry(request): You can provide cli option ``--docker-registry HOST:PORT`` to pytest in order reuse an already running Docker container as registry. """ - def make_context(host_and_port: str) -> Callable[[str], LocalDockerRegistry]: - @contextlib.contextmanager - def context(repository: str) -> DockerRegistry: - yield LocalDockerRegistry(host_and_port, repository) - return context - existing = request.config.getoption("--docker-registry") if existing is not None: - yield make_context(existing) + yield LocalDockerRegistry(existing) return test_name = normalize_request_name(request.node.name) container_name = f"{test_name}_registry" - port = find_free_port() - _logger.debug("Pulling Docker image with Docker registry") + port = find_free_port() client = docker.from_env() + _logger.debug("Pulling Docker image with Docker registry") client.images.pull(repository="registry", tag="2") - _logger.debug(f"Start container of {container_name}") + _logger.debug(f"Starting container {container_name}") try: client.containers.get(container_name).remove(force=True) except: @@ -103,12 +78,12 @@ def context(repository: str) -> DockerRegistry: image="registry:2", name=container_name, ports={5000: port}, - detach=True + detach=True, ) time.sleep(10) - _logger.debug(f"Finished start container of {container_name}") + _logger.debug(f"Finished starting container {container_name}") try: - yield make_context(f"localhost:{port}") + yield LocalDockerRegistry(f"localhost:{port}") finally: _logger.debug("Stopping container") container.stop() @@ -116,21 +91,19 @@ def context(repository: str) -> DockerRegistry: container.remove() -def test_push(dss_docker_image, docker_registry): +def test_push_tag(dss_docker_image, docker_registry): + repo = dss_docker_image.repository tag = "999.9.9" - with docker_registry(dss_docker_image.repository) as registry: - registry.tag_image(dss_docker_image.image_name, tag) - registry.push(tag) - assert dss_docker_image.repository in registry.repositories - assert tag in registry.images["tags"] + with tagged_image(dss_docker_image, docker_registry, tag) as tagged: + docker_registry.push(tagged.repository, tag) + assert repo in docker_registry.repositories + assert tag in docker_registry.images(repo)["tags"] def test_push_via_image(dss_docker_image, docker_registry): repo = dss_docker_image.repository tag = dss_docker_image.version - with docker_registry(repo) as registry: - dss_docker_image.registry = registry - registry.tag_image(dss_docker_image.image_name, tag) - dss_docker_image._push() - assert repo in registry.repositories - assert tag in registry.images["tags"] + with tagged_image(dss_docker_image, docker_registry) as tagged: + tagged._push() + assert repo in docker_registry.repositories + assert tag in docker_registry.images(repo)["tags"] diff --git a/test/unit/test_dss_docker_image.py b/test/unit/test_dss_docker_image.py index 8a346670..51c7a6ba 100644 --- a/test/unit/test_dss_docker_image.py +++ b/test/unit/test_dss_docker_image.py @@ -15,7 +15,6 @@ def sample_repo(): def test_constructor_defaults(sample_repo): testee = DssDockerImage(sample_repo) assert testee.image_name == f"{sample_repo}:{DSS_VERSION}" - assert testee.publish == False assert testee.keep_container == False @@ -24,14 +23,28 @@ def test_constructor(sample_repo): testee = DssDockerImage( repository=sample_repo, version=version, - publish=True, keep_container=True, ) assert testee.image_name == f"{sample_repo}:{version}" - assert testee.publish == True assert testee.keep_container == True +@pytest.mark.parametrize( + "testee", + [ + {}, + {"key": "sample value"}, + ]) +def test_nested_value_missing_entry(testee): + assert create_image.get_nested_value(testee, "missing_entry") == None + assert create_image.get_nested_value(testee, "key", "key-2") == None + + +def test_nested_value_level_2(): + testee = {"key": {"key-2": "value"}} + assert create_image.get_nested_value(testee, "key", "key-2") == "value" + + @pytest.mark.parametrize( "facts", [ From c7f526db7c92cc444f49b662b7135067f0d717b7 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 5 Dec 2023 08:20:47 +0100 Subject: [PATCH 3/9] Fixed test --- test/integration/conftest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/conftest.py b/test/integration/conftest.py index 6726e9af..6fd05472 100644 --- a/test/integration/conftest.py +++ b/test/integration/conftest.py @@ -31,7 +31,6 @@ def dss_docker_image(request): testee = DssDockerImage( "my-repo/dss-test-image", version=f"{DssDockerImage.timestamp()}", - publish=False, keep_container=False, ) testee.create() From 5dcc5cadadcdb6523c388528c6737d667ac32b89 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 5 Dec 2023 08:43:55 +0100 Subject: [PATCH 4/9] Updated cli descriptions and docstring --- exasol/ds/sandbox/cli/commands/create_docker_image.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/exasol/ds/sandbox/cli/commands/create_docker_image.py b/exasol/ds/sandbox/cli/commands/create_docker_image.py index dedac8e4..b4d86f3b 100644 --- a/exasol/ds/sandbox/cli/commands/create_docker_image.py +++ b/exasol/ds/sandbox/cli/commands/create_docker_image.py @@ -21,7 +21,8 @@ default="exasol/data-science-sandbox", help=""" Organization and repository on hub.docker.com to publish the - docker image to see https://docs.docker.com/engine/reference/commandline/tag. + docker image to. Optionally prefixed by a host and a port, + see https://docs.docker.com/engine/reference/commandline/tag. """), click.option( "--version", type=str, metavar="VERSION", @@ -34,7 +35,7 @@ default=lambda: os.environ.get(USER_ENV, None), help=f""" Username for Docker registry [defaults to environment - variable {USER_ENV}]. Password is always read + variable {USER_ENV}]. If specified then password is read from environment variable {PASSWORD_ENV}. """ ), @@ -53,8 +54,10 @@ def create_docker_image( log_level: str, ): """ - Create a Docker image for data-science-sandbox. If username and password - for the Docker registry are specified then deploy the image to the registry. + Create a Docker image for data-science-sandbox. If option + ``--publish`` is specified then deploy the image to the Docker registry + using the specified user name and reading the password from environment + variable ``PASSWORD_ENV``. """ def registry_password(): if registry_user is None: From 664c39590cbd38f60ea66ba9051e6445fef8b9c8 Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 5 Dec 2023 08:45:38 +0100 Subject: [PATCH 5/9] Remove unused import --- exasol/ds/sandbox/lib/dss_docker/create_image.py | 1 - 1 file changed, 1 deletion(-) diff --git a/exasol/ds/sandbox/lib/dss_docker/create_image.py b/exasol/ds/sandbox/lib/dss_docker/create_image.py index d57f7289..eb0f5b11 100644 --- a/exasol/ds/sandbox/lib/dss_docker/create_image.py +++ b/exasol/ds/sandbox/lib/dss_docker/create_image.py @@ -20,7 +20,6 @@ from exasol.ds.sandbox.lib.ansible.ansible_access import AnsibleAccess, AnsibleFacts from exasol.ds.sandbox.lib.setup_ec2.run_install_dependencies import run_install_dependencies from exasol.ds.sandbox.lib.setup_ec2.host_info import HostInfo -from exasol.ds.sandbox.lib.dss_docker.push_image import DockerRegistry DSS_VERSION = version("exasol-data-science-sandbox") _logger = get_status_logger(LogType.DOCKER_IMAGE) From 5346f8d2f035aec95c54c0bf95d02be76d464dbe Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 5 Dec 2023 10:53:33 +0100 Subject: [PATCH 6/9] Fixed import in dss_docker/__init__.py --- exasol/ds/sandbox/lib/dss_docker/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exasol/ds/sandbox/lib/dss_docker/__init__.py b/exasol/ds/sandbox/lib/dss_docker/__init__.py index a9d32e27..30bd24a4 100644 --- a/exasol/ds/sandbox/lib/dss_docker/__init__.py +++ b/exasol/ds/sandbox/lib/dss_docker/__init__.py @@ -1 +1,2 @@ -from .create_image import DssDockerImage, DockerRegistry +from .create_image import DssDockerImage +from .push_image import DockerRegistry From ab196c3850143addef677e7f87b8446800f7767c Mon Sep 17 00:00:00 2001 From: ckunki Date: Tue, 5 Dec 2023 17:40:49 +0100 Subject: [PATCH 7/9] test_push_docker_image now uses docker image registry:2 Added test to verify DssDockerImage calls registry.push() --- test/integration/test_push_docker_image.py | 77 ++++++++++++---------- test/unit/test_dss_docker_image.py | 28 +++++++- 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/test/integration/test_push_docker_image.py b/test/integration/test_push_docker_image.py index 6154b6b3..8ac56120 100644 --- a/test/integration/test_push_docker_image.py +++ b/test/integration/test_push_docker_image.py @@ -8,11 +8,11 @@ import time from exasol.ds.sandbox.lib.dss_docker import DssDockerImage +from test.ports import find_free_port +from dataclasses import dataclass from test.integration.local_docker_registry import ( - custom_docker_registry_context, LocalDockerRegistry, - local_docker_registry_context, ) _logger = logging.getLogger(__name__) @@ -24,32 +24,50 @@ def normalize_request_name(name: str): return re.sub(r"^_+|_+$", "", name) +@dataclass +class DockerImageSpec: + repository: str + tag: str + + @property + def name(self) -> str: + return f"{self.repository}:{self.tag}" + + +@pytest.fixture(scope="session") +def registry_image(): + spec = DockerImageSpec("registry", "2") + client = docker.from_env() + if not client.images.list(spec.name): + _logger.debug("Pulling Docker image with Docker registry") + client.images.pull(spec.repository, spec.tag) + return spec + + +@pytest.fixture(scope="session") +def sample_docker_image(registry_image): + return registry_image + + @contextlib.contextmanager -def tagged_image( - dss_docker_image: DssDockerImage, - registry: LocalDockerRegistry, - tag: str = None, -): +def tagged_image(old: DockerImageSpec, new: DockerImageSpec): """ Prepend host and port of LocalDockerRegistry to the repository name of the DssDockerImage to enable pushing the image to the local registry. """ - new_repo = f"{registry.host_and_port}/{dss_docker_image.repository}" client = docker.from_env() - tag = tag or dss_docker_image.version - _logger.info(f'tagging image to {new_repo}:{tag}') - image = client.images.get(dss_docker_image.image_name) - image.tag(new_repo, tag) - tagged = DssDockerImage(new_repo, tag) - tagged.registry = registry + _logger.debug(f'tagging image from {old.name} to {new.name}') + image = client.images.get(old.name) + image.tag(new.repository, new.tag) + tagged = DssDockerImage(new.repository, new.tag) try: yield tagged finally: - docker.from_env().images.remove(tagged.image_name) + docker.from_env().images.remove(new.name) @pytest.fixture(scope="session") -def docker_registry(request): +def docker_registry(request, registry_image): """ Provide a context for creating a LocalDockerRegistry accepting parameter ``repository``. @@ -67,8 +85,6 @@ def docker_registry(request): port = find_free_port() client = docker.from_env() - _logger.debug("Pulling Docker image with Docker registry") - client.images.pull(repository="registry", tag="2") _logger.debug(f"Starting container {container_name}") try: client.containers.get(container_name).remove(force=True) @@ -91,19 +107,14 @@ def docker_registry(request): container.remove() -def test_push_tag(dss_docker_image, docker_registry): - repo = dss_docker_image.repository - tag = "999.9.9" - with tagged_image(dss_docker_image, docker_registry, tag) as tagged: - docker_registry.push(tagged.repository, tag) - assert repo in docker_registry.repositories - assert tag in docker_registry.images(repo)["tags"] - - -def test_push_via_image(dss_docker_image, docker_registry): - repo = dss_docker_image.repository - tag = dss_docker_image.version - with tagged_image(dss_docker_image, docker_registry) as tagged: - tagged._push() +def test_push_tag(sample_docker_image, docker_registry): + old = DockerImageSpec("registry", "2") + repo = "org/sample_repo" + new = DockerImageSpec( + docker_registry.host_and_port + "/" + repo, + "999.9.9", + ) + with tagged_image(old, new) as tagged: + docker_registry.push(tagged.repository, new.tag) assert repo in docker_registry.repositories - assert tag in docker_registry.images(repo)["tags"] + assert new.tag in docker_registry.images(repo)["tags"] diff --git a/test/unit/test_dss_docker_image.py b/test/unit/test_dss_docker_image.py index 51c7a6ba..985e89dc 100644 --- a/test/unit/test_dss_docker_image.py +++ b/test/unit/test_dss_docker_image.py @@ -1,6 +1,8 @@ import pytest -from exasol.ds.sandbox.lib.dss_docker import create_image +from unittest.mock import MagicMock, Mock +from datetime import datetime +from exasol.ds.sandbox.lib.dss_docker import create_image, DockerRegistry from exasol.ds.sandbox.lib.dss_docker.create_image import ( DssDockerImage, DSS_VERSION, @@ -96,3 +98,27 @@ def test_entrypoint_with_copy_args(): "--notebooks", final, "--jupyter-server", jupyter, ] + +@pytest.fixture +def mocked_docker_image(): + testee = DssDockerImage("org/sample_repo", "version") + testee._start = datetime.now() + testee._start_container = MagicMock() + testee._install_dependencies = MagicMock() + testee._cleanup = MagicMock() + image = Mock(attrs={"Size": 1025}) + testee._commit_container = MagicMock(return_value=image) + return testee + + +def test_push_called(mocker, mocked_docker_image): + testee = mocked_docker_image + + registry = DockerRegistry("user", "password") + registry.push = MagicMock() + testee.registry = registry + + testee.create() + assert registry.push.called + expected = mocker.call(testee.repository, testee.version) + assert registry.push.call_args == expected From 8c0d9580ca58be90acb95f03e05b5540825c1ac2 Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 6 Dec 2023 14:57:02 +0100 Subject: [PATCH 8/9] Fixed review findings. --- test/integration/test_push_docker_image.py | 22 ++++++++++------------ test/unit/test_dss_docker_image.py | 12 ++++-------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/test/integration/test_push_docker_image.py b/test/integration/test_push_docker_image.py index 8ac56120..bd1db5ab 100644 --- a/test/integration/test_push_docker_image.py +++ b/test/integration/test_push_docker_image.py @@ -52,8 +52,8 @@ def sample_docker_image(registry_image): @contextlib.contextmanager def tagged_image(old: DockerImageSpec, new: DockerImageSpec): """ - Prepend host and port of LocalDockerRegistry to the repository name of the - DssDockerImage to enable pushing the image to the local registry. + Tag the Docker image with spec ``old`` to ``new`` to enable pushing + the image to a local registry. """ client = docker.from_env() _logger.debug(f'tagging image from {old.name} to {new.name}') @@ -69,11 +69,10 @@ def tagged_image(old: DockerImageSpec, new: DockerImageSpec): @pytest.fixture(scope="session") def docker_registry(request, registry_image): """ - Provide a context for creating a LocalDockerRegistry accepting - parameter ``repository``. - - You can provide cli option ``--docker-registry HOST:PORT`` to pytest in - order reuse an already running Docker container as registry. + Create a LocalDockerRegistry for executing integration tests. You can + provide cli option ``--docker-registry HOST:PORT`` to pytest in order + reuse an already running Docker container as registry, see file + ``conftest.py``. """ existing = request.config.getoption("--docker-registry") if existing is not None: @@ -108,13 +107,12 @@ def docker_registry(request, registry_image): def test_push_tag(sample_docker_image, docker_registry): - old = DockerImageSpec("registry", "2") repo = "org/sample_repo" - new = DockerImageSpec( + spec = DockerImageSpec( docker_registry.host_and_port + "/" + repo, "999.9.9", ) - with tagged_image(old, new) as tagged: - docker_registry.push(tagged.repository, new.tag) + with tagged_image(sample_docker_image, spec) as tagged: + docker_registry.push(tagged.repository, spec.tag) assert repo in docker_registry.repositories - assert new.tag in docker_registry.images(repo)["tags"] + assert spec.tag in docker_registry.images(repo)["tags"] diff --git a/test/unit/test_dss_docker_image.py b/test/unit/test_dss_docker_image.py index 985e89dc..9bc5e05b 100644 --- a/test/unit/test_dss_docker_image.py +++ b/test/unit/test_dss_docker_image.py @@ -1,6 +1,6 @@ import pytest -from unittest.mock import MagicMock, Mock +from unittest.mock import MagicMock, Mock, create_autospec from datetime import datetime from exasol.ds.sandbox.lib.dss_docker import create_image, DockerRegistry from exasol.ds.sandbox.lib.dss_docker.create_image import ( @@ -113,12 +113,8 @@ def mocked_docker_image(): def test_push_called(mocker, mocked_docker_image): testee = mocked_docker_image - - registry = DockerRegistry("user", "password") - registry.push = MagicMock() - testee.registry = registry - + testee.registry = create_autospec(DockerRegistry) testee.create() - assert registry.push.called + assert testee.registry.push.called expected = mocker.call(testee.repository, testee.version) - assert registry.push.call_args == expected + assert testee.registry.push.call_args == expected From 6ee1305ea1e46ee053b763f68950d98bc916440e Mon Sep 17 00:00:00 2001 From: ckunki Date: Wed, 6 Dec 2023 15:07:55 +0100 Subject: [PATCH 9/9] Updated curl from 7.68.0-1ubuntu2.20 --- .../ds/sandbox/runtime/ansible/roles/poetry/defaults/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exasol/ds/sandbox/runtime/ansible/roles/poetry/defaults/main.yml b/exasol/ds/sandbox/runtime/ansible/roles/poetry/defaults/main.yml index 55cb510d..e60a2225 100644 --- a/exasol/ds/sandbox/runtime/ansible/roles/poetry/defaults/main.yml +++ b/exasol/ds/sandbox/runtime/ansible/roles/poetry/defaults/main.yml @@ -1,6 +1,6 @@ --- apt_dependencies: - - curl=7.68.0-1ubuntu2.20 + - curl=7.68.0-1ubuntu2.21 - python3.8-venv=3.8.10-0ubuntu1~20.04.9 - python3-pip=20.0.2-5ubuntu1.10