refactor: move hash generation into if clause

Author: matthiasschaub

core/testcontainers/core/container.py

@@ -99,18 +99,6 @@ def start(self) -> Self:
         logger.info("Pulling image %s", self.image)
         self._configure()
 
-        # container hash consisting of run arguments
-        args = (
-            self.image,
-            self._command,
-            self.env,
-            self.ports,
-            self._name,
-            self.volumes,
-            str(tuple(sorted(self._kwargs.items()))),
-        )
-        hash_ = hashlib.sha256(bytes(str(args), encoding="utf-8")).hexdigest()
-
         if self._reuse and not c.tc_properties_testcontainers_reuse_enable:
             logging.warning(
                 "Reuse was requested (`with_reuse`) but the environment does not "
@@ -119,24 +107,36 @@ def start(self) -> Self:
             )
 
         if self._reuse and c.tc_properties_testcontainers_reuse_enable:
+            # NOTE: ideally the docker client would return the full container create
+            # request which could be used to generate the hash.
+            args = [  # Docker run arguments
+                self.image,
+                self._command,
+                self.env,
+                self.ports,
+                self._name,
+                self.volumes,
+                str(tuple(sorted(self._kwargs.values()))),
+            ]
+            hash_ = hashlib.sha256(bytes(str(args), encoding="utf-8")).hexdigest()
             docker_client = self.get_docker_client()
             container = docker_client.find_container_by_hash(hash_)
             if container:
                 if container.status != "running":
                     container.start()
                     logger.info("Existing container started: %s", container.id)
-                logger.info("Container is already running: %s", container.id)
                 self._container = container
+                logger.info("Container is already running: %s", container.id)
             else:
                 self._start(hash_)
         else:
-            self._start(hash_)
+            self._start()
 
         if self._network:
             self._network.connect(self._container.id, self._network_aliases)
         return self
 
-    def _start(self, hash_):
+    def _start(self, hash_=None):
         docker_client = self.get_docker_client()
         self._container = docker_client.run(
             self.image,
@@ -146,7 +146,7 @@ def _start(self, hash_):
             ports=self.ports,
             name=self._name,
             volumes=self.volumes,
-            labels={"hash": hash_},
+            labels={"hash": hash_} if hash is not None else {},
             **self._kwargs,
         )
         logger.info("Container started: %s", self._container.short_id)

core/tests/test_reusable_containers.py

@@ -83,13 +83,12 @@ def test_docker_container_with_reuse_reuse_enabled(monkeypatch):
     container.stop()
 
 
-def test_docker_container_with_reuse_reuse_enabled_ryuk_disabled_same_id(monkeypatch):
+def test_docker_container_with_reuse_reuse_enabled_same_id(monkeypatch):
     # Make sure Ryuk cleanup is not active from previous test runs
     Reaper.delete_instance()
 
     tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"}
     monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock)
-    monkeypatch.setattr(testcontainers_config, "ryuk_disabled", True)
 
     container_1 = DockerContainer("hello-world").with_reuse().start()
     id_1 = container_1._container.id
@@ -102,8 +101,16 @@ def test_docker_container_with_reuse_reuse_enabled_ryuk_disabled_same_id(monkeyp
     # container_2.stop() is not needed since it is the same as container_1
 
 
-def test_docker_container_labels_hash():
-    expected_hash = "91fde3c09244e1d3ec6f18a225b9261396b9a1cb0f6365b39b9795782817c128"
+def test_docker_container_labels_hash_default():
+    # w/out reuse
+    with DockerContainer("hello-world") as container:
+        assert container._container.labels["hash"] == ""
+
+
+def test_docker_container_labels_hash(monkeypatch):
+    tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"}
+    monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock)
+    expected_hash = "1bade17a9d8236ba71ffbb676f2ece3fb419ea0e6adb5f82b5a026213c431d8e"
     with DockerContainer("hello-world").with_reuse() as container:
         assert container._container.labels["hash"] == expected_hash
 
@@ -113,7 +120,9 @@ def test_docker_client_find_container_by_hash_not_existing():
         assert DockerClient().find_container_by_hash("foo") == None
 
 
-def test_docker_client_find_container_by_hash_existing():
+def test_docker_client_find_container_by_hash_existing(monkeypatch):
+    tc_properties_mock = testcontainers_config.tc_properties | {"testcontainers.reuse.enable": "true"}
+    monkeypatch.setattr(testcontainers_config, "tc_properties", tc_properties_mock)
     with DockerContainer("hello-world").with_reuse() as container:
         hash_ = container._container.labels["hash"]
         found_container = DockerClient().find_container_by_hash(hash_)