Bugs/container submit 409 (#94)

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.

Author: msherman64

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
 

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)

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"