docker.container: Recreate container when args change

This PR allows the `docker.container` operation to tear down and
recreate the container when operation arguments change, instead of
reporting `No change` and doing nothing. This is intended to reduce the
possibility for human error/need for manual intervention when changing
args to `docker.container` operations.

Since it is not possible to extract all operation args from e.g. `docker
inspect` output, this PR takes a similar approach to Docker Compose to
tackle this issue - it serializes the operation args in a deterministic
way, hashes the serialized bytes, and stores this as a label on the
container. If the hash differs from a currently-running container, the
container is recreated.

Tested: Added additional tests for behavior when args are
changing/static in different scenarios

Author: minor-fixes

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 ContainerSpec, handle_docker
+from .util.docker import ContainerSpec, CONTAINER_CONFIG_HASH_LABEL, handle_docker
 
 
 @operation()
@@ -75,28 +75,33 @@ def container(
     want_spec = ContainerSpec(
         image,
         args or list(),
-        ports or list(),
-        networks or list(),
+        set(ports) if ports else set(),
+        set(networks) if networks else set(),
         volumes or list(),
-        env_vars or list(),
+        set(env_vars) if env_vars else set(),
         pull_always,
     )
-    existent_container = host.get_fact(DockerContainer, object_id=container)
+    existent_container = next(iter(host.get_fact(DockerContainer, object_id=container)), {})
 
-    container_spec_changes = want_spec.diff_from_inspect(existent_container)
-
-    is_running = (
-        (existent_container[0]["State"]["Status"] == "running")
-        if existent_container and existent_container[0]
-        else False
+    old_hash = (
+        existent_container.get("Config", {})
+        .get("Labels", {})
+        .get(CONTAINER_CONFIG_HASH_LABEL, None)
     )
-    recreating = existent_container and (force or container_spec_changes)
+
+    container_spec_changed = old_hash != want_spec.config_hash()
+
+    is_running = existent_container.get("State", {}).get("Status", "") == "running"
+    recreating = existent_container and (force or container_spec_changed)
     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
+    do_create = not removing and ((present and not existent_container) or recreating)
+    do_start = present and start and (recreating or not is_running)
+    do_stop = not start and not removing and is_running and not recreating
+
+    if not (do_remove or do_create or do_start or do_stop):
+        host.noop("container configuration is already correct")
 
     if do_remove:
         yield handle_docker(

pyinfra/operations/util/docker.py

@@ -1,31 +1,49 @@
 import dataclasses
-from typing import Any, Dict, List
+import hashlib
+import json
+from typing import Any, Dict, List, Set
 
 from pyinfra.api import OperationError
 
+CONTAINER_CONFIG_HASH_LABEL = "com.github.pyinfra.config-hash"
+
+
+def _json_repr(obj: Any):
+    try:
+        return dataclasses.asdict(obj)
+    except TypeError:
+        pass
+
+    if isinstance(obj, set):
+        return sorted(obj)
+
+    # If there are other alternative types to try (e.g. dates) then do so here
+
+    raise TypeError(f"object {type(obj).__name__} not serializable")
+
 
 @dataclasses.dataclass
 class ContainerSpec:
     image: str = ""
     args: List[str] = dataclasses.field(default_factory=list)
-    ports: List[str] = dataclasses.field(default_factory=list)
-    networks: List[str] = dataclasses.field(default_factory=list)
+    ports: Set[str] = dataclasses.field(default_factory=set)
+    networks: Set[str] = dataclasses.field(default_factory=set)
     volumes: List[str] = dataclasses.field(default_factory=list)
-    env_vars: List[str] = dataclasses.field(default_factory=list)
+    env_vars: Set[str] = dataclasses.field(default_factory=set)
     pull_always: bool = False
 
     def container_create_args(self):
-        args = []
-        for network in self.networks:
+        args = [f"--label '{CONTAINER_CONFIG_HASH_LABEL}={self.config_hash()}'"]
+        for network in sorted(self.networks):
             args.append("--network {0}".format(network))
 
-        for port in self.ports:
+        for port in sorted(self.ports):
             args.append("-p {0}".format(port))
 
         for volume in self.volumes:
             args.append("-v {0}".format(volume))
 
-        for env_var in self.env_vars:
+        for env_var in sorted(self.env_vars):
             args.append("-e {0}".format(env_var))
 
         if self.pull_always:
@@ -36,13 +54,16 @@ def container_create_args(self):
 
         return args
 
-    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 config_hash(self) -> str:
+        serialized = json.dumps(
+            self,
+            default=_json_repr,
+            ensure_ascii=False,
+            sort_keys=True,
+            indent=None,
+            separators=(",", ":"),
+        ).encode("utf-8")
+        return hashlib.sha256(serialized).hexdigest()
 
 
 def _create_container(**kwargs):

tests/operations/docker.container/add_and_start_no_existent_container.json

@@ -7,8 +7,21 @@
             "-g",
             "'daemon off;'"
         ],
+        "networks": [
+            "foo",
+            "bar"
+        ],
+        "volumes": [
+            "/host/a:/container/a",
+            "/host/b:/container/b"
+        ],
         "ports": [
-            "80:80"
+            "80:80",
+            "8081:8081"
+        ],
+        "env_vars": [
+            "ENV_A=foo",
+            "ENV_B=bar"
         ],
         "present": true,
         "start": true
@@ -19,7 +32,7 @@
         }
     },
     "commands": [
-        "docker container create --name nginx -p 80:80 nginx:alpine nginx-debug -g 'daemon off;'",
+        "docker container create --name nginx --label 'com.github.pyinfra.config-hash=72d37ab8f5ea3db48272d045bc10e211bbd628f2b4bab5c54d948b073313c9a3' --network bar --network foo -p 8081:8081 -p 80:80 -v /host/a:/container/a -v /host/b:/container/b -e ENV_A=foo -e ENV_B=bar nginx:alpine nginx-debug -g 'daemon off;'",
         "docker container start nginx"
     ]
 }