default to 1.35, fall back to 1.44 (aws#8439)

Author: roger-zhangg

samcli/lib/constants.py

@@ -1 +1,2 @@
-DOCKER_MIN_API_VERSION = "1.44"
+DOCKER_MIN_API_VERSION = "1.35"
+DOCKER_MIN_API_VERSION_FALLBACK = "1.44"

samcli/local/docker/container_client.py

@@ -32,7 +32,7 @@
 import docker
 from docker.utils import kwargs_from_env
 
-from samcli.lib.constants import DOCKER_MIN_API_VERSION
+from samcli.lib.constants import DOCKER_MIN_API_VERSION, DOCKER_MIN_API_VERSION_FALLBACK
 from samcli.local.docker.exceptions import ContainerArchiveImageLoadFailedException, ContainerInvalidSocketPathException
 from samcli.local.docker.platform_config import get_finch_socket_path
 
@@ -62,7 +62,7 @@ class ContainerClient(docker.DockerClient, ABC):
     # Initialize socket_path
     socket_path: Optional[str] = None
 
-    def __init__(self, client_version, base_url=None):
+    def __init__(self, base_url=None):
         """
         Initialize the container client with environment variable processing and overrides.
 
@@ -87,19 +87,20 @@ def __init__(self, client_version, base_url=None):
 
         # Always start with environment variables
         current_env = os.environ.copy()
-        client_params = kwargs_from_env(environment=current_env)
+        self.client_params = kwargs_from_env(environment=current_env)
 
         # Override base_url if explicitly provided
         if base_url is not None:
-            client_params["base_url"] = base_url
+            self.client_params["base_url"] = base_url
 
         # Specify minimum version
-        client_params["version"] = client_version
+        self.client_params["version"] = DOCKER_MIN_API_VERSION
 
         # Initialize DockerClient with processed parameters
-        LOG.debug(f"Creating container client with parameters: {client_params}")
-        super().__init__(**client_params)
+        LOG.debug(f"Creating container client with parameters: {self.client_params}")
+        super().__init__(**self.client_params)
 
+    @abstractmethod
     def is_available(self) -> bool:
         """
         Check if this client instance is available and can connect.
@@ -111,12 +112,7 @@ def is_available(self) -> bool:
         Returns:
             bool: True if the client can successfully connect to the runtime
         """
-        try:
-            self.ping()
-            return True
-        except Exception as e:
-            LOG.debug(f"Container daemon availability check failed: {e}")
-            return False
+        pass
 
     @abstractmethod
     def get_socket_path(self) -> str:
@@ -292,10 +288,27 @@ def __init__(self):
 
         if socket_path:
             LOG.debug(f"Creating Docker container client with base_url={socket_path}.")
-            super().__init__(base_url=socket_path, client_version=DOCKER_MIN_API_VERSION)
+            super().__init__(base_url=socket_path)
         else:
             LOG.debug("Creating Docker container client from environment variable.")
-            super().__init__(client_version=DOCKER_MIN_API_VERSION)
+            super().__init__()
+
+    def is_available(self):
+        try:
+            self.ping()
+            return True
+        except Exception as e:
+            try:
+                # docker engine > 29 requires 1.44 as minimum api version
+                LOG.debug(f"Fall back docker api version to {DOCKER_MIN_API_VERSION_FALLBACK}: {e}")
+                self.client_params["version"] = DOCKER_MIN_API_VERSION_FALLBACK
+                self.api = docker.APIClient(**self.client_params)
+                self.ping()
+                LOG.debug(f"Docker daemon check succeeded with fallback: {e}")
+                return True
+            except Exception as e:
+                LOG.debug(f"Docker daemon availability check failed with fallback: {e}")
+                return False
 
     def get_runtime_type(self) -> str:
         """
@@ -504,9 +517,15 @@ def __init__(self):
             return None
 
         LOG.debug(f"Creating Finch container client with base_url={socket_path}")
-        super().__init__(
-            base_url=socket_path, client_version="1.35"
-        )  # TODO: Placeholder until Finch updates to Docker's min latest version
+        super().__init__(base_url=socket_path)  # TODO: Placeholder until Finch updates to Docker's min latest version
+
+    def is_available(self):
+        try:
+            self.ping()
+            return True
+        except Exception as e:
+            LOG.debug(f"Finch daemon availability check failed: {e}")
+            return False
 
     def get_socket_path(self) -> str:
         """

tests/unit/local/docker/test_container_client.py

@@ -33,7 +33,7 @@ def setUp(self):
         self.finch_socket = "unix:///tmp/finch.sock"
         self.docker_socket = "unix:///var/run/docker.sock"
         self.docker_version = DOCKER_MIN_API_VERSION
-        self.finch_version = "1.35"  # TODO: Update when Finch updates to latest Docker API version
+        self.finch_version = DOCKER_MIN_API_VERSION  # TODO: Update when Finch updates to latest Docker API version
 
     def create_mock_container_client(self, client_class, methods_to_bind=None):
         """Create a mock container client with bound methods for testing."""
@@ -95,6 +95,9 @@ def create_mock_containers(self, container_configs):
 class ConcreteContainerClient(ContainerClient):
     """Concrete implementation of ContainerClient for testing purposes"""
 
+    def is_available(self):
+        return True
+
     def get_socket_path(self):
         return "unix:///var/run/docker.sock"
 
@@ -135,6 +138,7 @@ def test_inherits_from_docker_client(self):
     def test_abstract_methods_defined(self):
         """Test that all required abstract methods are defined"""
         abstract_methods = {
+            "is_available",
             "get_runtime_type",
             "get_socket_path",
             "load_image_from_archive",
@@ -721,7 +725,7 @@ class TestContainerClientBaseInit(BaseContainerClientTestCase):
     def test_init_no_overrides(self, mock_log, mock_docker_init):
         """Test ContainerClient init with no environment overrides"""
         with patch.dict("os.environ", {}, clear=True):
-            client = ConcreteContainerClient(client_version=self.docker_version)
+            client = ConcreteContainerClient()
 
         # Verify DockerClient.__init__ was called with expected parameters
         mock_docker_init.assert_called_once()
@@ -736,7 +740,7 @@ def test_init_with_base_url_override(self, mock_log, mock_docker_init):
         override_url = "unix:///tmp/finch.sock"
 
         with patch.dict("os.environ", {}, clear=True):
-            client = ConcreteContainerClient(client_version=self.docker_version, base_url=override_url)
+            client = ConcreteContainerClient(base_url=override_url)
 
         # Verify DockerClient.__init__ was called with expected parameters
         mock_docker_init.assert_called_once()
@@ -755,26 +759,43 @@ def setUp(self):
         self.client = Mock(spec=ContainerClient)
 
         # Set up the methods we need to test
-        self.client.is_available = ContainerClient.is_available.__get__(self.client)
         self.client.is_finch = ContainerClient.is_finch.__get__(self.client)
         self.client.is_docker = ContainerClient.is_docker.__get__(self.client)
 
         # Set up the mocked docker client attributes
         self.client.ping = Mock(return_value=True)
         self.client.get_runtime_type = Mock(return_value="docker")
 
-    def test_is_available_success(self):
+    def test_is_available_finch_success(self):
+        """Test is_available returns True when ping succeeds"""
+        self.client.ping.return_value = True
+        # check finch
+        self.client.is_available = FinchContainerClient.is_available.__get__(self.client)
+        self.assertTrue(self.client.is_available())
+
+    def test_is_available_docker_success(self):
         """Test is_available returns True when ping succeeds"""
         self.client.ping.return_value = True
+        # check docker
+        self.client.is_available = DockerContainerClient.is_available.__get__(self.client)
         self.assertTrue(self.client.is_available())
 
     @patch("samcli.local.docker.container_client.LOG")
-    def test_is_available_failure(self, mock_log):
+    def test_is_available_finch_failure(self, mock_log):
         """Test is_available returns False when ping fails and logs debug"""
+        self.client.is_available = FinchContainerClient.is_available.__get__(self.client)
         self.client.ping.side_effect = Exception("Connection failed")
         self.assertFalse(self.client.is_available())
         mock_log.debug.assert_called_once()
 
+    @patch("samcli.local.docker.container_client.LOG")
+    def test_is_available_docker_failure(self, mock_log):
+        """Test is_available returns False when ping fails and logs debug"""
+        self.client.is_available = DockerContainerClient.is_available.__get__(self.client)
+        self.client.ping.side_effect = Exception("Connection failed")
+        self.assertFalse(self.client.is_available())
+        mock_log.debug.assert_called()
+
     @parameterized.expand(
         [
             ("is_finch", "finch", True),