Skip to content

Commit

Permalink
fix: append 'grader-' outside of image exists check
Browse files Browse the repository at this point in the history
chore: make local orchestrator containers work with bottlenose
  • Loading branch information
williams-jack committed Jul 30, 2024
1 parent bd82c58 commit cc69d15
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 34 deletions.
5 changes: 2 additions & 3 deletions orchestrator/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ services:
- "4000:4000"
depends_on:
- postgres
links:
- postgres
environment:
POSTGRES_URL: postgresql://postgres:password@postgres
POSTGRES_URL: postgresql://postgres:password@localhost:5434
container_name: orca-orchestrator
volumes:
- ./packages:/usr/packages
- /var/run/docker.sock:/var/run/docker.sock
restart: always
network_mode: host

postgres:
image: postgres:10
Expand Down
5 changes: 3 additions & 2 deletions worker/orca_grader/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,10 @@ def run_grading_job(grading_job: GradingJobJSON, no_container: bool,
if no_container:
return handle_grading_job(grading_job)
container_sha = grading_job["grader_image_sha"]
if not does_image_exist_locally(container_sha):
if not does_image_exist_locally(f"grader-{container_sha}"):
retrieve_image_tgz_from_url(
container_sha, f"{APP_CONFIG.orca_web_server_host}/images/{container_sha}.tgz")
container_sha, f"{APP_CONFIG.orca_web_server_host}/images/{container_sha}.tgz"
)
load_image_from_tgz("{0}.tgz".format(container_sha))
if container_command:
handle_grading_job(
Expand Down
26 changes: 13 additions & 13 deletions worker/orca_grader/docker_utils/images/image_loading.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import os
import subprocess
from orca_grader.common.services.download_file import download_file
from orca_grader.config import APP_CONFIG


def retrieve_image_tgz_from_url(container_sha: str, images_url: str) -> None:
file_name = "{0}.tgz".format(container_sha)
download_file(images_url, file_name)
file_name = "{0}.tgz".format(container_sha)
download_file(images_url, file_name)


def load_image_from_tgz(tgz_file_path: str) -> None:
program_args = [
"docker",
"load",
"-i",
tgz_file_path
]
with open(tgz_file_path, 'rb') as container_tgz_fp:
program_args = [
"docker",
"load",
"-i",
tgz_file_path
]
subprocess.run(program_args, stdout=subprocess.DEVNULL,
stderr=subprocess.STDOUT,
check=True)
os.remove(tgz_file_path) # To save resources, clean up tgz after load.
stderr=subprocess.STDOUT,
check=True)
os.remove(tgz_file_path) # To save resources, clean up tgz after load.
4 changes: 2 additions & 2 deletions worker/orca_grader/docker_utils/images/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from typing import List


def does_image_exist_locally(container_sha: str) -> bool:
def does_image_exist_locally(container_tag: str) -> bool:

This comment has been minimized.

Copy link
@blerner

blerner Aug 12, 2024

Collaborator

Why is this duplicating the program_args array and process call? Shouldn't it just be

def does_image_exist_locally(container_tag: str) -> bool:
    return container_tag in get_all_docker_images()
program_args = [
"docker",
"image",
Expand All @@ -12,7 +12,7 @@ def does_image_exist_locally(container_sha: str) -> bool:
]
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
return container_tag in image_names


def get_all_docker_images() -> List[str]:
Expand Down
31 changes: 17 additions & 14 deletions worker/orca_grader/tests/docker_images/test_docker_image_loading.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,26 @@
from os import path

from orca_grader.docker_utils.images.image_loading import retrieve_image_tgz_from_url, \
load_image_from_tgz
load_image_from_tgz
from orca_grader.docker_utils.images.utils import does_image_exist_locally


def _remove_example_contianer() -> None:
subprocess.run(["docker", "image", "rm", "hello-world"],
stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT)
subprocess.run(["docker", "image", "rm", "hello-world"],
stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT)


class TestDockerImageLoading(unittest.TestCase):

@classmethod
def setUpClass(cls) -> None:
_remove_example_contianer()
return super().setUpClass()

def test_image_download_and_loading(self):
retrieve_image_tgz_from_url("hello-world", "http://localhost:9000/images/hello-world.tgz")
self.assertTrue(path.exists("hello-world.tgz"))
load_image_from_tgz("hello-world.tgz")
self.assertTrue(does_image_exist_locally("hello-world"))
self.assertFalse(path.exists("hello-world.tgz"))
@classmethod
def setUpClass(cls) -> None:
_remove_example_contianer()
return super().setUpClass()

def test_image_download_and_loading(self):
retrieve_image_tgz_from_url(
"hello-world", "http://localhost:9000/images/hello-world.tgz")
self.assertTrue(path.exists("hello-world.tgz"))
load_image_from_tgz("hello-world.tgz")
self.assertTrue(does_image_exist_locally("hello-world"))
self.assertFalse(path.exists("hello-world.tgz"))

0 comments on commit cc69d15

Please sign in to comment.