From 6451881179910148bd02f256da74cdb74a0711f3 Mon Sep 17 00:00:00 2001 From: Michael Sherman Date: Thu, 12 Feb 2026 11:04:57 -0600 Subject: [PATCH 1/3] test cases for container.submit --- tests/test_container.py | 88 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/test_container.py b/tests/test_container.py index c18bc09..8ac6d10 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -18,6 +18,7 @@ from datetime import datetime import pytest +from zunclient.exceptions import Conflict from chi.container import Container, download, upload @@ -110,3 +111,90 @@ def test_download_extracts_tar_and_writes_file(mocker): assert os.path.exists(dest_path) with open(dest_path, "rb") as f: assert f.read() == file_content + + +def test_submit_idempotent_returns_existing_without_create(mocker): + # idempotent=true, wait=true + chi_container = Container(name="dup-name", image_ref="img") + existing_zun_container = mocker.Mock(uuid="existing-uuid", status="Running") + + mocker.patch("chi.container.get_container", return_value=existing_zun_container) + create_mock = mocker.patch("chi.container.create_container") + + submit_result = chi_container.submit( + idempotent=True, wait_for_active=True, wait_timeout=123, show="text" + ) + create_mock.assert_not_called() + existing_zun_container.wait.assert_called_once_with(status="Running", timeout=123) + existing_zun_container.show.assert_called_once_with( + type="text", wait_for_active=True + ) + + # submit returns only on idempotent=true + assert submit_result is existing_zun_container + + +def test_submit_idempotent_returns_existing_without_create_no_wait(mocker): + # idempotent=true, wait=false + chi_container = Container(name="dup-name", image_ref="img") + existing_zun_container = mocker.Mock(uuid="existing-uuid", status="Running") + + mocker.patch("chi.container.get_container", return_value=existing_zun_container) + create_mock = mocker.patch("chi.container.create_container") + + submit_result = chi_container.submit(idempotent=True, wait_for_active=False, show=None) + create_mock.assert_not_called() + existing_zun_container.wait.assert_not_called() + existing_zun_container.show.assert_not_called() + + # submit returns only on idempotent=true + assert submit_result is existing_zun_container + + +def test_submit_preserves_reference_on_create_wait_failure(mocker): + """Ensure that we keep the zun container id, even if create fails. + + This case can arise because the container moves to an error state, or if + the wait times out for another reason. + """ + chi_container = Container(name="test", image_ref="img") + leaked_zun_container = mocker.Mock(uuid="leaked-uuid", status="Error") + + mocker.patch("chi.container.create_container", side_effect=RuntimeError) + zun_mock = mocker.patch("chi.container.zun")() + zun_mock.containers.get.return_value = leaked_zun_container + + with pytest.raises(RuntimeError): + chi_container.submit(wait_for_active=False, show=None) + + assert chi_container.id == "leaked-uuid" + assert chi_container._status == "Error" + + +def test_submit_duplicate_name_tracks_created_uuid(mocker): + """Test the case where we re-run submit after a failure. + + An "old" container alreday already exists, with name = "dup-name". + If idempotent=false, it should be possible to make a new container with the same name. + However, name-based lookups will fail with a 409. + """ + + chi_container = Container(name="dup-name", image_ref="img") + new_zun_container = mocker.Mock(uuid="new-uuid", status="Running") + + def _get_side_effect(ref): + if ref == "dup-name": + raise Conflict( + "Multiple containers exist with same name. Please use the container uuid instead." + ) + if ref == "new-uuid": + return new_zun_container + raise AssertionError(ref) + + mocker.patch("chi.container.create_container", return_value=new_zun_container) + zun_mock = mocker.patch("chi.container.zun")() + zun_mock.containers.get.side_effect = _get_side_effect + + # disable optional behavor from wait_for_active and show + chi_container.submit(wait_for_active=False, show=None) + assert chi_container.id == "new-uuid" From ea5589cc6b64ea65104e1f70701861623c216430 Mon Sep 17 00:00:00 2001 From: Michael Sherman Date: Thu, 12 Feb 2026 12:29:50 -0600 Subject: [PATCH 2/3] pass zun container info via exception on wait exc. The root cause of submit() returning null container id was because _wait_for_status can raise inside create_container, between the zun creation and returning the zun object. By adding the zun container to a custom exception inside create_container, submit() can still populate self.id and self._status from the zun object. This also makes it possible to set details from the zun container status_reason and status_detail fields. Needs Review: submit() now raises a ContainerCreateWaitError, not ResourceError, which may break existing code which depends on handling that error. We might want to instead reraise a ResourceError from the new custom error. --- chi/container.py | 57 ++++++++++++++++++++++------------------- chi/exception.py | 15 +++++++++++ tests/test_container.py | 10 ++++++-- 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/chi/container.py b/chi/container.py index fe8e012..b3cadb5 100644 --- a/chi/container.py +++ b/chi/container.py @@ -28,7 +28,7 @@ from .clients import connection, zun from .context import session -from .exception import ResourceError, ServiceError +from .exception import ContainerCreateWaitError, ResourceError, ServiceError from .network import bind_floating_ip, get_free_floating_ip DEFAULT_IMAGE_DRIVER = "docker" @@ -150,24 +150,26 @@ def submit( if self.workdir: kwargs["workdir"] = self.workdir - container = create_container( - name=self.name, - image=self.image_ref, - exposed_ports=self.exposed_ports, - reservation_id=self.reservation_id, - start=self.start, - start_timeout=self.start_timeout, - runtime=self.runtime, - environment=self.environment, - device_profiles=self.device_profiles, - **kwargs, - ) - - if container: - self.id = zun().containers.get(self.name).uuid - self._status = zun().containers.get(self.name).status - else: - raise ResourceError("could not create container") + try: + container = create_container( + name=self.name, + image=self.image_ref, + exposed_ports=self.exposed_ports, + reservation_id=self.reservation_id, + start=self.start, + start_timeout=self.start_timeout, + runtime=self.runtime, + environment=self.environment, + device_profiles=self.device_profiles, + **kwargs, + ) + self.id = container.uuid + self._status = container.status + except ContainerCreateWaitError as exc: + # ensure container object gets params even on error + self.id = exc.zun_container.uuid + self._status = exc.zun_container.status + raise ResourceError(message=exc.zun_container.status_reason) from exc if wait_for_active and self.status != "Running": self.wait(status="Running", timeout=wait_timeout) @@ -435,13 +437,16 @@ def create_container( timeout = start_timeout or (60 * 30) LOG.info(f"Waiting up to {timeout}s for container creation ...") - if platform_version == 2: - container = _wait_for_status(container.uuid, "Running", timeout=timeout) - else: - container = _wait_for_status(container.uuid, "Created", timeout=timeout) - if start: - LOG.info("Starting container ...") - zun().containers.start(container.uuid) + try: + if platform_version == 2: + container = _wait_for_status(container.uuid, "Running", timeout=timeout) + else: + container = _wait_for_status(container.uuid, "Created", timeout=timeout) + if start: + LOG.info("Starting container ...") + zun().containers.start(container.uuid) + except (RuntimeError, TimeoutError) as exc: + raise ContainerCreateWaitError(zun_container=container, cause=exc) from exc return container diff --git a/chi/exception.py b/chi/exception.py index 5ef0a15..fc7a641 100644 --- a/chi/exception.py +++ b/chi/exception.py @@ -28,3 +28,18 @@ class ServiceError(Exception): def __init__(self, message): super().__init__(message) + + +class ContainerCreateWaitError(ResourceError): + """Raised when Zun creates a container but waiting for target status fails.""" + + def __init__(self, zun_container, cause): + self.zun_container = zun_container + self.cause = cause + message = ( + "Container {} was created, but waiting for target status failed: {}".format( + self.zun_container.uuid, + cause, + ) + ) + super().__init__(message) diff --git a/tests/test_container.py b/tests/test_container.py index 8ac6d10..91ac5d9 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -21,6 +21,7 @@ from zunclient.exceptions import Conflict from chi.container import Container, download, upload +from chi.exception import ContainerCreateWaitError, ResourceError @pytest.fixture() @@ -160,11 +161,16 @@ def test_submit_preserves_reference_on_create_wait_failure(mocker): chi_container = Container(name="test", image_ref="img") leaked_zun_container = mocker.Mock(uuid="leaked-uuid", status="Error") - mocker.patch("chi.container.create_container", side_effect=RuntimeError) + mocker.patch( + "chi.container.create_container", + side_effect=ContainerCreateWaitError( + zun_container=leaked_zun_container, cause=RuntimeError + ), + ) zun_mock = mocker.patch("chi.container.zun")() zun_mock.containers.get.return_value = leaked_zun_container - with pytest.raises(RuntimeError): + with pytest.raises(ResourceError): chi_container.submit(wait_for_active=False, show=None) assert chi_container.id == "leaked-uuid" From 669f844458b4d90aa1a23999cce8be54902283d0 Mon Sep 17 00:00:00 2001 From: Michael Sherman Date: Wed, 18 Feb 2026 12:34:48 -0600 Subject: [PATCH 3/3] apply linter fixes --- chi/exception.py | 4 ++-- tests/test_container.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/chi/exception.py b/chi/exception.py index fc7a641..4ad4b45 100644 --- a/chi/exception.py +++ b/chi/exception.py @@ -38,8 +38,8 @@ def __init__(self, zun_container, cause): self.cause = cause message = ( "Container {} was created, but waiting for target status failed: {}".format( - self.zun_container.uuid, + self.zun_container.uuid, cause, - ) + ) ) super().__init__(message) diff --git a/tests/test_container.py b/tests/test_container.py index 91ac5d9..e04e168 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -143,7 +143,9 @@ def test_submit_idempotent_returns_existing_without_create_no_wait(mocker): mocker.patch("chi.container.get_container", return_value=existing_zun_container) create_mock = mocker.patch("chi.container.create_container") - submit_result = chi_container.submit(idempotent=True, wait_for_active=False, show=None) + submit_result = chi_container.submit( + idempotent=True, wait_for_active=False, show=None + ) create_mock.assert_not_called() existing_zun_container.wait.assert_not_called() existing_zun_container.show.assert_not_called()