docker.container: Refactor to support container recreation

pyinfra/operations/docker.py

@@ -8,7 +8,7 @@
 from pyinfra.api import operation
 from pyinfra.facts.docker import DockerContainer, DockerNetwork, DockerVolume
 
-from .util.docker import handle_docker
+from .util.docker import ContainerSpec, handle_docker
 
 
 @operation()
@@ -70,56 +70,60 @@ def container(
         )
     """
 
+    want_spec = ContainerSpec(
+        image,
+        ports or list(),
+        networks or list(),
+        volumes or list(),
+        env_vars or list(),
+        pull_always,
+    )
     existent_container = host.get_fact(DockerContainer, object_id=container)
 
-    if force:
-        if existent_container:
-            yield handle_docker(
-                resource="container",
-                command="remove",
-                container=container,
-            )
+    container_spec_changes = want_spec.diff_from_inspect(existent_container)
 
-    if present:
-        if not existent_container or force:
-            yield handle_docker(
-                resource="container",
-                command="create",
-                container=container,
-                image=image,
-                ports=ports,
-                networks=networks,
-                volumes=volumes,
-                env_vars=env_vars,
-                pull_always=pull_always,
-                present=present,
-                force=force,
-                start=start,
-            )
-
-    if existent_container and start:
-        if existent_container[0]["State"]["Status"] != "running":
-            yield handle_docker(
-                resource="container",
-                command="start",
-                container=container,
-            )
-
-    if existent_container and not start:
-        if existent_container[0]["State"]["Status"] == "running":
-            yield handle_docker(
-                resource="container",
-                command="stop",
-                container=container,
-            )
-
-    if existent_container and not present:
+    is_running = (
+        (existent_container[0]["State"]["Status"] == "running")
+        if existent_container and existent_container[0]
+        else False
+    )
+    recreating = existent_container and (force or container_spec_changes)
+    removing = existent_container and not present
+
+    do_remove = recreating or removing
+    do_create = (present and not existent_container) or recreating
+    do_start = start and (recreating or not is_running)
+    do_stop = not start and not removing and is_running
+
+    if do_remove:
         yield handle_docker(
             resource="container",
             command="remove",
             container=container,
         )
 
+    if do_create:
+        yield handle_docker(
+            resource="container",
+            command="create",
+            container=container,
+            spec=want_spec,
+        )
+
+    if do_start:
+        yield handle_docker(
+            resource="container",
+            command="start",
+            container=container,
+        )
+
+    if do_stop:
+        yield handle_docker(
+            resource="container",
+            command="stop",
+            container=container,
+        )
+
 
 @operation(is_idempotent=False)
 def image(image, present=True):

pyinfra/operations/util/docker.py

@@ -1,38 +1,60 @@
+import dataclasses
+from typing import Any, Dict, List
+
 from pyinfra.api import OperationError
 
 
-def _create_container(**kwargs):
-    command = []
+@dataclasses.dataclass
+class ContainerSpec:
+    image: str = ""
+    ports: List[str] = dataclasses.field(default_factory=list)
+    networks: List[str] = dataclasses.field(default_factory=list)
+    volumes: List[str] = dataclasses.field(default_factory=list)
+    env_vars: List[str] = dataclasses.field(default_factory=list)
+    pull_always: bool = False
+
+    def container_create_args(self):
+        args = []
+        for network in self.networks:
+            args.append("--network {0}".format(network))
 
-    networks = kwargs["networks"] if kwargs["networks"] else []
-    ports = kwargs["ports"] if kwargs["ports"] else []
-    volumes = kwargs["volumes"] if kwargs["volumes"] else []
-    env_vars = kwargs["env_vars"] if kwargs["env_vars"] else []
+        for port in self.ports:
+            args.append("-p {0}".format(port))
 
-    if kwargs["image"] == "":
-        raise OperationError("missing 1 required argument: 'image'")
+        for volume in self.volumes:
+            args.append("-v {0}".format(volume))
 
-    command.append("docker container create --name {0}".format(kwargs["container"]))
+        for env_var in self.env_vars:
+            args.append("-e {0}".format(env_var))
 
-    for network in networks:
-        command.append("--network {0}".format(network))
+        if self.pull_always:
+            args.append("--pull always")
 
-    for port in ports:
-        command.append("-p {0}".format(port))
+        args.append(self.image)
 
-    for volume in volumes:
-        command.append("-v {0}".format(volume))
+        return args
 
-    for env_var in env_vars:
-        command.append("-e {0}".format(env_var))
+    def diff_from_inspect(self, inspect_dict: Dict[str, Any]) -> List[str]:
+        # TODO(@minor-fixes): Diff output of "docker inspect" against this spec
+        # to determine if the container needs to be recreated. Currently, this
+        # function will never recreate when attributes change, which is
+        # consistent with prior behavior.
+        del inspect_dict
+        return []
+
+
+def _create_container(**kwargs):
+    if "spec" not in kwargs:
+        raise OperationError("missing 1 required argument: 'spec'")
 
-    if kwargs["pull_always"]:
-        command.append("--pull always")
+    spec = kwargs["spec"]
 
-    command.append(kwargs["image"])
+    if not spec.image:
+        raise OperationError("Docker image not specified")
 
-    if kwargs["start"]:
-        command.append("; {0}".format(_start_container(container=kwargs["container"])))
+    command = [
+        "docker container create --name {0}".format(kwargs["container"])
+    ] + spec.container_create_args()
 
     return " ".join(command)
 

tests/operations/docker.container/add_and_start_no_existent_container.json

@@ -10,10 +10,11 @@
     },
     "facts": {
         "docker.DockerContainer": {
-            "object_id=nginx": [] 
+            "object_id=nginx": []
         }
     },
     "commands": [
-        "docker container create --name nginx -p 80:80 nginx:alpine ; docker container start nginx"
+        "docker container create --name nginx -p 80:80 nginx:alpine",
+        "docker container start nginx"
     ]
 }