From 605943c0beeea1beb86f49221492134fd55eeca2 Mon Sep 17 00:00:00 2001 From: cblmemo Date: Sun, 17 Sep 2023 23:29:51 -0700 Subject: [PATCH 1/6] accept both --- sky/skylet/providers/command_runner.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sky/skylet/providers/command_runner.py b/sky/skylet/providers/command_runner.py index 02eea737727..e12305882b9 100644 --- a/sky/skylet/providers/command_runner.py +++ b/sky/skylet/providers/command_runner.py @@ -143,7 +143,9 @@ def run_init(self, *, as_head: bool, file_mounts: Dict[str, str], docker_login_config.password, docker_login_config.server, )) - specific_image = f'{docker_login_config.server}/{specific_image}' + server_prefix = f'{docker_login_config.server}/' + if not specific_image.startswith(server_prefix): + specific_image = f'{server_prefix}{specific_image}' if self.docker_config.get('pull_before_run', True): assert specific_image, ('Image must be included in config if ' + From 9aa1636b862aa35fd0f05bf5e141578110032930 Mon Sep 17 00:00:00 2001 From: cblmemo Date: Mon, 18 Sep 2023 00:23:06 -0700 Subject: [PATCH 2/6] apply suggestion from code review --- sky/skylet/providers/command_runner.py | 4 +--- sky/task.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/sky/skylet/providers/command_runner.py b/sky/skylet/providers/command_runner.py index e12305882b9..02eea737727 100644 --- a/sky/skylet/providers/command_runner.py +++ b/sky/skylet/providers/command_runner.py @@ -143,9 +143,7 @@ def run_init(self, *, as_head: bool, file_mounts: Dict[str, str], docker_login_config.password, docker_login_config.server, )) - server_prefix = f'{docker_login_config.server}/' - if not specific_image.startswith(server_prefix): - specific_image = f'{server_prefix}{specific_image}' + specific_image = f'{docker_login_config.server}/{specific_image}' if self.docker_config.get('pull_before_run', True): assert specific_image, ('Image must be included in config if ' + diff --git a/sky/task.py b/sky/task.py index 9eb45ec1ea7..9e9850f83af 100644 --- a/sky/task.py +++ b/sky/task.py @@ -126,13 +126,21 @@ def _with_docker_login_config( task_envs) def _add_docker_login_config(resources: 'resources_lib.Resources'): - if resources.extract_docker_image() is None: + docker_image = resources.extract_docker_image() + if docker_image is None: logger.warning(f'{colorama.Fore.YELLOW}Docker login configs ' f'{", ".join(all_keys)} are provided, but no docker ' 'image is specified in `image_id`. The login configs' f' will be ignored.{colorama.Style.RESET_ALL}') return resources - return resources.copy(_docker_login_config=docker_login_config) + # Already checked in extract_docker_image + assert len(resources.image_id) == 1 + region = list(resources.image_id.keys())[0] + server_prefix = f'{docker_login_config.server}' + if not docker_image.startswith(server_prefix): + docker_image = f'{server_prefix}{docker_image}' + return resources.copy(image_id={region: 'docker:' + docker_image}, + _docker_login_config=docker_login_config) return {_add_docker_login_config(resources) for resources in resources_set} From fda980fe30a2ba5ecfddd77995532be1f60553cc Mon Sep 17 00:00:00 2001 From: cblmemo Date: Mon, 18 Sep 2023 00:27:51 -0700 Subject: [PATCH 3/6] fix --- sky/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/task.py b/sky/task.py index 9e9850f83af..ac57f4a8234 100644 --- a/sky/task.py +++ b/sky/task.py @@ -136,7 +136,7 @@ def _add_docker_login_config(resources: 'resources_lib.Resources'): # Already checked in extract_docker_image assert len(resources.image_id) == 1 region = list(resources.image_id.keys())[0] - server_prefix = f'{docker_login_config.server}' + server_prefix = f'{docker_login_config.server}/' if not docker_image.startswith(server_prefix): docker_image = f'{server_prefix}{docker_image}' return resources.copy(image_id={region: 'docker:' + docker_image}, From ae328018b273cd7380e67ba747d7cd69962537a9 Mon Sep 17 00:00:00 2001 From: cblmemo Date: Mon, 18 Sep 2023 00:38:14 -0700 Subject: [PATCH 4/6] fix --- sky/skylet/providers/command_runner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sky/skylet/providers/command_runner.py b/sky/skylet/providers/command_runner.py index 02eea737727..69ec5adfce8 100644 --- a/sky/skylet/providers/command_runner.py +++ b/sky/skylet/providers/command_runner.py @@ -143,7 +143,6 @@ def run_init(self, *, as_head: bool, file_mounts: Dict[str, str], docker_login_config.password, docker_login_config.server, )) - specific_image = f'{docker_login_config.server}/{specific_image}' if self.docker_config.get('pull_before_run', True): assert specific_image, ('Image must be included in config if ' + From 19524a94c3c1285214f3bb17c7eb4a712659afa0 Mon Sep 17 00:00:00 2001 From: Tian Xia Date: Mon, 18 Sep 2023 01:17:08 -0700 Subject: [PATCH 5/6] Update sky/task.py Co-authored-by: Zhanghao Wu --- sky/task.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sky/task.py b/sky/task.py index f623be6a548..f6652d9e1c7 100644 --- a/sky/task.py +++ b/sky/task.py @@ -147,6 +147,8 @@ def _add_docker_login_config(resources: 'resources_lib.Resources'): # Already checked in extract_docker_image assert len(resources.image_id) == 1 region = list(resources.image_id.keys())[0] + # We automatically add the server prefix to the image name if + # the user did not add it. server_prefix = f'{docker_login_config.server}/' if not docker_image.startswith(server_prefix): docker_image = f'{server_prefix}{docker_image}' From 283c5551b39d66075cffdd9cf730627c8c1108d2 Mon Sep 17 00:00:00 2001 From: Tian Xia Date: Mon, 18 Sep 2023 01:17:36 -0700 Subject: [PATCH 6/6] Update sky/task.py Co-authored-by: Zhanghao Wu --- sky/task.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/task.py b/sky/task.py index f6652d9e1c7..74ccb5f35c7 100644 --- a/sky/task.py +++ b/sky/task.py @@ -145,7 +145,7 @@ def _add_docker_login_config(resources: 'resources_lib.Resources'): f'ignored.{colorama.Style.RESET_ALL}') return resources # Already checked in extract_docker_image - assert len(resources.image_id) == 1 + assert len(resources.image_id) == 1, resources.image_id region = list(resources.image_id.keys())[0] # We automatically add the server prefix to the image name if # the user did not add it.