From 47a91f4fd77216516007ccc7f71a2297111cf757 Mon Sep 17 00:00:00 2001 From: Kanishk Pachauri Date: Sat, 23 Nov 2024 23:36:23 +0530 Subject: [PATCH 1/5] fix: Enhance environment connection error handling Signed-off-by: Kanishk Pachauri --- podman/client.py | 52 +++++++++++++++++++++-------------- podman/errors/exceptions.py | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 20 deletions(-) diff --git a/podman/client.py b/podman/client.py index 5785f6b9..05b2c163 100644 --- a/podman/client.py +++ b/podman/client.py @@ -19,6 +19,7 @@ from podman.domain.secrets import SecretsManager from podman.domain.system import SystemManager from podman.domain.volumes import VolumesManager +from podman.errors.exceptions import PodmanConnectionError logger = logging.getLogger("podman") @@ -114,27 +115,38 @@ def from_env( Client used to communicate with a Podman service. Raises: - ValueError when required environment variable is not set + PodmanConnectionError: When connection to service fails or environment is invalid """ - environment = environment or os.environ - credstore_env = credstore_env or {} - - if version == "auto": - version = None - - kwargs = { - 'version': version, - 'timeout': timeout, - 'tls': False, - 'credstore_env': credstore_env, - 'max_pool_size': max_pool_size, - } - - host = environment.get("CONTAINER_HOST") or environment.get("DOCKER_HOST") or None - if host is not None: - kwargs['base_url'] = host - - return PodmanClient(**kwargs) + try: + environment = environment or os.environ + credstore_env = credstore_env or {} + + if version == "auto": + version = None + + kwargs = { + 'version': version, + 'timeout': timeout, + 'tls': False, + 'credstore_env': credstore_env, + 'max_pool_size': max_pool_size, + } + + host = environment.get("CONTAINER_HOST") or environment.get("DOCKER_HOST") or None + if host is not None: + kwargs['base_url'] = host + + return PodmanClient(**kwargs) + except Exception as exc: + error_msg = "Failed to initialize Podman client from environment" + if isinstance(exc, ValueError): + error_msg = "Invalid environment configuration for Podman client" + elif isinstance(exc, (ConnectionError, TimeoutError)): + error_msg = "Failed to connect to Podman service" + + raise PodmanConnectionError( + message=error_msg, environment=environment, host=host, original_error=exc + ) from exc @cached_property def containers(self) -> ContainersManager: diff --git a/podman/errors/exceptions.py b/podman/errors/exceptions.py index 7cf5970a..67a269df 100644 --- a/podman/errors/exceptions.py +++ b/podman/errors/exceptions.py @@ -142,3 +142,58 @@ def __init__( class InvalidArgument(PodmanError): """Parameter to method/function was not valid.""" + + +class PodmanConnectionError(PodmanError): + """Exception raised when connection to Podman service fails using environment configuration.""" + + def __init__( + self, + message: str, + environment: Optional[Dict[str, str]] = None, + host: Optional[str] = None, + original_error: Optional[Exception] = None, + ): + """Initialize PodmanConnectionError. + + Args: + message: Description of the error + environment: Environment variables used in connection attempt + host: URL to Podman service that failed + original_error: Original exception that caused this error + """ + super().__init__(message) + self.environment = environment + self.host = host + self.original_error = original_error + + def __str__(self) -> str: + """Format error message with details about connection attempt.""" + msg = [super().__str__()] + + if self.host: + msg.append(f"Host: {self.host}") + + if self.environment: + relevant_vars = { + k: v + for k, v in self.environment.items() + if k + in ( + 'DOCKER_HOST', + 'CONTAINER_HOST', + 'DOCKER_TLS_VERIFY', + 'CONTAINER_TLS_VERIFY', + 'DOCKER_CERT_PATH', + 'CONTAINER_CERT_PATH', + ) + } + if relevant_vars: + msg.append("Environment:") + for key, value in relevant_vars.items(): + msg.append(f" {key}={value}") + + if self.original_error: + msg.append(f"Caused by: {str(self.original_error)}") + + return " | ".join(msg) From e491427eea0809fa547cfb776951fdef7405ad88 Mon Sep 17 00:00:00 2001 From: Kanishk Pachauri Date: Sun, 16 Feb 2025 20:42:24 +0530 Subject: [PATCH 2/5] refactor: improve error handling in Podman client initialization --- podman/client.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/podman/client.py b/podman/client.py index e1e618b7..d91ed246 100644 --- a/podman/client.py +++ b/podman/client.py @@ -137,16 +137,21 @@ def from_env( kwargs['base_url'] = host return PodmanClient(**kwargs) - except Exception as exc: + except ValueError as e: + error_msg = "Invalid environment configuration for Podman client" + raise PodmanConnectionError( + message=error_msg, environment=environment, host=host, original_error=e + ) + except (ConnectionError, TimeoutError) as e: + error_msg = "Failed to connect to Podman service" + raise PodmanConnectionError( + message=error_msg, environment=environment, host=host, original_error=e + ) + except Exception as e: error_msg = "Failed to initialize Podman client from environment" - if isinstance(exc, ValueError): - error_msg = "Invalid environment configuration for Podman client" - elif isinstance(exc, (ConnectionError, TimeoutError)): - error_msg = "Failed to connect to Podman service" - raise PodmanConnectionError( - message=error_msg, environment=environment, host=host, original_error=exc - ) from exc + message=error_msg, environment=environment, host=host, original_error=e + ) @cached_property def containers(self) -> ContainersManager: From 11af35f64c14279016d818d9b98332bcb5391d13 Mon Sep 17 00:00:00 2001 From: Kanishk Pachauri Date: Sun, 16 Feb 2025 20:50:31 +0530 Subject: [PATCH 3/5] fix: typing error --- podman/errors/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/podman/errors/exceptions.py b/podman/errors/exceptions.py index a8f6af74..6bbd5e75 100644 --- a/podman/errors/exceptions.py +++ b/podman/errors/exceptions.py @@ -151,7 +151,7 @@ class PodmanConnectionError(PodmanError): def __init__( self, message: str, - environment: Optional[Dict[str, str]] = None, + environment: Optional[dict[str, str]] = None, host: Optional[str] = None, original_error: Optional[Exception] = None, ): From a95e13bebce30e02c1affab5cb55d173f1892ea1 Mon Sep 17 00:00:00 2001 From: Kanishk Pachauri Date: Sun, 16 Feb 2025 20:58:45 +0530 Subject: [PATCH 4/5] fix: Ensure PodmanClient raises exception on connection failure --- podman/client.py | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/podman/client.py b/podman/client.py index d91ed246..1b4e71fe 100644 --- a/podman/client.py +++ b/podman/client.py @@ -74,6 +74,18 @@ def __init__(self, **kwargs) -> None: api_kwargs["base_url"] = "http+unix://" + path self.api = APIClient(**api_kwargs) + # Check if the connection to the Podman service is successful + try: + SystemManager(client=self.api).version() + except Exception as e: + error_msg = "Failed to connect to Podman service" + raise PodmanConnectionError( + message=error_msg, + environment=os.environ, + host=api_kwargs.get("base_url"), + original_error=e, + ) + def __enter__(self) -> "PodmanClient": return self @@ -125,16 +137,20 @@ def from_env( version = None kwargs = { - 'version': version, - 'timeout': timeout, - 'tls': False, - 'credstore_env': credstore_env, - 'max_pool_size': max_pool_size, + "version": version, + "timeout": timeout, + "tls": False, + "credstore_env": credstore_env, + "max_pool_size": max_pool_size, } - host = environment.get("CONTAINER_HOST") or environment.get("DOCKER_HOST") or None + host = ( + environment.get("CONTAINER_HOST") + or environment.get("DOCKER_HOST") + or None + ) if host is not None: - kwargs['base_url'] = host + kwargs["base_url"] = host return PodmanClient(**kwargs) except ValueError as e: @@ -192,7 +208,9 @@ def secrets(self): def system(self): return SystemManager(client=self.api) - def df(self) -> dict[str, Any]: # pylint: disable=missing-function-docstring,invalid-name + def df( + self, + ) -> dict[str, Any]: # pylint: disable=missing-function-docstring,invalid-name return self.system.df() df.__doc__ = SystemManager.df.__doc__ @@ -234,7 +252,9 @@ def swarm(self): Raises: NotImplemented: Swarm not supported by Podman service """ - raise NotImplementedError("Swarm operations are not supported by Podman service.") + raise NotImplementedError( + "Swarm operations are not supported by Podman service." + ) # Aliases to cover all swarm methods services = swarm From d549745260bb951a5d3432df17e180864f3c0e4e Mon Sep 17 00:00:00 2001 From: Kanishk Pachauri Date: Sun, 16 Feb 2025 21:02:33 +0530 Subject: [PATCH 5/5] test: Add tests for podman connection error --- podman/errors/__init__.py | 2 ++ podman/tests/integration/test_system.py | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/podman/errors/__init__.py b/podman/errors/__init__.py index ae8d9fa0..2e108e96 100644 --- a/podman/errors/__init__.py +++ b/podman/errors/__init__.py @@ -22,6 +22,7 @@ 'NotFoundError', 'PodmanError', 'StreamParseError', + 'PodmanConnectionError' ] try: @@ -34,6 +35,7 @@ NotFound, PodmanError, StreamParseError, + PodmanConnectionError ) except ImportError: pass diff --git a/podman/tests/integration/test_system.py b/podman/tests/integration/test_system.py index bcc38711..9a869847 100644 --- a/podman/tests/integration/test_system.py +++ b/podman/tests/integration/test_system.py @@ -16,7 +16,7 @@ import podman.tests.integration.base as base from podman import PodmanClient -from podman.errors import APIError +from podman.errors import APIError, PodmanConnectionError class SystemIntegrationTest(base.IntegrationTest): @@ -64,3 +64,8 @@ def test_login(self): def test_from_env(self): """integration: from_env() no error""" PodmanClient.from_env() + + def test_from_env_exceptions(self): + """integration: from_env() returns exceptions""" + with self.assertRaises(PodmanConnectionError): + PodmanClient.from_env(base_url="unix:///path/to/nonexistent.sock") \ No newline at end of file