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..4ad4b45 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 c18bc09..e04e168 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -18,8 +18,10 @@ from datetime import datetime import pytest +from zunclient.exceptions import Conflict from chi.container import Container, download, upload +from chi.exception import ContainerCreateWaitError, ResourceError @pytest.fixture() @@ -110,3 +112,97 @@ 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=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(ResourceError): + 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"