From bd82c5888349ca52f6c6b91b7d38b5d564f7c18a Mon Sep 17 00:00:00 2001 From: williams-jack Date: Mon, 29 Jul 2024 21:41:58 -0400 Subject: [PATCH] fix: append grader to image tag for build Docker does not allow setting the image repository (essentially, a tag) as a 64-byte SHA Sum string. --- .../src/process-request/image-creation.ts | 12 +++---- .../orca_grader/docker_utils/images/utils.py | 32 +++++++++---------- .../docker_grading_job_executor_builder.py | 8 ++--- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts index 53ac7123..c3683f87 100644 --- a/orchestrator/packages/image-build-service/src/process-request/image-creation.ts +++ b/orchestrator/packages/image-build-service/src/process-request/image-creation.ts @@ -52,7 +52,7 @@ const buildImage = ({ dockerfile_sha_sum }: GraderImageBuildRequest, buildLogs: const dockerBuildArgs = [ "build", "-t", - dockerfile_sha_sum, + `grader-${dockerfile_sha_sum}`, "-f", path.join(CONFIG.dockerImageFolder, `${dockerfile_sha_sum}.Dockerfile`), ".", @@ -61,7 +61,7 @@ const buildImage = ({ dockerfile_sha_sum }: GraderImageBuildRequest, buildLogs: execFile( "docker", dockerBuildArgs, - (err, stdout, stderr) => { + (err, _stdout, stderr) => { const step: ImageBuildStep = "Run docker build on Dockerfile."; if (err) { buildLogs.push({ @@ -101,12 +101,12 @@ const removeDockerfileAfterBuild = (dockerfilePath: string, buildLogs: Array): Promise => { +const saveImageToTgz = (shaSum: string, buildLogs: Array): Promise => { const dockerSaveCommandArgs = [ "save", "-o", - path.join(CONFIG.dockerImageFolder, `${imageName}.tgz`), - imageName, + path.join(CONFIG.dockerImageFolder, `${shaSum}.tgz`), + `grader-${shaSum}`, ]; return new Promise((resolve, reject) => { execFile( @@ -118,7 +118,7 @@ const saveImageToTgz = (imageName: string, buildLogs: Array): Pro buildLogs.push({ step, error: stderr }); reject({ was_successful: false, logs: buildLogs }); } else { - buildLogs.push({ step, output: `Successfully saved image to ${imageName}.tgz.` }); + buildLogs.push({ step, output: `Successfully saved image to ${shaSum}.tgz.` }); resolve(); } }, diff --git a/worker/orca_grader/docker_utils/images/utils.py b/worker/orca_grader/docker_utils/images/utils.py index 616cc967..ad9c8018 100644 --- a/worker/orca_grader/docker_utils/images/utils.py +++ b/worker/orca_grader/docker_utils/images/utils.py @@ -1,23 +1,23 @@ import subprocess from typing import List + def does_image_exist_locally(container_sha: str) -> bool: - program_args = [ - "docker", - "image", - "ls", - "--format", - "{{.Repository}}" - ] - proc_res = subprocess.run(program_args, capture_output=True, check=True) - image_names = proc_res.stdout.decode().split('\n')[:-1] - return container_sha in image_names + program_args = [ + "docker", + "image", + "ls", + "--format", + "{{.Repository}}" + ] + proc_res = subprocess.run(program_args, capture_output=True, check=True) + image_names = proc_res.stdout.decode().split('\n')[:-1] + return f"grader-{container_sha}" in image_names def get_all_docker_images() -> List[str]: - res = subprocess.run(["docker", "image", "ls", "--format", "{{.Repository}}"], - capture_output=True) - output = res.stdout.decode() - image_names = output.split('\n')[:-1] - return image_names - + res = subprocess.run(["docker", "image", "ls", "--format", "{{.Repository}}"], + capture_output=True) + output = res.stdout.decode() + image_names = output.split('\n')[:-1] + return image_names diff --git a/worker/orca_grader/executor/builder/docker_grading_job_executor_builder.py b/worker/orca_grader/executor/builder/docker_grading_job_executor_builder.py index 4c24f616..777de8fc 100644 --- a/worker/orca_grader/executor/builder/docker_grading_job_executor_builder.py +++ b/worker/orca_grader/executor/builder/docker_grading_job_executor_builder.py @@ -14,14 +14,14 @@ class DockerGradingJobExecutorBuilder(GradingJobExecutorBuilder): def __init__(self, container_sha: str, container_command: List[str] = __DEFAULT_CONTIANER_CMD) -> None: - self.__container_sha = container_sha + self.__container_tag = f"grader-{container_sha}" self.__container_command = container_command self.__file_mappings: Dict[str, str] = dict() self.__env_variable_mappings: Dict[str, str] = dict() def __num_containers_with_same_sha(self) -> int: filter_op: Callable[[str], bool] = lambda n: n.startswith( - self.__container_sha) + self.__container_tag) return len(list(filter(filter_op, get_all_container_names()))) def add_docker_volume_mapping(self, local_path: str, container_path: str) -> None: @@ -32,7 +32,7 @@ def add_docker_environment_variable_mapping(self, name: str, value: str) -> None def build(self) -> GradingJobExecutor: # TODO: How could we maintain a 'reference count' for a unique id? - container_name = f"{self.__container_sha}_{int(time.time() * 100_000_000)}" + container_name = f"{self.__container_tag}_{int(time.time() * 100_000_000)}" program_sequence = ["docker", "run", "--rm", "--name", container_name] program_sequence.extend(["--network", "orca-testing"]) for name, value in self.__env_variable_mappings.items(): @@ -41,6 +41,6 @@ def build(self) -> GradingJobExecutor: for local_path, container_path in self.__file_mappings.items(): program_sequence.append("-v") program_sequence.append(f"{local_path}:{container_path}") - program_sequence.append(self.__container_sha) + program_sequence.append(self.__container_tag) program_sequence.extend(self.__container_command) return DockerGradingJobExecutor(create_runnable_job_subprocess(program_sequence), container_name)