From be8c96d050b769c0b31b387c865b130a8e8b8983 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 15:54:03 +0300 Subject: [PATCH 01/28] add image parsing logic --- python/neuromation/cli/image.py | 52 +++++----- python/neuromation/cli/job.py | 11 ++- python/neuromation/client/__init__.py | 2 + python/neuromation/client/images.py | 91 +++++++++--------- python/neuromation/client/parsing_utils.py | 105 +++++++++++++++++++++ 5 files changed, 186 insertions(+), 75 deletions(-) create mode 100644 python/neuromation/client/parsing_utils.py diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 1099c75ae..dae4e082c 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -2,10 +2,8 @@ import sys import click -from yarl import URL -# TODO(asvetlov): rename the class to avoid the namig conflict -from neuromation.client.images import Image +from neuromation.client import ImageParser from .command_spinner import SpinnerBase from .rc import Config @@ -43,22 +41,25 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: """ - username = cfg.username + parser = ImageParser(cfg.username, cfg.registry_url) + local_img = parser.parse_local(image_name) + remote_img = ( + parser.parse_remote(remote_image_name) + if remote_image_name + else parser.parse_remote(image_name, require_scheme=False) + ) - local_image = remote_image = Image.from_local(image_name, username) - if remote_image_name: - remote_image = Image.from_url(URL(remote_image_name), username) - - log.info(f"Using remote image '{remote_image.url}'") - log.info(f"Using local image '{local_image.url}'") + # TODO: check out all todos by ajuszkowski in this PR! + log.info(f"Using local image '{local_img.as_local()}'") + log.info(f"Using remote image '{remote_img.as_url()}'") + log.debug(f"LOCAL: '{local_img}'") + log.debug(f"REMOTE: '{remote_img}'") spinner = SpinnerBase.create_spinner(sys.stdout.isatty(), "Pushing image {} ") async with cfg.make_client() as client: - result_remote_image = await client.images.push( - local_image, remote_image, spinner - ) - click.echo(result_remote_image.url) + result_remote_image = await client.images.push(local_img, remote_img, spinner) + click.echo(result_remote_image.as_url()) @command() @@ -81,21 +82,24 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: """ - username = cfg.username + parser = ImageParser(cfg.username, cfg.registry_url) + remote_img = parser.parse_remote(image_name) + local_img = ( + parser.parse_local(local_image_name) + if local_image_name + else parser.parse_local(remote_img.as_local()) + ) - remote_image = local_image = Image.from_url(URL(image_name), username) - if local_image_name: - local_image = Image.from_local(local_image_name, username) - log.info(f"Using remote image '{remote_image.url}'") - log.info(f"Using local image '{local_image.url}'") + log.info(f"Using remote image '{remote_img.as_url()}'") + log.info(f"Using local image '{local_img.as_local()}'") + log.debug(f"REMOTE: '{remote_img}'") + log.debug(f"LOCAL: '{local_img}'") spinner = SpinnerBase.create_spinner(sys.stdout.isatty(), "Pulling image {} ") async with cfg.make_client() as client: - result_local_image = await client.images.pull( - remote_image, local_image, spinner - ) - click.echo(result_local_image.local) + result_local_image = await client.images.pull(remote_img, local_img, spinner) + click.echo(result_local_image.as_local()) @command() diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index 7543a0913..1bc8dc7e9 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -10,6 +10,7 @@ from neuromation.client import ( Image, + ImageParser, JobStatus, NetworkPortForwarding, Resources, @@ -193,11 +194,15 @@ async def submit( log.debug(f'cmd="{cmd}"') memory = to_megabytes_str(memory) - image_obj = Image(image=image, command=cmd) + + image_parser = ImageParser(cfg.username, cfg.registry_url) + parsed_image = image_parser.parse_remote(image, require_scheme=False) # TODO (ajuszkowski 01-Feb-19) process --quiet globally to set up logger+click if not quiet: - # TODO (ajuszkowski 01-Feb-19) normalize image name to URI (issue 452) - log.info(f"Using image '{image_obj.image}'") + log.info(f"Using image '{parsed_image.as_url()}'") + log.debug(f"IMAGE: {parsed_image}") + image_obj = Image(image=parsed_image.as_repo(), command=cmd) + network = NetworkPortForwarding.from_cli(http, ssh) resources = Resources.create(cpu, gpu, gpu_model, memory, extshm) volumes = Volume.from_cli_list(username, volume) diff --git a/python/neuromation/client/__init__.py b/python/neuromation/client/__init__.py index 4093fa10c..68f35c942 100644 --- a/python/neuromation/client/__init__.py +++ b/python/neuromation/client/__init__.py @@ -32,9 +32,11 @@ from .users import Action, Permission, Users from .images import Images from .config import Config +from .parsing_utils import ImageParser __all__ = ( "Image", + "ImageParser", "JobDescription", "JobStatus", "JobStatusHistory", diff --git a/python/neuromation/client/images.py b/python/neuromation/client/images.py index 68926120b..ae44563ca 100644 --- a/python/neuromation/client/images.py +++ b/python/neuromation/client/images.py @@ -1,7 +1,8 @@ +import logging import re from contextlib import suppress from dataclasses import dataclass -from typing import Dict, List +from typing import Dict, List, Optional import aiodocker import aiohttp @@ -17,44 +18,32 @@ STATUS_FORBIDDEN = 403 STATUS_NOT_FOUND = 404 STATUS_CUSTOM_ERROR = 900 -DEFAULT_TAG = "latest" +log = logging.getLogger(__name__) -@dataclass(frozen=True) -class Image: - url: URL - local: str - - @classmethod - def from_url(cls, url: URL, username: str) -> "Image": - if not url: - raise ValueError(f"Image URL cannot be empty") - if url.scheme != "image": - raise ValueError(f"Invalid scheme, for image URL: {url}") - if url.path == "/" or url.query or url.fragment or url.user or url.port: - raise ValueError(f"Invalid image URL: {url}") - colon_count = url.path.count(":") - if colon_count > 1: - raise ValueError(f"Invalid image URL, only one colon allowed: {url}") +IMAGE_SCHEME = "image" - if not colon_count: - url = url.with_path(f"{url.path}:{DEFAULT_TAG}") - if not url.host: - url = URL(f"image://{username}/{url.path.lstrip('/')}") - - return cls(url=url, local=url.path.lstrip("/")) +# TODO: maybe move to cli/utqils as image_utils +@dataclass(frozen=True) +class DockerImage: + name: str + tag: str = "latest" + owner: Optional[str] = None + registry: Optional[str] = None - @classmethod - def from_local(cls, name: str, username: str) -> "Image": - colon_count = name.count(":") - if colon_count > 1: - raise ValueError(f"Invalid image name, only one colon allowed: {name}") + def as_url(self) -> str: + # TODO (ajuszkowski, 11-Feb-2019) should be host:port (see URL.explicit_port) + pre = f"{IMAGE_SCHEME}://{self.owner}/" if self.registry and self.owner else "" + return f"{pre}{self.name}:{self.tag}" - if not colon_count: - name = f"{name}:{DEFAULT_TAG}" + def as_repo(self) -> str: + # TODO (ajuszkowski, 11-Feb-2019) should be host:port (see URL.explicit_port) + pre = f"{self.registry}/{self.owner}/" if self.registry and self.owner else "" + return pre + self.name - return cls(url=URL(f"image://{username}/{name}"), local=name) + def as_local(self) -> str: + return f"{self.name}:{self.tag}" class Images: @@ -93,22 +82,21 @@ async def close(self) -> None: def _auth(self) -> Dict[str, str]: return {"username": "token", "password": self._config.token} - def _repo(self, image: Image) -> str: - # TODO (ajuszkowski, 11-Feb-2019) here we use only registry host, not host:port - return f"{self._config.registry_url.host}/{image.url.host}{image.url.path}" - async def push( - self, local_image: Image, remote_image: Image, spinner: AbstractSpinner - ) -> Image: - repo = self._repo(local_image) + self, + local_image: DockerImage, + remote_image: DockerImage, + spinner: AbstractSpinner, + ) -> DockerImage: + repo = remote_image.as_repo() spinner.start("Pushing image ...") try: - await self._docker.images.tag(local_image.local, repo) + await self._docker.images.tag(local_image.as_local(), repo) except DockerError as error: spinner.complete() if error.status == STATUS_NOT_FOUND: raise ValueError( - f"Image {local_image.local} was not found " + f"Image {local_image.as_local()} was not found " "in your local docker images" ) from error spinner.tick() @@ -121,7 +109,9 @@ async def push( spinner.complete() # TODO check this part when registry fixed if error.status == STATUS_FORBIDDEN: - raise AuthorizationError(f"Access denied {remote_image.url}") from error + raise AuthorizationError( + f"Access denied {remote_image.as_url()}" + ) from error raise # pragma: no cover async for obj in stream: spinner.tick() @@ -133,9 +123,12 @@ async def push( return remote_image async def pull( - self, remote_image: Image, local_image: Image, spinner: AbstractSpinner - ) -> Image: - repo = self._repo(remote_image) + self, + remote_image: DockerImage, + local_image: DockerImage, + spinner: AbstractSpinner, + ) -> DockerImage: + repo = remote_image.as_repo() spinner.start("Pulling image ...") try: stream = await self._docker.pull( @@ -146,11 +139,13 @@ async def pull( spinner.complete() if error.status == STATUS_NOT_FOUND: raise ValueError( - f"Image {remote_image.url} was not found " "in registry" + f"Image {remote_image.as_url()} was not found " "in registry" ) from error # TODO check this part when registry fixed elif error.status == STATUS_FORBIDDEN: - raise AuthorizationError(f"Access denied {remote_image.url}") from error + raise AuthorizationError( + f"Access denied {remote_image.as_url()}" + ) from error raise # pragma: no cover spinner.tick() @@ -162,7 +157,7 @@ async def pull( raise DockerError(STATUS_CUSTOM_ERROR, error_details) spinner.tick() - await self._docker.images.tag(repo, local_image.local) + await self._docker.images.tag(repo, local_image.as_local()) spinner.complete() return local_image diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py new file mode 100644 index 000000000..00fffd608 --- /dev/null +++ b/python/neuromation/client/parsing_utils.py @@ -0,0 +1,105 @@ +from typing import Tuple + +from yarl import URL + +from .images import IMAGE_SCHEME, DockerImage + + +class ImageParser: + default_tag = "latest" + + def __init__(self, username: str, registry_url: str): + self._username = username + self._registry = self._get_registry_hostname(registry_url) + + def parse_local(self, image: str) -> DockerImage: + try: + return self._parse_local(image) + except ValueError as e: + raise ValueError(f"Invalid local image '{image}': {e}") from e + + def parse_remote(self, image: str, require_scheme: bool = True) -> DockerImage: + try: + return self._parse_remote(image, require_scheme) + except ValueError as e: + if not require_scheme: + try: + as_local = self.parse_local(image) + return DockerImage(name=as_local.name, tag=as_local.tag) + except ValueError: + pass + raise ValueError(f"Invalid remote image '{image}': {e}") from e + + def _parse_local(self, image: str) -> DockerImage: + if not image: + raise ValueError("empty image name") + + # This is hack because URL("ubuntu:v1") is parsed as scheme=ubuntu path=v1 + if image.startswith(f"{IMAGE_SCHEME}://"): + raise ValueError( + f"scheme '{IMAGE_SCHEME}://' is not allowed for local images" + ) + + name, tag = self._split_image_name(image.lstrip("/")) + + return DockerImage(name=name, tag=tag) + + def _parse_remote(self, image: str, require_scheme: bool) -> DockerImage: + if not image: + raise ValueError("empty image name") + + url = URL(image) + + self._check_allowed_uri_elements(url) + + registry, owner = None, None + if url.scheme: + if url.scheme != IMAGE_SCHEME: + scheme = f"{url.scheme}://" if url.scheme else "" + raise ValueError( + f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}" + ) + registry = self._registry + owner = self._username if not url.host or url.host == "~" else url.host + elif require_scheme: + raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") + + name, tag = self._split_image_name(url.path.lstrip("/")) + + return DockerImage(name=name, tag=tag, registry=registry, owner=owner) + + def _split_image_name(self, image: str) -> Tuple[str, str]: + colon_count = image.count(":") + if colon_count == 0: + image, tag = image, self.default_tag + elif colon_count == 1: + image, tag = image.split(":") + else: + raise ValueError(f"cannot parse image name '{image}': too many tags") + return image, tag + + def _get_registry_hostname(self, registry_url: str) -> str: + try: + url = URL(registry_url) + except ValueError as e: + raise ValueError(f"Could not parse registry URL: {e}") + if not url.host: + raise ValueError( + f"Empty hostname in registry URL '{registry_url}': " + "please consider updating configuration" + ) + return url.host + + def _check_allowed_uri_elements(self, url) -> None: + if url.path == "/": + raise ValueError("no image name specified") + if url.query: + raise ValueError(f"query is not allowed, found: '{url.query}'") + if url.fragment: + raise ValueError(f"fragment is not allowed, found: '{url.fragment}'") + if url.user: + raise ValueError(f"user is not allowed, found: '{url.user}'") + if url.password: + raise ValueError(f"password is not allowed, found: '{url.password}'") + if url.port: + raise ValueError(f"password is not allowed, found: '{url.port}'") From d216fe37e538d8bec23c15e5c79daa00fe73c452 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 15:56:25 +0300 Subject: [PATCH 02/28] remove unused logger --- python/neuromation/client/url_utils.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/python/neuromation/client/url_utils.py b/python/neuromation/client/url_utils.py index 0b1f38495..d277e81f0 100644 --- a/python/neuromation/client/url_utils.py +++ b/python/neuromation/client/url_utils.py @@ -1,12 +1,8 @@ -import logging from pathlib import Path from yarl import URL -log = logging.getLogger(__name__) - - def normalize_storage_path_uri(uri: URL, username: str) -> URL: """Normalize storage url.""" if uri.scheme != "storage": From 97b4e1ee9dba717eb95f2d4449a7644a3a7948d4 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 16:03:51 +0300 Subject: [PATCH 03/28] rename param --- python/neuromation/client/parsing_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 00fffd608..f3e06bf6a 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -8,8 +8,8 @@ class ImageParser: default_tag = "latest" - def __init__(self, username: str, registry_url: str): - self._username = username + def __init__(self, default_user: str, registry_url: str): + self._default_user = default_user self._registry = self._get_registry_hostname(registry_url) def parse_local(self, image: str) -> DockerImage: @@ -60,7 +60,7 @@ def _parse_remote(self, image: str, require_scheme: bool) -> DockerImage: f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}" ) registry = self._registry - owner = self._username if not url.host or url.host == "~" else url.host + owner = self._default_user if not url.host or url.host == "~" else url.host elif require_scheme: raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") From 644a85f08bb21b881ecb3b9d556984f8d8acf2ff Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 17:10:13 +0300 Subject: [PATCH 04/28] cleanup a bit --- python/neuromation/client/parsing_utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index f3e06bf6a..7b9687fe7 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -40,7 +40,7 @@ def _parse_local(self, image: str) -> DockerImage: f"scheme '{IMAGE_SCHEME}://' is not allowed for local images" ) - name, tag = self._split_image_name(image.lstrip("/")) + name, tag = self._split_image_name(image) return DockerImage(name=name, tag=tag) @@ -57,7 +57,7 @@ def _parse_remote(self, image: str, require_scheme: bool) -> DockerImage: if url.scheme != IMAGE_SCHEME: scheme = f"{url.scheme}://" if url.scheme else "" raise ValueError( - f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}" + f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}'" ) registry = self._registry owner = self._default_user if not url.host or url.host == "~" else url.host @@ -90,8 +90,8 @@ def _get_registry_hostname(self, registry_url: str) -> str: ) return url.host - def _check_allowed_uri_elements(self, url) -> None: - if url.path == "/": + def _check_allowed_uri_elements(self, url: URL) -> None: + if not url.path or url.path == "/": raise ValueError("no image name specified") if url.query: raise ValueError(f"query is not allowed, found: '{url.query}'") @@ -102,4 +102,4 @@ def _check_allowed_uri_elements(self, url) -> None: if url.password: raise ValueError(f"password is not allowed, found: '{url.password}'") if url.port: - raise ValueError(f"password is not allowed, found: '{url.port}'") + raise ValueError(f"port is not allowed, found: '{url.port}'") From fa36ccafe5f6ae79b7e0ca41cbb8d1b730779fc9 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 17:10:25 +0300 Subject: [PATCH 05/28] rewrite unit tests --- python/tests/client/test_images.py | 539 ++++++++++++++++++++++++----- 1 file changed, 448 insertions(+), 91 deletions(-) diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index a30afc66e..fef571f14 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -7,12 +7,12 @@ from yarl import URL from neuromation.cli.command_spinner import SpinnerBase -from neuromation.client import AuthorizationError, Client +from neuromation.client import AuthorizationError, Client, ImageParser from neuromation.client.images import ( STATUS_CUSTOM_ERROR, STATUS_FORBIDDEN, STATUS_NOT_FOUND, - Image, + DockerImage, ) @@ -24,95 +24,450 @@ def patch_docker_host(): yield -class TestImage: +class TestImageParser: + parser = ImageParser(default_user="alice", registry_url="https://reg.neu.ro") + @pytest.mark.parametrize( - "test_url,expected_url,expected_local", + "registry_url", [ - (URL("image://bob/php:7-fpm"), URL("image://bob/php:7-fpm"), "php:7-fpm"), - (URL("image://bob/php"), URL("image://bob/php:latest"), "php:latest"), - (URL("image:php:7-fpm"), URL("image://bob/php:7-fpm"), "php:7-fpm"), - (URL("image:php"), URL("image://bob/php:latest"), "php:latest"), - ( - URL("image://bob/project/php:7-fpm"), - URL("image://bob/project/php:7-fpm"), - "project/php:7-fpm", - ), - ( - URL("image:project/php"), - URL("image://bob/project/php:latest"), - "project/php:latest", - ), + "http://reg.neu.ro", + "https://reg.neu.ro", + "http://reg.neu.ro:5000", + "https://reg.neu.ro/bla/bla", + "http://reg.neu.ro:5000/bla/bla", ], ) - def test_correct_url(self, test_url, expected_url, expected_local): - image = Image.from_url(test_url, "bob") - assert image.url == expected_url - assert image.local == expected_local - - def test_from_empty_url(self): - with pytest.raises(ValueError, match="Image URL cannot be empty"): - Image.from_url(url=URL(""), username="bob") - pass - - def test_from_invalid_scheme_url(self): - with pytest.raises(ValueError, match=r"Invalid scheme"): - Image.from_url( - url=URL("http://neuromation.io/what/does/the/fox/say"), username="ylvis" - ) - pass - - def test_empty_path_url(self): - with pytest.raises(ValueError, match=r"Image URL cannot be empty"): - Image.from_url(url=URL("image:"), username="bob") - with pytest.raises(ValueError, match=r"Invalid image"): - Image.from_url(url=URL("image:///"), username="bob") - pass - - def test_url_with_query(self): - with pytest.raises(ValueError, match=r"Invalid image"): - Image.from_url(url=URL("image://bob/image?bad=idea"), username="bob") - pass - - def test_url_with_user(self): - with pytest.raises(ValueError, match=r"Invalid image"): - Image.from_url(url=URL("image://alien@bob/image"), username="bob") - pass - - def test_url_with_port(self): - with pytest.raises(ValueError, match=r"Invalid image"): - Image.from_url(url=URL("image://bob:80/image"), username="bob") - pass - - def test_url_with_few_colons(self): - with pytest.raises(ValueError, match=r"only one colon allowed"): - Image.from_url(url=URL("image://bob/image:tag1:tag2"), username="bob") - pass + def test__get_registry_hostname(self, registry_url): + registry = self.parser._get_registry_hostname(registry_url) + assert registry == "reg.neu.ro" + + @pytest.mark.parametrize( + "registry_url", + ["", "reg.neu.ro", "reg.neu.ro:5000", "https://", "https:///bla/bla"], + ) + def test__get_registry_hostname_bad_registry_url(self, registry_url): + with pytest.raises(ValueError, match="Empty hostname in registry URL"): + self.parser._get_registry_hostname(registry_url) + + def test__split_image_name_no_colon(self): + splitted = self.parser._split_image_name("ubuntu") + assert splitted == ("ubuntu", "latest") + + def test__split_image_name_1_colon(self): + splitted = self.parser._split_image_name("ubuntu:v10.04") + assert splitted == ("ubuntu", "v10.04") + + def test__split_image_name_2_colon(self): + with pytest.raises(ValueError, match="too many tags"): + self.parser._split_image_name("ubuntu:v10.04:LTS") + + @pytest.mark.parametrize( + "url", ["", "/", "image://", "image:///", "image://bob", "image://bob/"] + ) + def test__check_allowed_uri_elements__no_path(self, url): + with pytest.raises(ValueError, match="no image name specified"): + self.parser._check_allowed_uri_elements(URL(url)) + + @pytest.mark.parametrize( + "url", + [ + "image://bob/ubuntu:v10.04?key=value", + "image://bob/ubuntu?key=value", + "image:///ubuntu?key=value", + "image:ubuntu?key=value", + ], + ) + def test__check_allowed_uri_elements__with_query(self, url): + with pytest.raises(ValueError, match="query is not allowed"): + self.parser._check_allowed_uri_elements(URL(url)) @pytest.mark.parametrize( - "test_local,expected_url,expected_local", + "url", [ - ("php:7-fpm", URL("image://bob/php:7-fpm"), "php:7-fpm"), - ("php", URL("image://bob/php:latest"), "php:latest"), - ( - "project/php:7-fpm", - URL("image://bob/project/php:7-fpm"), - "project/php:7-fpm", - ), - ( - "project/php", - URL("image://bob/project/php:latest"), - "project/php:latest", - ), + "image://bob/ubuntu:v10.04#fragment", + "image://bob/ubuntu#fragment", + "image:///ubuntu#fragment", + "image:ubuntu#fragment", ], ) - def test_correct_local(self, test_local, expected_url, expected_local): - image = Image.from_local(test_local, "bob") - assert image.url == expected_url - assert image.local == expected_local + def test__check_allowed_uri_elements__with_fragment(self, url): + with pytest.raises(ValueError, match="fragment is not allowed"): + self.parser._check_allowed_uri_elements(URL(url)) + + def test__check_allowed_uri_elements__with_user(self): + url = "image://user@bob/ubuntu" + with pytest.raises(ValueError, match="user is not allowed"): + self.parser._check_allowed_uri_elements(URL(url)) + + def test__check_allowed_uri_elements__with_password(self): + url = "image://:password@bob/ubuntu" + with pytest.raises(ValueError, match="password is not allowed"): + self.parser._check_allowed_uri_elements(URL(url)) + + def test__check_allowed_uri_elements__with_port(self): + url = "image://bob:443/ubuntu" + with pytest.raises(ValueError, match="port is not allowed"): + self.parser._check_allowed_uri_elements(URL(url)) + + # public method: parse_local + + def test_parse_local_empty_fail(self): + image = "" + with pytest.raises(ValueError, match="empty image name"): + self.parser.parse_local(image) + + def test_parse_local_none_fail(self): + image = None + with pytest.raises(ValueError, match="empty image name"): + self.parser.parse_local(image) + + def test_parse_local_with_image_scheme_fail(self): + image = "image://ubuntu" + msg = "scheme 'image://' is not allowed for local images" + with pytest.raises(ValueError, match=msg): + self.parser.parse_local(image) + + def test_parse_local_with_other_scheme_ok(self): + image = "http://ubuntu" + parsed = self.parser.parse_local(image) + # instead of parser, the docker client will fail + assert parsed == DockerImage(name="http", tag="//ubuntu") + + def test_parse_local_no_tag(self): + image = "ubuntu" + parsed = self.parser.parse_local(image) + assert parsed == DockerImage(name="ubuntu", tag="latest") + + def test_parse_local_with_tag(self): + image = "ubuntu:v10.04" + parsed = self.parser.parse_local(image) + assert parsed == DockerImage(name="ubuntu", tag="v10.04") + + def test_parse_local_2_tag_fail(self): + image = "ubuntu:v10.04:LTS" + msg = "cannot parse image name 'ubuntu:v10.04:LTS': too many tags" + with pytest.raises(ValueError, match=msg): + self.parser.parse_local(image) + + # public method: parse_remote + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_empty_fail(self, require_scheme): + image = "" + with pytest.raises(ValueError, match="empty image name"): + self.parser.parse_remote(image, require_scheme=require_scheme) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_none_fail(self, require_scheme): + image = None + with pytest.raises(ValueError, match="empty image name"): + self.parser.parse_remote(image, require_scheme=require_scheme) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_with_user_with_tag(self, require_scheme): + image = "image://bob/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_with_user_with_tag_2(self, require_scheme): + image = "image://bob/library/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_with_user_no_tag(self, require_scheme): + image = "image://bob/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_with_user_no_tag_2(self, require_scheme): + image = "image://bob/library/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_no_slash_no_user_no_tag(self, require_scheme): + image = "image:ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_no_slash_no_user_no_tag_2(self, require_scheme): + image = "image:library/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_no_slash_no_user_with_tag(self, require_scheme): + image = "image:ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_no_slash_no_user_with_tag_2(self, require_scheme): + image = "image:library/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_1_slash_no_user_no_tag(self, require_scheme): + image = "image:/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_1_slash_no_user_no_tag_2(self, require_scheme): + image = "image:/library/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_1_slash_no_user_with_tag(self, require_scheme): + image = "image:/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_1_slash_no_user_with_tag_2(self, require_scheme): + image = "image:/library/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_2_slash_user_no_tag_fail(self, require_scheme): + image = "image://ubuntu" + with pytest.raises(ValueError, match="no image name specified"): + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_2_slash_user_with_tag_fail(self, require_scheme): + image = "image://ubuntu:v10.04" + with pytest.raises(ValueError, match="port can't be converted to integer"): + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_3_slash_no_user_no_tag(self, require_scheme): + image = "image:///ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_3_slash_no_user_no_tag_2(self, require_scheme): + image = "image:///library/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_3_slash_no_user_with_tag(self, require_scheme): + image = "image:///ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) - def test_local_with_few_colons(self): - with pytest.raises(ValueError, match=r"only one colon allowed"): - Image.from_local("image:tag1:tag2", "bob") + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_3_slash_no_user_with_tag_2(self, require_scheme): + image = "image:///library/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_4_slash_no_user_with_tag(self, require_scheme): + image = "image:////ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_4_slash_no_user_with_tag_2(self, require_scheme): + image = "image:////library/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme): + image = "image:////ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme): + image = "image:////library/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_tilde_user_no_tag(self, require_scheme): + image = "image://~/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_tilde_user_no_tag_2(self, require_scheme): + image = "image://~/library/ubuntu" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_tilde_user_with_tag(self, require_scheme): + image = "image://~/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + @pytest.mark.parametrize("require_scheme", [True, False]) + def test_parse_remote_with_scheme_tilde_user_with_tag_2(self, require_scheme): + image = "image://~/library/ubuntu:v10.04" + parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" + ) + + def test_parse_remote_no_scheme_no_slash_no_tag(self): + image = "ubuntu" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="ubuntu", tag="latest", owner=None, registry=None + ) + + def test_parse_remote_no_scheme_no_slash_with_tag(self): + image = "ubuntu:v10.04" + + msg = "scheme 'image://' expected, found: 'ubuntu://'" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="ubuntu", tag="v10.04", owner=None, registry=None + ) + + def test_parse_remote_no_scheme_1_slash_no_tag(self): + image = "library/ubuntu" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="library/ubuntu", tag="latest", owner=None, registry=None + ) + + def test_parse_remote_no_scheme_1_slash_with_tag(self): + image = "library/ubuntu:v10.04" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="library/ubuntu", tag="v10.04", owner=None, registry=None + ) + + def test_parse_remote_no_scheme_2_slash_no_tag(self): + image = "docker.io/library/ubuntu" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="docker.io/library/ubuntu", tag="latest", owner=None, registry=None + ) + + def test_parse_remote_no_scheme_2_slash_with_tag(self): + image = "docker.io/library/ubuntu:v10.04" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="docker.io/library/ubuntu", tag="v10.04", owner=None, registry=None + ) + + def test_parse_remote_no_scheme_3_slash_no_tag(self): + image = "something/docker.io/library/ubuntu" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="something/docker.io/library/ubuntu", + tag="latest", + owner=None, + registry=None, + ) + + def test_parse_remote_no_scheme_3_slash_with_tag(self): + image = "something/docker.io/library/ubuntu:v10.04" + + msg = "scheme 'image://' is required" + with pytest.raises(ValueError, match=msg): + parsed = self.parser.parse_remote(image, require_scheme=True) + + parsed = self.parser.parse_remote(image, require_scheme=False) + assert parsed == DockerImage( + name="something/docker.io/library/ubuntu", + tag="v10.04", + owner=None, + registry=None, + ) @pytest.mark.usefixtures("patch_docker_host") @@ -130,7 +485,7 @@ def spinner(self) -> SpinnerBase: async def test_unavailable_docker(self, patched_init, token, spinner): async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError, match=r"Docker engine is not available.+"): - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:bananas"), client.username ) await client.images.pull(image, image, spinner) @@ -141,7 +496,7 @@ async def test_unavailable_docker(self, patched_init, token, spinner): async def test_unknown_docker_error(self, patched_init, token, spinner): async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"something went wrong"): - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:bananas"), client.username ) await client.images.pull(image, image, spinner) @@ -153,7 +508,7 @@ async def test_push_non_existent_image(self, patched_tag, token, spinner): ) async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"not found"): - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:bananas-no-more"), client.username ) await client.images.push(image, image, spinner) @@ -169,7 +524,7 @@ async def test_push_image_to_foreign_repo( ) async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:bananas-not-for-you"), client.username ) await client.images.push(image, image, spinner) @@ -186,7 +541,7 @@ async def error_generator(): patched_push.return_value = error_generator() async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:bananas-wrong-food"), client.username ) await client.images.push(image, image, spinner) @@ -202,7 +557,7 @@ async def message_generator(): patched_tag.return_value = True patched_push.return_value = message_generator() async with Client(URL("https://api.localhost.localdomain"), token) as client: - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:banana-is-here"), client.username ) result = await client.images.push(image, image, spinner) @@ -215,7 +570,7 @@ async def test_pull_non_existent_image(self, patched_pull, token, spinner): ) async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"not found"): - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:no-bananas-here"), client.username ) await client.images.pull(image, image, spinner) @@ -227,7 +582,7 @@ async def test_pull_image_from_foreign_repo(self, patched_pull, token, spinner): ) async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:not-your-bananas"), client.username ) await client.images.pull(image, image, spinner) @@ -240,7 +595,7 @@ async def error_generator(): patched_pull.return_value = error_generator() async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: - image = Image.from_url( + image = DockerImage.build_remote( URL("image://bob/image:nuts-here"), client.username ) await client.images.pull(image, image, spinner) @@ -256,7 +611,9 @@ async def message_generator(): patched_tag.return_value = True patched_pull.return_value = message_generator() async with Client(URL("https://api.localhost.localdomain"), token) as client: - image = Image.from_url(URL("image://bob/image:bananas"), client.username) + image = DockerImage.build_remote( + URL("image://bob/image:bananas"), client.username + ) result = await client.images.pull(image, image, spinner) assert result == image From 5c2f26ea0c38934da5147a24c6941671a0f0e7a7 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 17:37:01 +0300 Subject: [PATCH 06/28] fix other tests --- python/tests/client/test_images.py | 42 +++++++++--------------------- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index fef571f14..aae2c3359 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -472,6 +472,8 @@ def test_parse_remote_no_scheme_3_slash_with_tag(self): @pytest.mark.usefixtures("patch_docker_host") class TestImages: + parser = ImageParser(default_user="bob", registry_url="https://reg.neu.ro") + @pytest.fixture() def spinner(self) -> SpinnerBase: return SpinnerBase.create_spinner(False) @@ -483,22 +485,18 @@ def spinner(self) -> SpinnerBase: ), ) async def test_unavailable_docker(self, patched_init, token, spinner): + image = self.parser.parse_remote(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError, match=r"Docker engine is not available.+"): - image = DockerImage.build_remote( - URL("image://bob/image:bananas"), client.username - ) await client.images.pull(image, image, spinner) @asynctest.mock.patch( "aiodocker.Docker.__init__", side_effect=ValueError("something went wrong") ) async def test_unknown_docker_error(self, patched_init, token, spinner): + image = self.parser.parse_remote(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"something went wrong"): - image = DockerImage.build_remote( - URL("image://bob/image:bananas"), client.username - ) await client.images.pull(image, image, spinner) @asynctest.mock.patch("aiodocker.images.DockerImages.tag") @@ -506,11 +504,9 @@ async def test_push_non_existent_image(self, patched_tag, token, spinner): patched_tag.side_effect = DockerError( STATUS_NOT_FOUND, {"message": "Mocked error"} ) + image = self.parser.parse_remote(f"image://bob/image:bananas-no-more") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"not found"): - image = DockerImage.build_remote( - URL("image://bob/image:bananas-no-more"), client.username - ) await client.images.push(image, image, spinner) @asynctest.mock.patch("aiodocker.images.DockerImages.tag") @@ -522,11 +518,9 @@ async def test_push_image_to_foreign_repo( patched_push.side_effect = DockerError( STATUS_FORBIDDEN, {"message": "Mocked error"} ) + image = self.parser.parse_remote(f"image://bob/image:bananas-no-more") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): - image = DockerImage.build_remote( - URL("image://bob/image:bananas-not-for-you"), client.username - ) await client.images.push(image, image, spinner) @asynctest.mock.patch("aiodocker.images.DockerImages.tag") @@ -539,11 +533,9 @@ async def error_generator(): patched_tag.return_value = True patched_push.return_value = error_generator() + image = self.parser.parse_remote(f"image://bob/image:bananas-wrong-food") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: - image = DockerImage.build_remote( - URL("image://bob/image:bananas-wrong-food"), client.username - ) await client.images.push(image, image, spinner) assert exc_info.value.status == STATUS_CUSTOM_ERROR assert exc_info.value.message == "Mocked message" @@ -556,10 +548,8 @@ async def message_generator(): patched_tag.return_value = True patched_push.return_value = message_generator() + image = self.parser.parse_remote(f"image://bob/image:bananas-is-here") async with Client(URL("https://api.localhost.localdomain"), token) as client: - image = DockerImage.build_remote( - URL("image://bob/image:banana-is-here"), client.username - ) result = await client.images.push(image, image, spinner) assert result == image @@ -569,10 +559,8 @@ async def test_pull_non_existent_image(self, patched_pull, token, spinner): STATUS_NOT_FOUND, {"message": "Mocked error"} ) async with Client(URL("https://api.localhost.localdomain"), token) as client: + image = self.parser.parse_remote(f"image://bob/image:no-bananas-here") with pytest.raises(ValueError, match=r"not found"): - image = DockerImage.build_remote( - URL("image://bob/image:no-bananas-here"), client.username - ) await client.images.pull(image, image, spinner) @asynctest.mock.patch("aiodocker.images.DockerImages.pull") @@ -580,11 +568,9 @@ async def test_pull_image_from_foreign_repo(self, patched_pull, token, spinner): patched_pull.side_effect = DockerError( STATUS_FORBIDDEN, {"message": "Mocked error"} ) + image = self.parser.parse_remote(f"image://bob/image:not-your-bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): - image = DockerImage.build_remote( - URL("image://bob/image:not-your-bananas"), client.username - ) await client.images.pull(image, image, spinner) @asynctest.mock.patch("aiodocker.images.DockerImages.pull") @@ -593,11 +579,9 @@ async def error_generator(): yield {"error": True, "errorDetail": {"message": "Mocked message"}} patched_pull.return_value = error_generator() + image = self.parser.parse_remote(f"image://bob/image:nuts-here") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: - image = DockerImage.build_remote( - URL("image://bob/image:nuts-here"), client.username - ) await client.images.pull(image, image, spinner) assert exc_info.value.status == STATUS_CUSTOM_ERROR assert exc_info.value.message == "Mocked message" @@ -610,10 +594,8 @@ async def message_generator(): patched_tag.return_value = True patched_pull.return_value = message_generator() + image = self.parser.parse_remote(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: - image = DockerImage.build_remote( - URL("image://bob/image:bananas"), client.username - ) result = await client.images.pull(image, image, spinner) assert result == image From 2177b2abd0f00eff08fca53955bf76084c2abbd9 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 18:22:29 +0300 Subject: [PATCH 07/28] fix default remote registry --- python/neuromation/cli/image.py | 12 ++++++++---- python/neuromation/cli/job.py | 4 +++- python/neuromation/client/parsing_utils.py | 20 +++++++++++++++++--- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index dae4e082c..770ca6020 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -41,10 +41,12 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: """ - parser = ImageParser(cfg.username, cfg.registry_url) + parser = ImageParser( + cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=True + ) local_img = parser.parse_local(image_name) remote_img = ( - parser.parse_remote(remote_image_name) + parser.parse_remote(remote_image_name, require_scheme=True) if remote_image_name else parser.parse_remote(image_name, require_scheme=False) ) @@ -82,8 +84,10 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: """ - parser = ImageParser(cfg.username, cfg.registry_url) - remote_img = parser.parse_remote(image_name) + parser = ImageParser( + cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=True + ) + remote_img = parser.parse_remote(image_name, require_scheme=True) local_img = ( parser.parse_local(local_image_name) if local_image_name diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index 1bc8dc7e9..f63ce8055 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -195,7 +195,9 @@ async def submit( memory = to_megabytes_str(memory) - image_parser = ImageParser(cfg.username, cfg.registry_url) + image_parser = ImageParser( + cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=False + ) parsed_image = image_parser.parse_remote(image, require_scheme=False) # TODO (ajuszkowski 01-Feb-19) process --quiet globally to set up logger+click if not quiet: diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 7b9687fe7..5f6071766 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -8,9 +8,15 @@ class ImageParser: default_tag = "latest" - def __init__(self, default_user: str, registry_url: str): + def __init__( + self, + default_user: str, + registry_url: str, + remote_by_default_in_neuro_registry: bool, + ): self._default_user = default_user self._registry = self._get_registry_hostname(registry_url) + self._remote_by_default_in_neuro_registry = remote_by_default_in_neuro_registry def parse_local(self, image: str) -> DockerImage: try: @@ -18,14 +24,19 @@ def parse_local(self, image: str) -> DockerImage: except ValueError as e: raise ValueError(f"Invalid local image '{image}': {e}") from e - def parse_remote(self, image: str, require_scheme: bool = True) -> DockerImage: + def parse_remote(self, image: str, require_scheme: bool) -> DockerImage: try: return self._parse_remote(image, require_scheme) except ValueError as e: if not require_scheme: try: as_local = self.parse_local(image) - return DockerImage(name=as_local.name, tag=as_local.tag) + return DockerImage( + name=as_local.name, + tag=as_local.tag, + owner=self._default_user, + registry=self._registry, + ) except ValueError: pass raise ValueError(f"Invalid remote image '{image}': {e}") from e @@ -63,6 +74,9 @@ def _parse_remote(self, image: str, require_scheme: bool) -> DockerImage: owner = self._default_user if not url.host or url.host == "~" else url.host elif require_scheme: raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") + elif self._remote_by_default_in_neuro_registry: + registry = self._registry + owner = self._default_user name, tag = self._split_image_name(url.path.lstrip("/")) From ae78d7a74049c6b8cb75c181c646edaf28c6e937 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 18:23:09 +0300 Subject: [PATCH 08/28] rename methods --- python/neuromation/cli/image.py | 12 +- python/neuromation/cli/job.py | 2 +- python/neuromation/client/parsing_utils.py | 6 +- python/tests/client/test_images.py | 122 ++++++++++----------- 4 files changed, 71 insertions(+), 71 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 770ca6020..7c2a77d84 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -44,11 +44,11 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: parser = ImageParser( cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=True ) - local_img = parser.parse_local(image_name) + local_img = parser.parse_as_local(image_name) remote_img = ( - parser.parse_remote(remote_image_name, require_scheme=True) + parser.parse_as_remote(remote_image_name, require_scheme=True) if remote_image_name - else parser.parse_remote(image_name, require_scheme=False) + else parser.parse_as_remote(image_name, require_scheme=False) ) # TODO: check out all todos by ajuszkowski in this PR! @@ -87,11 +87,11 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: parser = ImageParser( cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=True ) - remote_img = parser.parse_remote(image_name, require_scheme=True) + remote_img = parser.parse_as_remote(image_name, require_scheme=True) local_img = ( - parser.parse_local(local_image_name) + parser.parse_as_local(local_image_name) if local_image_name - else parser.parse_local(remote_img.as_local()) + else parser.parse_as_local(remote_img.as_local()) ) log.info(f"Using remote image '{remote_img.as_url()}'") diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index f63ce8055..8d79c7b4e 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -198,7 +198,7 @@ async def submit( image_parser = ImageParser( cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=False ) - parsed_image = image_parser.parse_remote(image, require_scheme=False) + parsed_image = image_parser.parse_as_remote(image, require_scheme=False) # TODO (ajuszkowski 01-Feb-19) process --quiet globally to set up logger+click if not quiet: log.info(f"Using image '{parsed_image.as_url()}'") diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 5f6071766..16f3230f6 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -18,19 +18,19 @@ def __init__( self._registry = self._get_registry_hostname(registry_url) self._remote_by_default_in_neuro_registry = remote_by_default_in_neuro_registry - def parse_local(self, image: str) -> DockerImage: + def parse_as_local(self, image: str) -> DockerImage: try: return self._parse_local(image) except ValueError as e: raise ValueError(f"Invalid local image '{image}': {e}") from e - def parse_remote(self, image: str, require_scheme: bool) -> DockerImage: + def parse_as_remote(self, image: str, require_scheme: bool) -> DockerImage: try: return self._parse_remote(image, require_scheme) except ValueError as e: if not require_scheme: try: - as_local = self.parse_local(image) + as_local = self.parse_as_local(image) return DockerImage( name=as_local.name, tag=as_local.tag, diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index aae2c3359..48e0f26b1 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -114,40 +114,40 @@ def test__check_allowed_uri_elements__with_port(self): def test_parse_local_empty_fail(self): image = "" with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_local(image) + self.parser.parse_as_local(image) def test_parse_local_none_fail(self): image = None with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_local(image) + self.parser.parse_as_local(image) def test_parse_local_with_image_scheme_fail(self): image = "image://ubuntu" msg = "scheme 'image://' is not allowed for local images" with pytest.raises(ValueError, match=msg): - self.parser.parse_local(image) + self.parser.parse_as_local(image) def test_parse_local_with_other_scheme_ok(self): image = "http://ubuntu" - parsed = self.parser.parse_local(image) + parsed = self.parser.parse_as_local(image) # instead of parser, the docker client will fail assert parsed == DockerImage(name="http", tag="//ubuntu") def test_parse_local_no_tag(self): image = "ubuntu" - parsed = self.parser.parse_local(image) + parsed = self.parser.parse_as_local(image) assert parsed == DockerImage(name="ubuntu", tag="latest") def test_parse_local_with_tag(self): image = "ubuntu:v10.04" - parsed = self.parser.parse_local(image) + parsed = self.parser.parse_as_local(image) assert parsed == DockerImage(name="ubuntu", tag="v10.04") def test_parse_local_2_tag_fail(self): image = "ubuntu:v10.04:LTS" msg = "cannot parse image name 'ubuntu:v10.04:LTS': too many tags" with pytest.raises(ValueError, match=msg): - self.parser.parse_local(image) + self.parser.parse_as_local(image) # public method: parse_remote @@ -155,18 +155,18 @@ def test_parse_local_2_tag_fail(self): def test_parse_remote_empty_fail(self, require_scheme): image = "" with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_remote(image, require_scheme=require_scheme) + self.parser.parse_as_remote(image, require_scheme=require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_none_fail(self, require_scheme): image = None with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_remote(image, require_scheme=require_scheme) + self.parser.parse_as_remote(image, require_scheme=require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_with_user_with_tag(self, require_scheme): image = "image://bob/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" ) @@ -174,7 +174,7 @@ def test_parse_remote_with_scheme_with_user_with_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_with_user_with_tag_2(self, require_scheme): image = "image://bob/library/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" ) @@ -182,7 +182,7 @@ def test_parse_remote_with_scheme_with_user_with_tag_2(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_with_user_no_tag(self, require_scheme): image = "image://bob/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" ) @@ -190,7 +190,7 @@ def test_parse_remote_with_scheme_with_user_no_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_with_user_no_tag_2(self, require_scheme): image = "image://bob/library/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" ) @@ -198,7 +198,7 @@ def test_parse_remote_with_scheme_with_user_no_tag_2(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_no_slash_no_user_no_tag(self, require_scheme): image = "image:ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -206,7 +206,7 @@ def test_parse_remote_with_scheme_no_slash_no_user_no_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_no_slash_no_user_no_tag_2(self, require_scheme): image = "image:library/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -214,7 +214,7 @@ def test_parse_remote_with_scheme_no_slash_no_user_no_tag_2(self, require_scheme @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_no_slash_no_user_with_tag(self, require_scheme): image = "image:ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -222,7 +222,7 @@ def test_parse_remote_with_scheme_no_slash_no_user_with_tag(self, require_scheme @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_no_slash_no_user_with_tag_2(self, require_scheme): image = "image:library/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -230,7 +230,7 @@ def test_parse_remote_with_scheme_no_slash_no_user_with_tag_2(self, require_sche @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_1_slash_no_user_no_tag(self, require_scheme): image = "image:/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -238,7 +238,7 @@ def test_parse_remote_with_scheme_1_slash_no_user_no_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_1_slash_no_user_no_tag_2(self, require_scheme): image = "image:/library/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -246,7 +246,7 @@ def test_parse_remote_with_scheme_1_slash_no_user_no_tag_2(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_1_slash_no_user_with_tag(self, require_scheme): image = "image:/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -254,7 +254,7 @@ def test_parse_remote_with_scheme_1_slash_no_user_with_tag(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_1_slash_no_user_with_tag_2(self, require_scheme): image = "image:/library/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -263,18 +263,18 @@ def test_parse_remote_with_scheme_1_slash_no_user_with_tag_2(self, require_schem def test_parse_remote_with_scheme_2_slash_user_no_tag_fail(self, require_scheme): image = "image://ubuntu" with pytest.raises(ValueError, match="no image name specified"): - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_2_slash_user_with_tag_fail(self, require_scheme): image = "image://ubuntu:v10.04" with pytest.raises(ValueError, match="port can't be converted to integer"): - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_3_slash_no_user_no_tag(self, require_scheme): image = "image:///ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -282,7 +282,7 @@ def test_parse_remote_with_scheme_3_slash_no_user_no_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_3_slash_no_user_no_tag_2(self, require_scheme): image = "image:///library/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -290,7 +290,7 @@ def test_parse_remote_with_scheme_3_slash_no_user_no_tag_2(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_3_slash_no_user_with_tag(self, require_scheme): image = "image:///ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -298,7 +298,7 @@ def test_parse_remote_with_scheme_3_slash_no_user_with_tag(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_3_slash_no_user_with_tag_2(self, require_scheme): image = "image:///library/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -306,7 +306,7 @@ def test_parse_remote_with_scheme_3_slash_no_user_with_tag_2(self, require_schem @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_4_slash_no_user_with_tag(self, require_scheme): image = "image:////ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -314,7 +314,7 @@ def test_parse_remote_with_scheme_4_slash_no_user_with_tag(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_4_slash_no_user_with_tag_2(self, require_scheme): image = "image:////library/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -322,7 +322,7 @@ def test_parse_remote_with_scheme_4_slash_no_user_with_tag_2(self, require_schem @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme): image = "image:////ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -330,7 +330,7 @@ def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme): image = "image:////library/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -338,7 +338,7 @@ def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_tilde_user_no_tag(self, require_scheme): image = "image://~/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -346,7 +346,7 @@ def test_parse_remote_with_scheme_tilde_user_no_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_tilde_user_no_tag_2(self, require_scheme): image = "image://~/library/ubuntu" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @@ -354,7 +354,7 @@ def test_parse_remote_with_scheme_tilde_user_no_tag_2(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_tilde_user_with_tag(self, require_scheme): image = "image://~/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -362,7 +362,7 @@ def test_parse_remote_with_scheme_tilde_user_with_tag(self, require_scheme): @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_remote_with_scheme_tilde_user_with_tag_2(self, require_scheme): image = "image://~/library/ubuntu:v10.04" - parsed = self.parser.parse_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @@ -372,9 +372,9 @@ def test_parse_remote_no_scheme_no_slash_no_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="ubuntu", tag="latest", owner=None, registry=None ) @@ -384,9 +384,9 @@ def test_parse_remote_no_scheme_no_slash_with_tag(self): msg = "scheme 'image://' expected, found: 'ubuntu://'" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner=None, registry=None ) @@ -396,9 +396,9 @@ def test_parse_remote_no_scheme_1_slash_no_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner=None, registry=None ) @@ -408,9 +408,9 @@ def test_parse_remote_no_scheme_1_slash_with_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner=None, registry=None ) @@ -420,9 +420,9 @@ def test_parse_remote_no_scheme_2_slash_no_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="docker.io/library/ubuntu", tag="latest", owner=None, registry=None ) @@ -432,9 +432,9 @@ def test_parse_remote_no_scheme_2_slash_with_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="docker.io/library/ubuntu", tag="v10.04", owner=None, registry=None ) @@ -444,9 +444,9 @@ def test_parse_remote_no_scheme_3_slash_no_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="something/docker.io/library/ubuntu", tag="latest", @@ -459,9 +459,9 @@ def test_parse_remote_no_scheme_3_slash_with_tag(self): msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_remote(image, require_scheme=True) + parsed = self.parser.parse_as_remote(image, require_scheme=True) - parsed = self.parser.parse_remote(image, require_scheme=False) + parsed = self.parser.parse_as_remote(image, require_scheme=False) assert parsed == DockerImage( name="something/docker.io/library/ubuntu", tag="v10.04", @@ -485,7 +485,7 @@ def spinner(self) -> SpinnerBase: ), ) async def test_unavailable_docker(self, patched_init, token, spinner): - image = self.parser.parse_remote(f"image://bob/image:bananas") + image = self.parser.parse_as_remote(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError, match=r"Docker engine is not available.+"): await client.images.pull(image, image, spinner) @@ -494,7 +494,7 @@ async def test_unavailable_docker(self, patched_init, token, spinner): "aiodocker.Docker.__init__", side_effect=ValueError("something went wrong") ) async def test_unknown_docker_error(self, patched_init, token, spinner): - image = self.parser.parse_remote(f"image://bob/image:bananas") + image = self.parser.parse_as_remote(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"something went wrong"): await client.images.pull(image, image, spinner) @@ -504,7 +504,7 @@ async def test_push_non_existent_image(self, patched_tag, token, spinner): patched_tag.side_effect = DockerError( STATUS_NOT_FOUND, {"message": "Mocked error"} ) - image = self.parser.parse_remote(f"image://bob/image:bananas-no-more") + image = self.parser.parse_as_remote(f"image://bob/image:bananas-no-more") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"not found"): await client.images.push(image, image, spinner) @@ -518,7 +518,7 @@ async def test_push_image_to_foreign_repo( patched_push.side_effect = DockerError( STATUS_FORBIDDEN, {"message": "Mocked error"} ) - image = self.parser.parse_remote(f"image://bob/image:bananas-no-more") + image = self.parser.parse_as_remote(f"image://bob/image:bananas-no-more") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): await client.images.push(image, image, spinner) @@ -533,7 +533,7 @@ async def error_generator(): patched_tag.return_value = True patched_push.return_value = error_generator() - image = self.parser.parse_remote(f"image://bob/image:bananas-wrong-food") + image = self.parser.parse_as_remote(f"image://bob/image:bananas-wrong-food") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: await client.images.push(image, image, spinner) @@ -548,7 +548,7 @@ async def message_generator(): patched_tag.return_value = True patched_push.return_value = message_generator() - image = self.parser.parse_remote(f"image://bob/image:bananas-is-here") + image = self.parser.parse_as_remote(f"image://bob/image:bananas-is-here") async with Client(URL("https://api.localhost.localdomain"), token) as client: result = await client.images.push(image, image, spinner) assert result == image @@ -559,7 +559,7 @@ async def test_pull_non_existent_image(self, patched_pull, token, spinner): STATUS_NOT_FOUND, {"message": "Mocked error"} ) async with Client(URL("https://api.localhost.localdomain"), token) as client: - image = self.parser.parse_remote(f"image://bob/image:no-bananas-here") + image = self.parser.parse_as_remote(f"image://bob/image:no-bananas-here") with pytest.raises(ValueError, match=r"not found"): await client.images.pull(image, image, spinner) @@ -568,7 +568,7 @@ async def test_pull_image_from_foreign_repo(self, patched_pull, token, spinner): patched_pull.side_effect = DockerError( STATUS_FORBIDDEN, {"message": "Mocked error"} ) - image = self.parser.parse_remote(f"image://bob/image:not-your-bananas") + image = self.parser.parse_as_remote(f"image://bob/image:not-your-bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): await client.images.pull(image, image, spinner) @@ -579,7 +579,7 @@ async def error_generator(): yield {"error": True, "errorDetail": {"message": "Mocked message"}} patched_pull.return_value = error_generator() - image = self.parser.parse_remote(f"image://bob/image:nuts-here") + image = self.parser.parse_as_remote(f"image://bob/image:nuts-here") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: await client.images.pull(image, image, spinner) @@ -594,7 +594,7 @@ async def message_generator(): patched_tag.return_value = True patched_pull.return_value = message_generator() - image = self.parser.parse_remote(f"image://bob/image:bananas") + image = self.parser.parse_as_remote(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: result = await client.images.pull(image, image, spinner) assert result == image From ba44f30e1c1bd621b093b0b1dabb3363fcafc799 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 18:34:01 +0300 Subject: [PATCH 09/28] cleanup again --- python/neuromation/cli/image.py | 28 +++++------ python/neuromation/cli/job.py | 14 +++--- python/neuromation/client/images.py | 30 ++++++------ python/neuromation/client/parsing_utils.py | 55 +++++++++------------- 4 files changed, 57 insertions(+), 70 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 7c2a77d84..5cb1abb07 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -41,19 +41,17 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: """ - parser = ImageParser( - cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=True - ) + parser = ImageParser(cfg.username, cfg.registry_url) local_img = parser.parse_as_local(image_name) remote_img = ( - parser.parse_as_remote(remote_image_name, require_scheme=True) + parser.parse_as_remote(remote_image_name) if remote_image_name - else parser.parse_as_remote(image_name, require_scheme=False) + else parser.convert_to_remote_in_neuro_registry(local_img) ) # TODO: check out all todos by ajuszkowski in this PR! - log.info(f"Using local image '{local_img.as_local()}'") - log.info(f"Using remote image '{remote_img.as_url()}'") + log.info(f"Using local image '{local_img.as_local_str()}'") + log.info(f"Using remote image '{remote_img.as_url_str()}'") log.debug(f"LOCAL: '{local_img}'") log.debug(f"REMOTE: '{remote_img}'") @@ -61,7 +59,7 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: async with cfg.make_client() as client: result_remote_image = await client.images.push(local_img, remote_img, spinner) - click.echo(result_remote_image.as_url()) + click.echo(result_remote_image.as_url_str()) @command() @@ -84,18 +82,16 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: """ - parser = ImageParser( - cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=True - ) - remote_img = parser.parse_as_remote(image_name, require_scheme=True) + parser = ImageParser(cfg.username, cfg.registry_url) + remote_img = parser.parse_as_remote(image_name) local_img = ( parser.parse_as_local(local_image_name) if local_image_name - else parser.parse_as_local(remote_img.as_local()) + else parser.convert_to_local(remote_img) ) - log.info(f"Using remote image '{remote_img.as_url()}'") - log.info(f"Using local image '{local_img.as_local()}'") + log.info(f"Using remote image '{remote_img.as_url_str()}'") + log.info(f"Using local image '{local_img.as_local_str()}'") log.debug(f"REMOTE: '{remote_img}'") log.debug(f"LOCAL: '{local_img}'") @@ -103,7 +99,7 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: async with cfg.make_client() as client: result_local_image = await client.images.pull(remote_img, local_img, spinner) - click.echo(result_local_image.as_local()) + click.echo(result_local_image.as_local_str()) @command() diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index 8d79c7b4e..7b148e57f 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -195,15 +195,17 @@ async def submit( memory = to_megabytes_str(memory) - image_parser = ImageParser( - cfg.username, cfg.registry_url, remote_by_default_in_neuro_registry=False - ) - parsed_image = image_parser.parse_as_remote(image, require_scheme=False) + image_parser = ImageParser(cfg.username, cfg.registry_url) + try: + parsed_image = image_parser.parse_as_remote(image) + except ValueError: + parsed_image = image_parser.parse_as_local(image) + # TODO (ajuszkowski 01-Feb-19) process --quiet globally to set up logger+click if not quiet: - log.info(f"Using image '{parsed_image.as_url()}'") + log.info(f"Using image '{parsed_image.as_url_str()}'") log.debug(f"IMAGE: {parsed_image}") - image_obj = Image(image=parsed_image.as_repo(), command=cmd) + image_obj = Image(image=parsed_image.as_repo_str(), command=cmd) network = NetworkPortForwarding.from_cli(http, ssh) resources = Resources.create(cpu, gpu, gpu_model, memory, extshm) diff --git a/python/neuromation/client/images.py b/python/neuromation/client/images.py index ae44563ca..9056474ef 100644 --- a/python/neuromation/client/images.py +++ b/python/neuromation/client/images.py @@ -32,17 +32,19 @@ class DockerImage: owner: Optional[str] = None registry: Optional[str] = None - def as_url(self) -> str: - # TODO (ajuszkowski, 11-Feb-2019) should be host:port (see URL.explicit_port) - pre = f"{IMAGE_SCHEME}://{self.owner}/" if self.registry and self.owner else "" + def is_in_neuro_registry(self) -> bool: + return bool(self.registry and self.owner) + + def as_url_str(self) -> str: + pre = f"{IMAGE_SCHEME}://{self.owner}/" if self.is_in_neuro_registry() else "" return f"{pre}{self.name}:{self.tag}" - def as_repo(self) -> str: + def as_repo_str(self) -> str: # TODO (ajuszkowski, 11-Feb-2019) should be host:port (see URL.explicit_port) - pre = f"{self.registry}/{self.owner}/" if self.registry and self.owner else "" + pre = f"{self.registry}/{self.owner}/" if self.is_in_neuro_registry() else "" return pre + self.name - def as_local(self) -> str: + def as_local_str(self) -> str: return f"{self.name}:{self.tag}" @@ -88,15 +90,15 @@ async def push( remote_image: DockerImage, spinner: AbstractSpinner, ) -> DockerImage: - repo = remote_image.as_repo() + repo = remote_image.as_repo_str() spinner.start("Pushing image ...") try: - await self._docker.images.tag(local_image.as_local(), repo) + await self._docker.images.tag(local_image.as_local_str(), repo) except DockerError as error: spinner.complete() if error.status == STATUS_NOT_FOUND: raise ValueError( - f"Image {local_image.as_local()} was not found " + f"Image {local_image.as_local_str()} was not found " "in your local docker images" ) from error spinner.tick() @@ -110,7 +112,7 @@ async def push( # TODO check this part when registry fixed if error.status == STATUS_FORBIDDEN: raise AuthorizationError( - f"Access denied {remote_image.as_url()}" + f"Access denied {remote_image.as_url_str()}" ) from error raise # pragma: no cover async for obj in stream: @@ -128,7 +130,7 @@ async def pull( local_image: DockerImage, spinner: AbstractSpinner, ) -> DockerImage: - repo = remote_image.as_repo() + repo = remote_image.as_repo_str() spinner.start("Pulling image ...") try: stream = await self._docker.pull( @@ -139,12 +141,12 @@ async def pull( spinner.complete() if error.status == STATUS_NOT_FOUND: raise ValueError( - f"Image {remote_image.as_url()} was not found " "in registry" + f"Image {remote_image.as_url_str()} was not found " "in registry" ) from error # TODO check this part when registry fixed elif error.status == STATUS_FORBIDDEN: raise AuthorizationError( - f"Access denied {remote_image.as_url()}" + f"Access denied {remote_image.as_url_str()}" ) from error raise # pragma: no cover spinner.tick() @@ -157,7 +159,7 @@ async def pull( raise DockerError(STATUS_CUSTOM_ERROR, error_details) spinner.tick() - await self._docker.images.tag(repo, local_image.as_local()) + await self._docker.images.tag(repo, local_image.as_local_str()) spinner.complete() return local_image diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 16f3230f6..43d71659a 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -8,15 +8,9 @@ class ImageParser: default_tag = "latest" - def __init__( - self, - default_user: str, - registry_url: str, - remote_by_default_in_neuro_registry: bool, - ): + def __init__(self, default_user: str, registry_url: str): self._default_user = default_user self._registry = self._get_registry_hostname(registry_url) - self._remote_by_default_in_neuro_registry = remote_by_default_in_neuro_registry def parse_as_local(self, image: str) -> DockerImage: try: @@ -24,23 +18,23 @@ def parse_as_local(self, image: str) -> DockerImage: except ValueError as e: raise ValueError(f"Invalid local image '{image}': {e}") from e - def parse_as_remote(self, image: str, require_scheme: bool) -> DockerImage: + def parse_as_remote(self, image: str) -> DockerImage: try: - return self._parse_remote(image, require_scheme) + return self._parse_remote(image) except ValueError as e: - if not require_scheme: - try: - as_local = self.parse_as_local(image) - return DockerImage( - name=as_local.name, - tag=as_local.tag, - owner=self._default_user, - registry=self._registry, - ) - except ValueError: - pass raise ValueError(f"Invalid remote image '{image}': {e}") from e + def convert_to_remote_in_neuro_registry(self, image: DockerImage) -> DockerImage: + return DockerImage( + name=image.name, + tag=image.tag, + owner=self._default_user, + registry=self._registry, + ) + + def convert_to_local(self, image: DockerImage) -> DockerImage: + return DockerImage(name=image.name, tag=image.tag) + def _parse_local(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") @@ -55,7 +49,7 @@ def _parse_local(self, image: str) -> DockerImage: return DockerImage(name=name, tag=tag) - def _parse_remote(self, image: str, require_scheme: bool) -> DockerImage: + def _parse_remote(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") @@ -63,21 +57,14 @@ def _parse_remote(self, image: str, require_scheme: bool) -> DockerImage: self._check_allowed_uri_elements(url) - registry, owner = None, None - if url.scheme: - if url.scheme != IMAGE_SCHEME: - scheme = f"{url.scheme}://" if url.scheme else "" - raise ValueError( - f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}'" - ) - registry = self._registry - owner = self._default_user if not url.host or url.host == "~" else url.host - elif require_scheme: + if not url.scheme: raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") - elif self._remote_by_default_in_neuro_registry: - registry = self._registry - owner = self._default_user + if url.scheme != IMAGE_SCHEME: + scheme = f"{url.scheme}://" if url.scheme else "" + raise ValueError(f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}'") + registry = self._registry + owner = self._default_user if not url.host or url.host == "~" else url.host name, tag = self._split_image_name(url.path.lstrip("/")) return DockerImage(name=name, tag=tag, registry=registry, owner=owner) From a68c851d2561339bde8605db7d71e8049a3b9179 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 20:02:49 +0300 Subject: [PATCH 10/28] cleanup + fix job submit logic --- python/neuromation/cli/image.py | 10 +- python/neuromation/cli/job.py | 10 +- python/neuromation/client/images.py | 3 +- python/neuromation/client/parsing_utils.py | 21 +- python/tests/client/test_images.py | 290 ++++++++++----------- 5 files changed, 164 insertions(+), 170 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 5cb1abb07..6c83268ea 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -42,9 +42,9 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: """ parser = ImageParser(cfg.username, cfg.registry_url) - local_img = parser.parse_as_local(image_name) + local_img = parser.parse_as_docker_image(image_name) remote_img = ( - parser.parse_as_remote(remote_image_name) + parser.parse_as_neuro_image(remote_image_name) if remote_image_name else parser.convert_to_remote_in_neuro_registry(local_img) ) @@ -83,11 +83,11 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: """ parser = ImageParser(cfg.username, cfg.registry_url) - remote_img = parser.parse_as_remote(image_name) + remote_img = parser.parse_as_neuro_image(image_name) local_img = ( - parser.parse_as_local(local_image_name) + parser.parse_as_docker_image(local_image_name) if local_image_name - else parser.convert_to_local(remote_img) + else parser.convert_to_docker_image(remote_img) ) log.info(f"Using remote image '{remote_img.as_url_str()}'") diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index 7b148e57f..5e6c397a2 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -16,6 +16,7 @@ Resources, Volume, ) +from neuromation.client.images import IMAGE_SCHEME from neuromation.strings.parse import to_megabytes_str from .defaults import ( @@ -196,11 +197,10 @@ async def submit( memory = to_megabytes_str(memory) image_parser = ImageParser(cfg.username, cfg.registry_url) - try: - parsed_image = image_parser.parse_as_remote(image) - except ValueError: - parsed_image = image_parser.parse_as_local(image) - + if image_parser.is_in_neuro_registry(image): + parsed_image = image_parser.parse_as_neuro_image(image) + else: + parsed_image = image_parser.parse_as_docker_image(image) # TODO (ajuszkowski 01-Feb-19) process --quiet globally to set up logger+click if not quiet: log.info(f"Using image '{parsed_image.as_url_str()}'") diff --git a/python/neuromation/client/images.py b/python/neuromation/client/images.py index 9056474ef..8a19223cc 100644 --- a/python/neuromation/client/images.py +++ b/python/neuromation/client/images.py @@ -24,8 +24,9 @@ IMAGE_SCHEME = "image" -# TODO: maybe move to cli/utqils as image_utils @dataclass(frozen=True) +# TODO (ajuszkowski, 20-feb-2019): rename this class: docker-images refer to both local +# images and images in docker hub, and neuro-images refer to an image in neuro registry class DockerImage: name: str tag: str = "latest" diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 43d71659a..7aac96ebc 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -12,18 +12,22 @@ def __init__(self, default_user: str, registry_url: str): self._default_user = default_user self._registry = self._get_registry_hostname(registry_url) - def parse_as_local(self, image: str) -> DockerImage: + def parse_as_docker_image(self, image: str) -> DockerImage: try: - return self._parse_local(image) + return self._parse_as_docker_image(image) except ValueError as e: raise ValueError(f"Invalid local image '{image}': {e}") from e - def parse_as_remote(self, image: str) -> DockerImage: + def parse_as_neuro_image(self, image: str) -> DockerImage: try: - return self._parse_remote(image) + return self._parse_as_neuro_image(image) except ValueError as e: raise ValueError(f"Invalid remote image '{image}': {e}") from e + def is_in_neuro_registry(self, image: str) -> bool: + # not use URL here because URL("ubuntu:v1") is parsed as scheme=ubuntu path=v1 + return image.startswith(f"{IMAGE_SCHEME}:") + def convert_to_remote_in_neuro_registry(self, image: DockerImage) -> DockerImage: return DockerImage( name=image.name, @@ -32,15 +36,14 @@ def convert_to_remote_in_neuro_registry(self, image: DockerImage) -> DockerImage registry=self._registry, ) - def convert_to_local(self, image: DockerImage) -> DockerImage: + def convert_to_docker_image(self, image: DockerImage) -> DockerImage: return DockerImage(name=image.name, tag=image.tag) - def _parse_local(self, image: str) -> DockerImage: + def _parse_as_docker_image(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") - # This is hack because URL("ubuntu:v1") is parsed as scheme=ubuntu path=v1 - if image.startswith(f"{IMAGE_SCHEME}://"): + if self.is_in_neuro_registry(image): raise ValueError( f"scheme '{IMAGE_SCHEME}://' is not allowed for local images" ) @@ -49,7 +52,7 @@ def _parse_local(self, image: str) -> DockerImage: return DockerImage(name=name, tag=tag) - def _parse_remote(self, image: str) -> DockerImage: + def _parse_as_neuro_image(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index 48e0f26b1..bf8621fc1 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -111,363 +111,349 @@ def test__check_allowed_uri_elements__with_port(self): # public method: parse_local - def test_parse_local_empty_fail(self): + def test_parse_as_docker_image_empty_fail(self): image = "" with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_as_local(image) + self.parser.parse_as_docker_image(image) - def test_parse_local_none_fail(self): + def test_parse_as_docker_image_none_fail(self): image = None with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_as_local(image) + self.parser.parse_as_docker_image(image) - def test_parse_local_with_image_scheme_fail(self): + def test_parse_as_docker_image_with_image_scheme_fail(self): image = "image://ubuntu" msg = "scheme 'image://' is not allowed for local images" with pytest.raises(ValueError, match=msg): - self.parser.parse_as_local(image) + self.parser.parse_as_docker_image(image) - def test_parse_local_with_other_scheme_ok(self): + def test_parse_as_docker_image_with_other_scheme_ok(self): image = "http://ubuntu" - parsed = self.parser.parse_as_local(image) + parsed = self.parser.parse_as_docker_image(image) # instead of parser, the docker client will fail assert parsed == DockerImage(name="http", tag="//ubuntu") - def test_parse_local_no_tag(self): + def test_parse_as_docker_image_no_tag(self): image = "ubuntu" - parsed = self.parser.parse_as_local(image) + parsed = self.parser.parse_as_docker_image(image) assert parsed == DockerImage(name="ubuntu", tag="latest") - def test_parse_local_with_tag(self): + def test_parse_as_docker_image_with_tag(self): image = "ubuntu:v10.04" - parsed = self.parser.parse_as_local(image) + parsed = self.parser.parse_as_docker_image(image) assert parsed == DockerImage(name="ubuntu", tag="v10.04") - def test_parse_local_2_tag_fail(self): + def test_parse_as_docker_image_2_tag_fail(self): image = "ubuntu:v10.04:LTS" msg = "cannot parse image name 'ubuntu:v10.04:LTS': too many tags" with pytest.raises(ValueError, match=msg): - self.parser.parse_as_local(image) + self.parser.parse_as_docker_image(image) # public method: parse_remote @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_empty_fail(self, require_scheme): + def test_parse_as_neuro_image_empty_fail(self, require_scheme): image = "" with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_as_remote(image, require_scheme=require_scheme) + self.parser.parse_as_neuro_image(image) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_none_fail(self, require_scheme): + def test_parse_as_neuro_image_none_fail(self, require_scheme): image = None with pytest.raises(ValueError, match="empty image name"): - self.parser.parse_as_remote(image, require_scheme=require_scheme) + self.parser.parse_as_neuro_image(image) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_with_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_with_tag(self, require_scheme): image = "image://bob/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_with_user_with_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_with_tag_2( + self, require_scheme + ): image = "image://bob/library/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_with_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_no_tag(self, require_scheme): image = "image://bob/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_with_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_no_tag_2(self, require_scheme): image = "image://bob/library/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_no_slash_no_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_no_tag( + self, require_scheme + ): image = "image:ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_no_slash_no_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_no_tag_2( + self, require_scheme + ): image = "image:library/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_no_slash_no_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_with_tag( + self, require_scheme + ): image = "image:ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_no_slash_no_user_with_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_with_tag_2( + self, require_scheme + ): image = "image:library/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_1_slash_no_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_no_tag( + self, require_scheme + ): image = "image:/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_1_slash_no_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_no_tag_2( + self, require_scheme + ): image = "image:/library/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_1_slash_no_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_with_tag( + self, require_scheme + ): image = "image:/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_1_slash_no_user_with_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_with_tag_2( + self, require_scheme + ): image = "image:/library/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_2_slash_user_no_tag_fail(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_2_slash_user_no_tag_fail( + self, require_scheme + ): image = "image://ubuntu" with pytest.raises(ValueError, match="no image name specified"): - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_2_slash_user_with_tag_fail(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_2_slash_user_with_tag_fail( + self, require_scheme + ): image = "image://ubuntu:v10.04" with pytest.raises(ValueError, match="port can't be converted to integer"): - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_3_slash_no_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag( + self, require_scheme + ): image = "image:///ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_3_slash_no_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag_2( + self, require_scheme + ): image = "image:///library/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_3_slash_no_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_with_tag( + self, require_scheme + ): image = "image:///ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_3_slash_no_user_with_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_with_tag_2( + self, require_scheme + ): image = "image:///library/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_4_slash_no_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag( + self, require_scheme + ): image = "image:////ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_4_slash_no_user_with_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag_2( + self, require_scheme + ): image = "image:////library/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag_2( + self, require_scheme + ): image = "image:////ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_4_slash_no_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag_2( + self, require_scheme + ): image = "image:////library/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_tilde_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_no_tag(self, require_scheme): image = "image://~/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_tilde_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_no_tag_2(self, require_scheme): image = "image://~/library/ubuntu" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_tilde_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag(self, require_scheme): image = "image://~/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_remote_with_scheme_tilde_user_with_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag_2( + self, require_scheme + ): image = "image://~/library/ubuntu:v10.04" - parsed = self.parser.parse_as_remote(image, require_scheme=require_scheme) + parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - def test_parse_remote_no_scheme_no_slash_no_tag(self): + def test_parse_as_neuro_image_no_scheme_no_slash_no_tag_fail(self): image = "ubuntu" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) + parsed = self.parser.parse_as_neuro_image(image) - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="ubuntu", tag="latest", owner=None, registry=None - ) - - def test_parse_remote_no_scheme_no_slash_with_tag(self): + def test_parse_as_neuro_image_no_scheme_no_slash_with_tag_fail(self): image = "ubuntu:v10.04" - msg = "scheme 'image://' expected, found: 'ubuntu://'" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) + parsed = self.parser.parse_as_neuro_image(image) - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="ubuntu", tag="v10.04", owner=None, registry=None - ) - - def test_parse_remote_no_scheme_1_slash_no_tag(self): + def test_parse_as_neuro_image_no_scheme_1_slash_no_tag_fail(self): image = "library/ubuntu" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) + parsed = self.parser.parse_as_neuro_image(image) - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="library/ubuntu", tag="latest", owner=None, registry=None - ) - - def test_parse_remote_no_scheme_1_slash_with_tag(self): + def test_parse_as_neuro_image_no_scheme_1_slash_with_tag_fail(self): image = "library/ubuntu:v10.04" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) + parsed = self.parser.parse_as_neuro_image(image) - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="library/ubuntu", tag="v10.04", owner=None, registry=None - ) - - def test_parse_remote_no_scheme_2_slash_no_tag(self): + def test_parse_as_neuro_image_no_scheme_2_slash_no_tag_fail(self): image = "docker.io/library/ubuntu" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) - - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="docker.io/library/ubuntu", tag="latest", owner=None, registry=None - ) + parsed = self.parser.parse_as_neuro_image(image) - def test_parse_remote_no_scheme_2_slash_with_tag(self): + def test_parse_as_neuro_image_no_scheme_2_slash_with_tag_fail(self): image = "docker.io/library/ubuntu:v10.04" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) - - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="docker.io/library/ubuntu", tag="v10.04", owner=None, registry=None - ) + parsed = self.parser.parse_as_neuro_image(image) - def test_parse_remote_no_scheme_3_slash_no_tag(self): + def test_parse_as_neuro_image_no_scheme_3_slash_no_tag_fail(self): image = "something/docker.io/library/ubuntu" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) + parsed = self.parser.parse_as_neuro_image(image) - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="something/docker.io/library/ubuntu", - tag="latest", - owner=None, - registry=None, - ) - - def test_parse_remote_no_scheme_3_slash_with_tag(self): + def test_parse_as_neuro_image_no_scheme_3_slash_with_tag_fail(self): image = "something/docker.io/library/ubuntu:v10.04" - msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_remote(image, require_scheme=True) - - parsed = self.parser.parse_as_remote(image, require_scheme=False) - assert parsed == DockerImage( - name="something/docker.io/library/ubuntu", - tag="v10.04", - owner=None, - registry=None, - ) + parsed = self.parser.parse_as_neuro_image(image) @pytest.mark.usefixtures("patch_docker_host") @@ -485,7 +471,7 @@ def spinner(self) -> SpinnerBase: ), ) async def test_unavailable_docker(self, patched_init, token, spinner): - image = self.parser.parse_as_remote(f"image://bob/image:bananas") + image = self.parser.parse_as_neuro_image(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError, match=r"Docker engine is not available.+"): await client.images.pull(image, image, spinner) @@ -494,7 +480,7 @@ async def test_unavailable_docker(self, patched_init, token, spinner): "aiodocker.Docker.__init__", side_effect=ValueError("something went wrong") ) async def test_unknown_docker_error(self, patched_init, token, spinner): - image = self.parser.parse_as_remote(f"image://bob/image:bananas") + image = self.parser.parse_as_neuro_image(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"something went wrong"): await client.images.pull(image, image, spinner) @@ -504,7 +490,7 @@ async def test_push_non_existent_image(self, patched_tag, token, spinner): patched_tag.side_effect = DockerError( STATUS_NOT_FOUND, {"message": "Mocked error"} ) - image = self.parser.parse_as_remote(f"image://bob/image:bananas-no-more") + image = self.parser.parse_as_neuro_image(f"image://bob/image:bananas-no-more") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(ValueError, match=r"not found"): await client.images.push(image, image, spinner) @@ -518,7 +504,7 @@ async def test_push_image_to_foreign_repo( patched_push.side_effect = DockerError( STATUS_FORBIDDEN, {"message": "Mocked error"} ) - image = self.parser.parse_as_remote(f"image://bob/image:bananas-no-more") + image = self.parser.parse_as_neuro_image(f"image://bob/image:bananas-no-more") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): await client.images.push(image, image, spinner) @@ -533,7 +519,9 @@ async def error_generator(): patched_tag.return_value = True patched_push.return_value = error_generator() - image = self.parser.parse_as_remote(f"image://bob/image:bananas-wrong-food") + image = self.parser.parse_as_neuro_image( + f"image://bob/image:bananas-wrong-food" + ) async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: await client.images.push(image, image, spinner) @@ -548,7 +536,7 @@ async def message_generator(): patched_tag.return_value = True patched_push.return_value = message_generator() - image = self.parser.parse_as_remote(f"image://bob/image:bananas-is-here") + image = self.parser.parse_as_neuro_image(f"image://bob/image:bananas-is-here") async with Client(URL("https://api.localhost.localdomain"), token) as client: result = await client.images.push(image, image, spinner) assert result == image @@ -559,7 +547,9 @@ async def test_pull_non_existent_image(self, patched_pull, token, spinner): STATUS_NOT_FOUND, {"message": "Mocked error"} ) async with Client(URL("https://api.localhost.localdomain"), token) as client: - image = self.parser.parse_as_remote(f"image://bob/image:no-bananas-here") + image = self.parser.parse_as_neuro_image( + f"image://bob/image:no-bananas-here" + ) with pytest.raises(ValueError, match=r"not found"): await client.images.pull(image, image, spinner) @@ -568,7 +558,7 @@ async def test_pull_image_from_foreign_repo(self, patched_pull, token, spinner): patched_pull.side_effect = DockerError( STATUS_FORBIDDEN, {"message": "Mocked error"} ) - image = self.parser.parse_as_remote(f"image://bob/image:not-your-bananas") + image = self.parser.parse_as_neuro_image(f"image://bob/image:not-your-bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(AuthorizationError): await client.images.pull(image, image, spinner) @@ -579,7 +569,7 @@ async def error_generator(): yield {"error": True, "errorDetail": {"message": "Mocked message"}} patched_pull.return_value = error_generator() - image = self.parser.parse_as_remote(f"image://bob/image:nuts-here") + image = self.parser.parse_as_neuro_image(f"image://bob/image:nuts-here") async with Client(URL("https://api.localhost.localdomain"), token) as client: with pytest.raises(DockerError) as exc_info: await client.images.pull(image, image, spinner) @@ -594,7 +584,7 @@ async def message_generator(): patched_tag.return_value = True patched_pull.return_value = message_generator() - image = self.parser.parse_as_remote(f"image://bob/image:bananas") + image = self.parser.parse_as_neuro_image(f"image://bob/image:bananas") async with Client(URL("https://api.localhost.localdomain"), token) as client: result = await client.images.pull(image, image, spinner) assert result == image From 9333f968f2bba1e9277dc45287bc89414e9e5df5 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 20:20:12 +0300 Subject: [PATCH 11/28] set up e2e test --- python/tests/e2e/test_e2e_images.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/python/tests/e2e/test_e2e_images.py b/python/tests/e2e/test_e2e_images.py index eee943daf..1bccfc629 100644 --- a/python/tests/e2e/test_e2e_images.py +++ b/python/tests/e2e/test_e2e_images.py @@ -68,7 +68,7 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): pulled_image = f"{image}-pull" # Pull image as with another tag - captured = run_cli(["image", "pull", str(image_url), pulled_image]) + captured = run_cli(["image", "pull", f"image:{image}", pulled_image]) # stderr has "Used image ..." lines # assert not captured.err assert pulled_image == captured.out.strip() @@ -77,16 +77,11 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): # Execute image and check result config = ConfigFactory.load() - registry_url = URL(config.registry_url) - path = image_url.path - image_with_repo = f'{registry_url.host}/{image_url.host}/{path.lstrip("/")}' captured = run_cli( [ "job", "submit", - image_with_repo, - "--memory", - "100M", + f"image:{image}", "-g", "0", "-q", From 68d57fe5633a6f91cc9b5ffd2a0b866a2d0315bf Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 20:24:41 +0300 Subject: [PATCH 12/28] lint --- python/neuromation/cli/job.py | 1 - python/tests/client/test_images.py | 22 +++++++++++----------- python/tests/e2e/test_e2e_images.py | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index 5e6c397a2..30251954f 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -16,7 +16,6 @@ Resources, Volume, ) -from neuromation.client.images import IMAGE_SCHEME from neuromation.strings.parse import to_megabytes_str from .defaults import ( diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index bf8621fc1..7e323f359 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -283,7 +283,7 @@ def test_parse_as_neuro_image_with_scheme_2_slash_user_no_tag_fail( ): image = "image://ubuntu" with pytest.raises(ValueError, match="no image name specified"): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_as_neuro_image_with_scheme_2_slash_user_with_tag_fail( @@ -291,7 +291,7 @@ def test_parse_as_neuro_image_with_scheme_2_slash_user_with_tag_fail( ): image = "image://ubuntu:v10.04" with pytest.raises(ValueError, match="port can't be converted to integer"): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) @pytest.mark.parametrize("require_scheme", [True, False]) def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag( @@ -354,7 +354,7 @@ def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag_2( ) @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag_2( + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag( self, require_scheme ): image = "image:////ubuntu" @@ -411,49 +411,49 @@ def test_parse_as_neuro_image_no_scheme_no_slash_no_tag_fail(self): image = "ubuntu" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_no_slash_with_tag_fail(self): image = "ubuntu:v10.04" msg = "scheme 'image://' expected, found: 'ubuntu://'" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_1_slash_no_tag_fail(self): image = "library/ubuntu" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_1_slash_with_tag_fail(self): image = "library/ubuntu:v10.04" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_2_slash_no_tag_fail(self): image = "docker.io/library/ubuntu" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_2_slash_with_tag_fail(self): image = "docker.io/library/ubuntu:v10.04" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_3_slash_no_tag_fail(self): image = "something/docker.io/library/ubuntu" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_3_slash_with_tag_fail(self): image = "something/docker.io/library/ubuntu:v10.04" msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): - parsed = self.parser.parse_as_neuro_image(image) + self.parser.parse_as_neuro_image(image) @pytest.mark.usefixtures("patch_docker_host") diff --git a/python/tests/e2e/test_e2e_images.py b/python/tests/e2e/test_e2e_images.py index 1bccfc629..c392d908c 100644 --- a/python/tests/e2e/test_e2e_images.py +++ b/python/tests/e2e/test_e2e_images.py @@ -76,7 +76,7 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): loop.run_until_complete(docker.images.delete(pulled_image, force=True)) # Execute image and check result - config = ConfigFactory.load() + ConfigFactory.load() captured = run_cli( [ "job", From 06396c4e6ab9c02bbbb86c901e3465d49980b223 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 20:41:41 +0300 Subject: [PATCH 13/28] cleanup a bit --- python/neuromation/cli/image.py | 2 +- python/neuromation/client/parsing_utils.py | 20 ++++++-------------- python/tests/client/test_images.py | 4 ++-- python/tests/e2e/test_e2e_images.py | 2 +- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 6c83268ea..7fbed8d55 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -46,7 +46,7 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: remote_img = ( parser.parse_as_neuro_image(remote_image_name) if remote_image_name - else parser.convert_to_remote_in_neuro_registry(local_img) + else parser.convert_to_neuro_image(local_img) ) # TODO: check out all todos by ajuszkowski in this PR! diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 7aac96ebc..5cc2854a9 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -16,7 +16,7 @@ def parse_as_docker_image(self, image: str) -> DockerImage: try: return self._parse_as_docker_image(image) except ValueError as e: - raise ValueError(f"Invalid local image '{image}': {e}") from e + raise ValueError(f"Invalid docker image '{image}': {e}") from e def parse_as_neuro_image(self, image: str) -> DockerImage: try: @@ -28,7 +28,7 @@ def is_in_neuro_registry(self, image: str) -> bool: # not use URL here because URL("ubuntu:v1") is parsed as scheme=ubuntu path=v1 return image.startswith(f"{IMAGE_SCHEME}:") - def convert_to_remote_in_neuro_registry(self, image: DockerImage) -> DockerImage: + def convert_to_neuro_image(self, image: DockerImage) -> DockerImage: return DockerImage( name=image.name, tag=image.tag, @@ -42,34 +42,26 @@ def convert_to_docker_image(self, image: DockerImage) -> DockerImage: def _parse_as_docker_image(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") - if self.is_in_neuro_registry(image): raise ValueError( - f"scheme '{IMAGE_SCHEME}://' is not allowed for local images" + f"scheme '{IMAGE_SCHEME}://' is not allowed for docker images" ) - name, tag = self._split_image_name(image) - return DockerImage(name=name, tag=tag) def _parse_as_neuro_image(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") + if not self.is_in_neuro_registry(image): + raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") url = URL(image) - + assert url.scheme == IMAGE_SCHEME self._check_allowed_uri_elements(url) - if not url.scheme: - raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") - if url.scheme != IMAGE_SCHEME: - scheme = f"{url.scheme}://" if url.scheme else "" - raise ValueError(f"scheme '{IMAGE_SCHEME}://' expected, found: '{scheme}'") - registry = self._registry owner = self._default_user if not url.host or url.host == "~" else url.host name, tag = self._split_image_name(url.path.lstrip("/")) - return DockerImage(name=name, tag=tag, registry=registry, owner=owner) def _split_image_name(self, image: str) -> Tuple[str, str]: diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index 7e323f359..1a4acd3ed 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -123,7 +123,7 @@ def test_parse_as_docker_image_none_fail(self): def test_parse_as_docker_image_with_image_scheme_fail(self): image = "image://ubuntu" - msg = "scheme 'image://' is not allowed for local images" + msg = "scheme 'image://' is not allowed for docker images" with pytest.raises(ValueError, match=msg): self.parser.parse_as_docker_image(image) @@ -415,7 +415,7 @@ def test_parse_as_neuro_image_no_scheme_no_slash_no_tag_fail(self): def test_parse_as_neuro_image_no_scheme_no_slash_with_tag_fail(self): image = "ubuntu:v10.04" - msg = "scheme 'image://' expected, found: 'ubuntu://'" + msg = "scheme 'image://' is required" with pytest.raises(ValueError, match=msg): self.parser.parse_as_neuro_image(image) diff --git a/python/tests/e2e/test_e2e_images.py b/python/tests/e2e/test_e2e_images.py index c392d908c..f94d9b8e2 100644 --- a/python/tests/e2e/test_e2e_images.py +++ b/python/tests/e2e/test_e2e_images.py @@ -68,7 +68,7 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): pulled_image = f"{image}-pull" # Pull image as with another tag - captured = run_cli(["image", "pull", f"image:{image}", pulled_image]) + captured = run_cli(["image", "pull", f"image://~/{image}", pulled_image]) # stderr has "Used image ..." lines # assert not captured.err assert pulled_image == captured.out.strip() From 68bad06e3831b49949f797f37b1f6c95cd4b0361 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 20:44:40 +0300 Subject: [PATCH 14/28] remove todo --- python/neuromation/cli/image.py | 1 - python/neuromation/cli/model.py | 1 - 2 files changed, 2 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 7fbed8d55..717765ac7 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -49,7 +49,6 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: else parser.convert_to_neuro_image(local_img) ) - # TODO: check out all todos by ajuszkowski in this PR! log.info(f"Using local image '{local_img.as_local_str()}'") log.info(f"Using remote image '{remote_img.as_url_str()}'") log.debug(f"LOCAL: '{local_img}'") diff --git a/python/neuromation/cli/model.py b/python/neuromation/cli/model.py index a06bfc9c8..021ad0bbd 100644 --- a/python/neuromation/cli/model.py +++ b/python/neuromation/cli/model.py @@ -137,7 +137,6 @@ async def train( log.debug(f'cmdline="{cmdline}"') if not quiet: - # TODO (ajuszkowski 01-Feb-19) normalize image name to URI (issue 452) log.info(f"Using image '{image}'") log.info(f"Using dataset '{dataset_url}'") log.info(f"Using weights '{resultset_url}'") From 729351f08b95ba4e3ea282d75be73a9bb3ebc93a Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 20:51:10 +0300 Subject: [PATCH 15/28] final cleanup --- python/neuromation/client/parsing_utils.py | 4 +++- python/tests/e2e/test_e2e_images.py | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 5cc2854a9..8175efebb 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -53,7 +53,9 @@ def _parse_as_neuro_image(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") if not self.is_in_neuro_registry(image): - raise ValueError(f"scheme '{IMAGE_SCHEME}://' is required") + raise ValueError( + f"scheme '{IMAGE_SCHEME}://' is required for remote images" + ) url = URL(image) assert url.scheme == IMAGE_SCHEME diff --git a/python/tests/e2e/test_e2e_images.py b/python/tests/e2e/test_e2e_images.py index f94d9b8e2..69abee645 100644 --- a/python/tests/e2e/test_e2e_images.py +++ b/python/tests/e2e/test_e2e_images.py @@ -5,7 +5,6 @@ import pytest from yarl import URL -from neuromation.cli.rc import ConfigFactory from neuromation.client import JobStatus from tests.e2e.utils import attempt @@ -76,7 +75,6 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): loop.run_until_complete(docker.images.delete(pulled_image, force=True)) # Execute image and check result - ConfigFactory.load() captured = run_cli( [ "job", From 45859e1d633ea943513deee685b5440d6397c160 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Wed, 20 Feb 2019 21:02:19 +0300 Subject: [PATCH 16/28] fix tag in as_repo_str --- python/neuromation/client/images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/neuromation/client/images.py b/python/neuromation/client/images.py index 8a19223cc..17d5f70d5 100644 --- a/python/neuromation/client/images.py +++ b/python/neuromation/client/images.py @@ -43,7 +43,7 @@ def as_url_str(self) -> str: def as_repo_str(self) -> str: # TODO (ajuszkowski, 11-Feb-2019) should be host:port (see URL.explicit_port) pre = f"{self.registry}/{self.owner}/" if self.is_in_neuro_registry() else "" - return pre + self.name + return pre + self.as_local_str() def as_local_str(self) -> str: return f"{self.name}:{self.tag}" From e0f64e7d284f87253183158783841dc25aab8f7d Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Thu, 21 Feb 2019 18:09:01 +0300 Subject: [PATCH 17/28] review: cleanup tests file --- python/tests/client/test_images.py | 156 ++++++++--------------------- 1 file changed, 40 insertions(+), 116 deletions(-) diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index 1a4acd3ed..b2587edb3 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -123,8 +123,9 @@ def test_parse_as_docker_image_none_fail(self): def test_parse_as_docker_image_with_image_scheme_fail(self): image = "image://ubuntu" - msg = "scheme 'image://' is not allowed for docker images" - with pytest.raises(ValueError, match=msg): + with pytest.raises( + ValueError, match="scheme 'image://' is not allowed for docker images" + ): self.parser.parse_as_docker_image(image) def test_parse_as_docker_image_with_other_scheme_ok(self): @@ -145,262 +146,193 @@ def test_parse_as_docker_image_with_tag(self): def test_parse_as_docker_image_2_tag_fail(self): image = "ubuntu:v10.04:LTS" - msg = "cannot parse image name 'ubuntu:v10.04:LTS': too many tags" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="too many tags"): self.parser.parse_as_docker_image(image) # public method: parse_remote - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_empty_fail(self, require_scheme): + def test_parse_as_neuro_image_empty_fail(self): image = "" with pytest.raises(ValueError, match="empty image name"): self.parser.parse_as_neuro_image(image) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_none_fail(self, require_scheme): + def test_parse_as_neuro_image_none_fail(self): image = None with pytest.raises(ValueError, match="empty image name"): self.parser.parse_as_neuro_image(image) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_with_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_with_tag(self): image = "image://bob/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_with_user_with_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_with_user_with_tag_2(self): image = "image://bob/library/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="bob", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_with_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_no_tag(self): image = "image://bob/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_with_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_with_user_no_tag_2(self): image = "image://bob/library/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="bob", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_no_slash_no_user_no_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_no_tag(self): image = "image:ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_no_slash_no_user_no_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_no_tag_2(self): image = "image:library/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_no_slash_no_user_with_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_with_tag(self): image = "image:ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_no_slash_no_user_with_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_no_slash_no_user_with_tag_2(self): image = "image:library/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_1_slash_no_user_no_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_no_tag(self): image = "image:/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_1_slash_no_user_no_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_no_tag_2(self): image = "image:/library/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_1_slash_no_user_with_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_with_tag(self): image = "image:/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_1_slash_no_user_with_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_1_slash_no_user_with_tag_2(self): image = "image:/library/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_2_slash_user_no_tag_fail( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_2_slash_user_no_tag_fail(self): image = "image://ubuntu" with pytest.raises(ValueError, match="no image name specified"): self.parser.parse_as_neuro_image(image) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_2_slash_user_with_tag_fail( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_2_slash_user_with_tag_fail(self): image = "image://ubuntu:v10.04" with pytest.raises(ValueError, match="port can't be converted to integer"): self.parser.parse_as_neuro_image(image) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag(self): image = "image:///ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_no_tag_2(self): image = "image:///library/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_3_slash_no_user_with_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_with_tag(self): image = "image:///ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_3_slash_no_user_with_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_3_slash_no_user_with_tag_2(self): image = "image:///library/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag(self): image = "image:////ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_with_tag_2(self): image = "image:////library/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag(self): image = "image:////ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_4_slash_no_user_no_tag_2(self): image = "image:////library/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_tilde_user_no_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_no_tag(self): image = "image://~/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_tilde_user_no_tag_2(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_no_tag_2(self): image = "image://~/library/ubuntu" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="library/ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag(self, require_scheme): + def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag(self): image = "image://~/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( name="ubuntu", tag="v10.04", owner="alice", registry="reg.neu.ro" ) - @pytest.mark.parametrize("require_scheme", [True, False]) - def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag_2( - self, require_scheme - ): + def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag_2(self): image = "image://~/library/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) assert parsed == DockerImage( @@ -409,50 +341,42 @@ def test_parse_as_neuro_image_with_scheme_tilde_user_with_tag_2( def test_parse_as_neuro_image_no_scheme_no_slash_no_tag_fail(self): image = "ubuntu" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_no_slash_with_tag_fail(self): image = "ubuntu:v10.04" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_1_slash_no_tag_fail(self): image = "library/ubuntu" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_1_slash_with_tag_fail(self): image = "library/ubuntu:v10.04" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_2_slash_no_tag_fail(self): image = "docker.io/library/ubuntu" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_2_slash_with_tag_fail(self): image = "docker.io/library/ubuntu:v10.04" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_3_slash_no_tag_fail(self): image = "something/docker.io/library/ubuntu" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) def test_parse_as_neuro_image_no_scheme_3_slash_with_tag_fail(self): image = "something/docker.io/library/ubuntu:v10.04" - msg = "scheme 'image://' is required" - with pytest.raises(ValueError, match=msg): + with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) From ee0c35e93fa47517a03117d41365544b1eb2396d Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Thu, 21 Feb 2019 18:09:49 +0300 Subject: [PATCH 18/28] review: rename ImageParser -> ImageNameParser --- python/neuromation/cli/image.py | 6 +++--- python/neuromation/cli/job.py | 4 ++-- python/neuromation/client/__init__.py | 4 ++-- python/neuromation/client/parsing_utils.py | 2 +- python/tests/client/test_images.py | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 717765ac7..1e9f51dce 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -3,7 +3,7 @@ import click -from neuromation.client import ImageParser +from neuromation.client import ImageNameParser from .command_spinner import SpinnerBase from .rc import Config @@ -41,7 +41,7 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: """ - parser = ImageParser(cfg.username, cfg.registry_url) + parser = ImageNameParser(cfg.username, cfg.registry_url) local_img = parser.parse_as_docker_image(image_name) remote_img = ( parser.parse_as_neuro_image(remote_image_name) @@ -81,7 +81,7 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: """ - parser = ImageParser(cfg.username, cfg.registry_url) + parser = ImageNameParser(cfg.username, cfg.registry_url) remote_img = parser.parse_as_neuro_image(image_name) local_img = ( parser.parse_as_docker_image(local_image_name) diff --git a/python/neuromation/cli/job.py b/python/neuromation/cli/job.py index 30251954f..a222f9681 100644 --- a/python/neuromation/cli/job.py +++ b/python/neuromation/cli/job.py @@ -10,7 +10,7 @@ from neuromation.client import ( Image, - ImageParser, + ImageNameParser, JobStatus, NetworkPortForwarding, Resources, @@ -195,7 +195,7 @@ async def submit( memory = to_megabytes_str(memory) - image_parser = ImageParser(cfg.username, cfg.registry_url) + image_parser = ImageNameParser(cfg.username, cfg.registry_url) if image_parser.is_in_neuro_registry(image): parsed_image = image_parser.parse_as_neuro_image(image) else: diff --git a/python/neuromation/client/__init__.py b/python/neuromation/client/__init__.py index 68f35c942..63b9159db 100644 --- a/python/neuromation/client/__init__.py +++ b/python/neuromation/client/__init__.py @@ -32,11 +32,11 @@ from .users import Action, Permission, Users from .images import Images from .config import Config -from .parsing_utils import ImageParser +from .parsing_utils import ImageNameParser __all__ = ( "Image", - "ImageParser", + "ImageNameParser", "JobDescription", "JobStatus", "JobStatusHistory", diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 8175efebb..72003be0f 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -5,7 +5,7 @@ from .images import IMAGE_SCHEME, DockerImage -class ImageParser: +class ImageNameParser: default_tag = "latest" def __init__(self, default_user: str, registry_url: str): diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index b2587edb3..12deca316 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -7,7 +7,7 @@ from yarl import URL from neuromation.cli.command_spinner import SpinnerBase -from neuromation.client import AuthorizationError, Client, ImageParser +from neuromation.client import AuthorizationError, Client, ImageNameParser from neuromation.client.images import ( STATUS_CUSTOM_ERROR, STATUS_FORBIDDEN, @@ -25,7 +25,7 @@ def patch_docker_host(): class TestImageParser: - parser = ImageParser(default_user="alice", registry_url="https://reg.neu.ro") + parser = ImageNameParser(default_user="alice", registry_url="https://reg.neu.ro") @pytest.mark.parametrize( "registry_url", @@ -382,7 +382,7 @@ def test_parse_as_neuro_image_no_scheme_3_slash_with_tag_fail(self): @pytest.mark.usefixtures("patch_docker_host") class TestImages: - parser = ImageParser(default_user="bob", registry_url="https://reg.neu.ro") + parser = ImageNameParser(default_user="bob", registry_url="https://reg.neu.ro") @pytest.fixture() def spinner(self) -> SpinnerBase: From 39c7cd19563c4bf3aa7ac78ea327f043e062706d Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Thu, 21 Feb 2019 18:11:12 +0300 Subject: [PATCH 19/28] review: drop out ternary operators --- python/neuromation/cli/image.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index 1e9f51dce..c39b8a6b3 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -43,11 +43,10 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: parser = ImageNameParser(cfg.username, cfg.registry_url) local_img = parser.parse_as_docker_image(image_name) - remote_img = ( - parser.parse_as_neuro_image(remote_image_name) - if remote_image_name - else parser.convert_to_neuro_image(local_img) - ) + if remote_image_name: + remote_img = parser.parse_as_neuro_image(remote_image_name) + else: + remote_img = parser.convert_to_neuro_image(local_img) log.info(f"Using local image '{local_img.as_local_str()}'") log.info(f"Using remote image '{remote_img.as_url_str()}'") @@ -83,11 +82,10 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: parser = ImageNameParser(cfg.username, cfg.registry_url) remote_img = parser.parse_as_neuro_image(image_name) - local_img = ( - parser.parse_as_docker_image(local_image_name) - if local_image_name - else parser.convert_to_docker_image(remote_img) - ) + if local_image_name: + local_img = parser.parse_as_docker_image(local_image_name) + else: + local_img = parser.convert_to_docker_image(remote_img) log.info(f"Using remote image '{remote_img.as_url_str()}'") log.info(f"Using local image '{local_img.as_local_str()}'") From def4d64c6fe58c862228ea9d6694af515c07fa37 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Mon, 25 Feb 2019 19:50:37 +0300 Subject: [PATCH 20/28] review: add e2e --- python/tests/e2e/test_e2e_images.py | 74 +++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/python/tests/e2e/test_e2e_images.py b/python/tests/e2e/test_e2e_images.py index 69abee645..5eb4c7a2d 100644 --- a/python/tests/e2e/test_e2e_images.py +++ b/python/tests/e2e/test_e2e_images.py @@ -12,6 +12,16 @@ TEST_IMAGE_NAME = "e2e-banana-image" +def parse_docker_ls_output(docker_ls_output): + return set( + repo_tag + for info in docker_ls_output + if info["RepoTags"] is not None + for repo_tag in info["RepoTags"] + if repo_tag + ) + + @pytest.fixture() async def docker(loop): client = aiodocker.Docker() @@ -45,15 +55,16 @@ async def image(loop, docker, tag): @pytest.mark.e2e def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): + # Let`s push image captured = run_cli(["image", "push", image]) # stderr has "Used image ..." lines # assert not captured.err - image_url = URL(captured.out.strip()) - assert image_url.scheme == "image" - assert image_url.path.lstrip("/") == image + image_full_str = f"image://{helper._config.username}/{image}" + assert captured.out.strip() == image_full_str + image_url = URL(image_full_str) # Check if image available on registry captured = run_cli(["image", "ls"]) @@ -64,22 +75,29 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): image_url_without_tag = image_url.with_path(image_url.path.replace(f":{tag}", "")) assert image_url_without_tag in image_urls - pulled_image = f"{image}-pull" + # delete local + loop.run_until_complete(docker.images.delete(image, force=True)) + docker_ls_output = loop.run_until_complete(docker.images.list()) + local_images = parse_docker_ls_output(docker_ls_output) + assert image not in local_images # Pull image as with another tag - captured = run_cli(["image", "pull", f"image://~/{image}", pulled_image]) + captured = run_cli(["image", "pull", f"image://~/{image}"]) # stderr has "Used image ..." lines # assert not captured.err - assert pulled_image == captured.out.strip() - # Check if image exists and remove, all-in-one swiss knife - loop.run_until_complete(docker.images.delete(pulled_image, force=True)) + assert image == captured.out.strip() + + # check pulled locally, delete for cleanup + docker_ls_output = loop.run_until_complete(docker.images.list()) + local_images = parse_docker_ls_output(docker_ls_output) + assert image in local_images # Execute image and check result captured = run_cli( [ "job", "submit", - f"image:{image}", + str(image_url), "-g", "0", "-q", @@ -99,3 +117,41 @@ def check_job_output(): assert captured.out == tag check_job_output() + + +@pytest.mark.e2e +def test_images_push_with_specified_name(helper, run_cli, image, tag, loop, docker): + # Let`s push image + image_no_tag = image.replace(f":{tag}", "") + pushed_no_tag = f"{image_no_tag}-pushed" + pulled_no_tag = f"{image_no_tag}-pulled" + pulled = f"{pulled_no_tag}:{tag}" + + captured = run_cli(["image", "push", image, f"image://~/{pushed_no_tag}:{tag}"]) + # stderr has "Used image ..." lines + # assert not captured.err + image_pushed_full_str = f"image://{helper._config.username}/{pushed_no_tag}:{tag}" + assert captured.out.strip() == image_pushed_full_str + image_url_without_tag = image_pushed_full_str.replace(f":{tag}", "") + + # Check if image available on registry + captured = run_cli(["image", "ls"]) + image_urls = captured.out.splitlines() + assert image_url_without_tag in image_urls + + # check locally + docker_ls_output = loop.run_until_complete(docker.images.list()) + local_images = parse_docker_ls_output(docker_ls_output) + assert pulled not in local_images + + # Pull image as with another name + captured = run_cli(["image", "pull", f"image:{pushed_no_tag}:{tag}", pulled]) + # stderr has "Used image ..." lines + # assert not captured.err + assert pulled == captured.out.strip() + # check locally + docker_ls_output = loop.run_until_complete(docker.images.list()) + local_images = parse_docker_ls_output(docker_ls_output) + assert pulled in local_images + + loop.run_until_complete(docker.images.delete(pulled, force=True)) From 0489bd741c5678218b39483945f44001b3536f66 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Mon, 25 Feb 2019 19:54:19 +0300 Subject: [PATCH 21/28] review: click.echo instead of log.info --- python/neuromation/cli/image.py | 8 ++++---- python/tests/e2e/test_e2e_images.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/python/neuromation/cli/image.py b/python/neuromation/cli/image.py index c39b8a6b3..c56a7d9f0 100644 --- a/python/neuromation/cli/image.py +++ b/python/neuromation/cli/image.py @@ -48,8 +48,8 @@ async def push(cfg: Config, image_name: str, remote_image_name: str) -> None: else: remote_img = parser.convert_to_neuro_image(local_img) - log.info(f"Using local image '{local_img.as_local_str()}'") - log.info(f"Using remote image '{remote_img.as_url_str()}'") + click.echo(f"Using local image '{local_img.as_local_str()}'") + click.echo(f"Using remote image '{remote_img.as_url_str()}'") log.debug(f"LOCAL: '{local_img}'") log.debug(f"REMOTE: '{remote_img}'") @@ -87,8 +87,8 @@ async def pull(cfg: Config, image_name: str, local_image_name: str) -> None: else: local_img = parser.convert_to_docker_image(remote_img) - log.info(f"Using remote image '{remote_img.as_url_str()}'") - log.info(f"Using local image '{local_img.as_local_str()}'") + click.echo(f"Using remote image '{remote_img.as_url_str()}'") + click.echo(f"Using local image '{local_img.as_local_str()}'") log.debug(f"REMOTE: '{remote_img}'") log.debug(f"LOCAL: '{local_img}'") diff --git a/python/tests/e2e/test_e2e_images.py b/python/tests/e2e/test_e2e_images.py index 5eb4c7a2d..8b7f14a94 100644 --- a/python/tests/e2e/test_e2e_images.py +++ b/python/tests/e2e/test_e2e_images.py @@ -63,7 +63,7 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): # assert not captured.err image_full_str = f"image://{helper._config.username}/{image}" - assert captured.out.strip() == image_full_str + assert captured.out.endswith(image_full_str) image_url = URL(image_full_str) # Check if image available on registry @@ -85,7 +85,7 @@ def test_images_complete_lifecycle(helper, run_cli, image, tag, loop, docker): captured = run_cli(["image", "pull", f"image://~/{image}"]) # stderr has "Used image ..." lines # assert not captured.err - assert image == captured.out.strip() + assert captured.out.endswith(image) # check pulled locally, delete for cleanup docker_ls_output = loop.run_until_complete(docker.images.list()) @@ -131,7 +131,7 @@ def test_images_push_with_specified_name(helper, run_cli, image, tag, loop, dock # stderr has "Used image ..." lines # assert not captured.err image_pushed_full_str = f"image://{helper._config.username}/{pushed_no_tag}:{tag}" - assert captured.out.strip() == image_pushed_full_str + assert captured.out.endswith(image_pushed_full_str) image_url_without_tag = image_pushed_full_str.replace(f":{tag}", "") # Check if image available on registry @@ -148,7 +148,7 @@ def test_images_push_with_specified_name(helper, run_cli, image, tag, loop, dock captured = run_cli(["image", "pull", f"image:{pushed_no_tag}:{tag}", pulled]) # stderr has "Used image ..." lines # assert not captured.err - assert pulled == captured.out.strip() + assert captured.out.endswith(pulled) # check locally docker_ls_output = loop.run_until_complete(docker.images.list()) local_images = parse_docker_ls_output(docker_ls_output) From 43c20b55c4ff3acf646521cb34603b40dc606760 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Mon, 25 Feb 2019 20:01:50 +0300 Subject: [PATCH 22/28] review: add unit on DockerImage --- python/tests/client/test_images.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index 12deca316..2ea79462c 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -379,6 +379,16 @@ def test_parse_as_neuro_image_no_scheme_3_slash_with_tag_fail(self): with pytest.raises(ValueError, match="scheme 'image://' is required"): self.parser.parse_as_neuro_image(image) + def test_convert_to_docker_image(self): + neuro_image = DockerImage(name="ubuntu", tag="latest", owner="artem", registry="reg.com") + docker_image = self.parser.convert_to_docker_image(neuro_image) + assert docker_image == DockerImage(name="ubuntu", tag="latest", owner=None, registry=None) + + def test_convert_to_neuro_image(self): + docker_image = DockerImage(name="ubuntu", tag="latest") + neuro_image = self.parser.convert_to_neuro_image(docker_image) + assert neuro_image == DockerImage(name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro") + @pytest.mark.usefixtures("patch_docker_host") class TestImages: From fcc25c95216e027faae453c1dfe6ef0696b7d1fb Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Tue, 26 Feb 2019 12:55:31 +0300 Subject: [PATCH 23/28] review: remove unnecessary try-catch --- python/neuromation/client/parsing_utils.py | 5 +---- python/tests/client/test_images.py | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index 72003be0f..a4eb40eb9 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -77,10 +77,7 @@ def _split_image_name(self, image: str) -> Tuple[str, str]: return image, tag def _get_registry_hostname(self, registry_url: str) -> str: - try: - url = URL(registry_url) - except ValueError as e: - raise ValueError(f"Could not parse registry URL: {e}") + url = URL(registry_url) if not url.host: raise ValueError( f"Empty hostname in registry URL '{registry_url}': " diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index 2ea79462c..9410e3dd2 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -45,7 +45,7 @@ def test__get_registry_hostname(self, registry_url): "registry_url", ["", "reg.neu.ro", "reg.neu.ro:5000", "https://", "https:///bla/bla"], ) - def test__get_registry_hostname_bad_registry_url(self, registry_url): + def test__get_registry_hostname__bad_url_empty_hostname(self, registry_url): with pytest.raises(ValueError, match="Empty hostname in registry URL"): self.parser._get_registry_hostname(registry_url) From d84ba0f7be9ad8731916890a09d8858a5a6ce8a7 Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Tue, 26 Feb 2019 12:58:57 +0300 Subject: [PATCH 24/28] improve assert message --- python/neuromation/client/parsing_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index a4eb40eb9..c4d9c693c 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -58,7 +58,7 @@ def _parse_as_neuro_image(self, image: str) -> DockerImage: ) url = URL(image) - assert url.scheme == IMAGE_SCHEME + assert url.scheme == IMAGE_SCHEME, f"invalid image scheme: '{url.scheme}'" self._check_allowed_uri_elements(url) registry = self._registry From 435676221f160cdf3092896e69571b9b5556740e Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Tue, 26 Feb 2019 13:25:02 +0300 Subject: [PATCH 25/28] review: wrong unit tests --- python/tests/client/test_images.py | 129 +++++++++++++++++------------ 1 file changed, 78 insertions(+), 51 deletions(-) diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index 9410e3dd2..cc166c0e1 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -61,55 +61,14 @@ def test__split_image_name_2_colon(self): with pytest.raises(ValueError, match="too many tags"): self.parser._split_image_name("ubuntu:v10.04:LTS") - @pytest.mark.parametrize( - "url", ["", "/", "image://", "image:///", "image://bob", "image://bob/"] - ) - def test__check_allowed_uri_elements__no_path(self, url): - with pytest.raises(ValueError, match="no image name specified"): - self.parser._check_allowed_uri_elements(URL(url)) - - @pytest.mark.parametrize( - "url", - [ - "image://bob/ubuntu:v10.04?key=value", - "image://bob/ubuntu?key=value", - "image:///ubuntu?key=value", - "image:ubuntu?key=value", - ], - ) - def test__check_allowed_uri_elements__with_query(self, url): - with pytest.raises(ValueError, match="query is not allowed"): - self.parser._check_allowed_uri_elements(URL(url)) + # public method: parse_local @pytest.mark.parametrize( - "url", - [ - "image://bob/ubuntu:v10.04#fragment", - "image://bob/ubuntu#fragment", - "image:///ubuntu#fragment", - "image:ubuntu#fragment", - ], + "url", ["image://", "image:///", "image://bob", "image://bob/"] ) - def test__check_allowed_uri_elements__with_fragment(self, url): - with pytest.raises(ValueError, match="fragment is not allowed"): - self.parser._check_allowed_uri_elements(URL(url)) - - def test__check_allowed_uri_elements__with_user(self): - url = "image://user@bob/ubuntu" - with pytest.raises(ValueError, match="user is not allowed"): - self.parser._check_allowed_uri_elements(URL(url)) - - def test__check_allowed_uri_elements__with_password(self): - url = "image://:password@bob/ubuntu" - with pytest.raises(ValueError, match="password is not allowed"): - self.parser._check_allowed_uri_elements(URL(url)) - - def test__check_allowed_uri_elements__with_port(self): - url = "image://bob:443/ubuntu" - with pytest.raises(ValueError, match="port is not allowed"): - self.parser._check_allowed_uri_elements(URL(url)) - - # public method: parse_local + def test_parse_as_neuro_image__no_image_name(self, url): + with pytest.raises(ValueError, match="no image name specified"): + self.parser.parse_as_neuro_image(url) def test_parse_as_docker_image_empty_fail(self): image = "" @@ -151,16 +110,78 @@ def test_parse_as_docker_image_2_tag_fail(self): # public method: parse_remote - def test_parse_as_neuro_image_empty_fail(self): + @pytest.mark.parametrize( + "url", + [ + "image://bob/ubuntu:v10.04?key=value", + "image://bob/ubuntu?key=value", + "image:///ubuntu?key=value", + "image:ubuntu?key=value", + ], + ) + def test_parse_as_neuro_image__with_query__fail(self, url): + with pytest.raises(ValueError, match="query is not allowed"): + self.parser.parse_as_neuro_image(url) + + @pytest.mark.parametrize( + "url", + [ + "image://bob/ubuntu:v10.04#fragment", + "image://bob/ubuntu#fragment", + "image:///ubuntu#fragment", + "image:ubuntu#fragment", + ], + ) + def test_parse_as_neuro_image__with_fragment__fail(self, url): + with pytest.raises(ValueError, match="fragment is not allowed"): + self.parser.parse_as_neuro_image(url) + + def test_parse_as_neuro_image__with_user__fail(self): + url = "image://user@bob/ubuntu" + with pytest.raises(ValueError, match="user is not allowed"): + self.parser.parse_as_neuro_image(url) + + def test_parse_as_neuro_image__with_password__fail(self): + url = "image://:password@bob/ubuntu" + with pytest.raises(ValueError, match="password is not allowed"): + self.parser.parse_as_neuro_image(url) + + def test_parse_as_neuro_image__with_port__fail(self): + url = "image://bob:443/ubuntu" + with pytest.raises(ValueError, match="port is not allowed"): + self.parser.parse_as_neuro_image(url) + + def test_parse_as_neuro_image_empty_fail__fail(self): image = "" with pytest.raises(ValueError, match="empty image name"): self.parser.parse_as_neuro_image(image) - def test_parse_as_neuro_image_none_fail(self): + def test_parse_as_neuro_image_none_fail__fail(self): image = None with pytest.raises(ValueError, match="empty image name"): self.parser.parse_as_neuro_image(image) + def test_parse_as_neuro_image_no_scheme_fail(self): + image = "ubuntu" + with pytest.raises( + ValueError, match="scheme 'image://' is required for remote images" + ): + self.parser.parse_as_neuro_image(image) + + def test_parse_as_neuro_image_invalid_scheme_1_fail(self): + image = "ubuntu:latest" + with pytest.raises( + ValueError, match="scheme 'image://' is required for remote images" + ): + self.parser.parse_as_neuro_image(image) + + def test_parse_as_neuro_image_invalid_scheme_2_fail(self): + image = "http://ubuntu" + with pytest.raises( + ValueError, match="scheme 'image://' is required for remote images" + ): + self.parser.parse_as_neuro_image(image) + def test_parse_as_neuro_image_with_scheme_with_user_with_tag(self): image = "image://bob/ubuntu:v10.04" parsed = self.parser.parse_as_neuro_image(image) @@ -380,14 +401,20 @@ def test_parse_as_neuro_image_no_scheme_3_slash_with_tag_fail(self): self.parser.parse_as_neuro_image(image) def test_convert_to_docker_image(self): - neuro_image = DockerImage(name="ubuntu", tag="latest", owner="artem", registry="reg.com") + neuro_image = DockerImage( + name="ubuntu", tag="latest", owner="artem", registry="reg.com" + ) docker_image = self.parser.convert_to_docker_image(neuro_image) - assert docker_image == DockerImage(name="ubuntu", tag="latest", owner=None, registry=None) + assert docker_image == DockerImage( + name="ubuntu", tag="latest", owner=None, registry=None + ) def test_convert_to_neuro_image(self): docker_image = DockerImage(name="ubuntu", tag="latest") neuro_image = self.parser.convert_to_neuro_image(docker_image) - assert neuro_image == DockerImage(name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro") + assert neuro_image == DockerImage( + name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" + ) @pytest.mark.usefixtures("patch_docker_host") From 09cca3d3834483a21f1c131b5893890d20e3951a Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Tue, 26 Feb 2019 13:39:36 +0300 Subject: [PATCH 26/28] review: add disambiguation check --- python/neuromation/client/parsing_utils.py | 8 ++++++++ python/tests/client/test_images.py | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/python/neuromation/client/parsing_utils.py b/python/neuromation/client/parsing_utils.py index c4d9c693c..410dc7c27 100644 --- a/python/neuromation/client/parsing_utils.py +++ b/python/neuromation/client/parsing_utils.py @@ -14,12 +14,14 @@ def __init__(self, default_user: str, registry_url: str): def parse_as_docker_image(self, image: str) -> DockerImage: try: + self._check_for_disambiguation(image) return self._parse_as_docker_image(image) except ValueError as e: raise ValueError(f"Invalid docker image '{image}': {e}") from e def parse_as_neuro_image(self, image: str) -> DockerImage: try: + self._check_for_disambiguation(image) return self._parse_as_neuro_image(image) except ValueError as e: raise ValueError(f"Invalid remote image '{image}': {e}") from e @@ -39,6 +41,12 @@ def convert_to_neuro_image(self, image: DockerImage) -> DockerImage: def convert_to_docker_image(self, image: DockerImage) -> DockerImage: return DockerImage(name=image.name, tag=image.tag) + def _check_for_disambiguation(self, image: str) -> None: + if image == "image:latest": + raise ValueError( + "ambiguous value: valid as both local and remote image name" + ) + def _parse_as_docker_image(self, image: str) -> DockerImage: if not image: raise ValueError("empty image name") diff --git a/python/tests/client/test_images.py b/python/tests/client/test_images.py index cc166c0e1..d0c07fab4 100644 --- a/python/tests/client/test_images.py +++ b/python/tests/client/test_images.py @@ -416,6 +416,18 @@ def test_convert_to_neuro_image(self): name="ubuntu", tag="latest", owner="alice", registry="reg.neu.ro" ) + # corner case 'image:latest' + + def test_parse_as_neuro_image__ambiguous_case__fail(self): + url = "image:latest" + with pytest.raises(ValueError, match="ambiguous value"): + self.parser.parse_as_neuro_image(url) + + def test_parse_as_docker_image__ambiguous_case__fail(self): + url = "image:latest" + with pytest.raises(ValueError, match="ambiguous value"): + self.parser.parse_as_docker_image(url) + @pytest.mark.usefixtures("patch_docker_host") class TestImages: From 4d048d43fc466aeb0936fb2bb65fa3665aa3aa4b Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Tue, 26 Feb 2019 15:21:00 +0300 Subject: [PATCH 27/28] add changelog message --- python/CHANGELOG.D/539.bugfix | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 python/CHANGELOG.D/539.bugfix diff --git a/python/CHANGELOG.D/539.bugfix b/python/CHANGELOG.D/539.bugfix new file mode 100644 index 000000000..3d59f58bd --- /dev/null +++ b/python/CHANGELOG.D/539.bugfix @@ -0,0 +1,2 @@ +Fix parsing image URIs. + From d4c4972705c4d9ac4b80b65506014da5ccf748fc Mon Sep 17 00:00:00 2001 From: Artem Yushkovskiy Date: Tue, 26 Feb 2019 15:42:36 +0300 Subject: [PATCH 28/28] lint conflict --- python/neuromation/client/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/neuromation/client/__init__.py b/python/neuromation/client/__init__.py index 40377c028..d57988884 100644 --- a/python/neuromation/client/__init__.py +++ b/python/neuromation/client/__init__.py @@ -32,9 +32,9 @@ Volume, ) from .models import Models, TrainResult +from .parsing_utils import ImageNameParser from .storage import FileStatus, FileStatusType, Storage from .users import Action, Permission, Users -from .parsing_utils import ImageNameParser __all__ = (