From fb6f53aa424d73e965fb2293f5e7ab5a08409095 Mon Sep 17 00:00:00 2001 From: Erwan Rouchet <rouchet@teklia.com> Date: Thu, 12 Sep 2024 16:44:39 +0200 Subject: [PATCH] Set container name when running Ponos tasks via RQ --- arkindex/ponos/tasks.py | 10 +++++++++ arkindex/ponos/tests/test_rq_tasks.py | 31 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/arkindex/ponos/tasks.py b/arkindex/ponos/tasks.py index c8ea34aa2b..172ef20806 100644 --- a/arkindex/ponos/tasks.py +++ b/arkindex/ponos/tasks.py @@ -162,6 +162,15 @@ def run_docker_task(client, task, temp_dir): return # 4. Do run the container asynchronously + container_name = f"ponos-{task.id}" + try: + client.containers.get(container_name) + except docker.errors.NotFound: + # No container exists with the name, carry on + pass + else: + raise ValueError(f"A Docker container named {container_name} already exists") + logger.debug("Running container") kwargs = { "environment": { @@ -171,6 +180,7 @@ def run_docker_task(client, task, temp_dir): "detach": True, "network": "host", "volumes": {str(temp_dir): {"bind": str(settings.PONOS_DATA_DIR), "mode": "rw"}}, + "name": container_name, } artifacts_dir = temp_dir / str(task.id) artifacts_dir.mkdir() diff --git a/arkindex/ponos/tests/test_rq_tasks.py b/arkindex/ponos/tests/test_rq_tasks.py index c25ce7561e..7a691e6399 100644 --- a/arkindex/ponos/tests/test_rq_tasks.py +++ b/arkindex/ponos/tests/test_rq_tasks.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, PropertyMock, call, patch, seal from django.test import override_settings +import docker from arkindex.ponos.models import Farm, State, Task from arkindex.ponos.tasks import run_docker_task from arkindex.process.models import ProcessMode, WorkerVersion @@ -97,6 +98,7 @@ class TestModels(FixtureAPITestCase): seal(sleep_mock) client_mock.images.pull.return_value = None + client_mock.containers.get.side_effect = docker.errors.NotFound("Not found.") container.reload.return_value = None container.stop.return_value = None # Sealing is not friendly with PropertyMocks, so we put a placeholder first @@ -120,6 +122,8 @@ class TestModels(FixtureAPITestCase): self.assertEqual(client_mock.images.pull.call_count, 1) self.assertEqual(client_mock.images.pull.call_args, call("image")) + self.assertEqual(client_mock.containers.get.call_count, 1) + self.assertEqual(client_mock.containers.get.call_args, call(f"ponos-{task.id}")) self.assertEqual(client_mock.containers.run.call_count, 1) self.assertEqual(client_mock.containers.run.call_args, call( "image", @@ -127,6 +131,7 @@ class TestModels(FixtureAPITestCase): detach=True, network="host", volumes={temp_dir: {"bind": "/data", "mode": "rw"}}, + name=f"ponos-{task.id}", )) self.assertEqual(container.reload.call_count, 2) @@ -141,3 +146,29 @@ class TestModels(FixtureAPITestCase): self.assertEqual(sleep_mock.call_count, 1) self.assertEqual(sleep_mock.call_args, call(1)) + + @override_settings(PONOS_RQ_EXECUTION=True) + def test_run_docker_task_conflict(self): + task = self.process.tasks.create( + slug="something", + image="image", + run=0, + depth=0, + state=State.Pending, + ) + + client_mock = MagicMock() + client_mock.images.pull.return_value = None + client_mock.containers.get.return_value = None + seal(client_mock) + + with ( + tempfile.TemporaryDirectory() as temp_dir, + self.assertRaisesMessage(ValueError, f"A Docker container named ponos-{task.id} already exists"), + ): + run_docker_task(client_mock, task, Path(temp_dir)) + + self.assertEqual(client_mock.images.pull.call_count, 1) + self.assertEqual(client_mock.images.pull.call_args, call("image")) + self.assertEqual(client_mock.containers.get.call_count, 1) + self.assertEqual(client_mock.containers.get.call_args, call(f"ponos-{task.id}")) -- GitLab