salt: Add retry on DynamicClient creation to avoid flaky

CHANGELOG.md

@@ -1,6 +1,12 @@
 # CHANGELOG
 ## Release 124.1.4 (in development)
 
+### Bug fixes
+
+- Fix flaky invalid `HTTPSConnectionPool` exception raised when loading
+  the pillar
+  (PR[#3979](https://github.com/scality/metalk8s/pull/3979))
+
 ## Release 124.1.3
 
 ### Enhancements

buildchain/buildchain/salt_tree.py

@@ -734,6 +734,7 @@ def task(self) -> types.TaskDict:
     Path("salt/_states/metalk8s_package_manager.py"),
     Path("salt/_states/metalk8s_sysctl.py"),
     Path("salt/_states/metalk8s_volumes.py"),
+    Path("salt/_utils/metalk8s_kubernetes.py"),
     Path("salt/_utils/metalk8s_utils.py"),
     Path("salt/_utils/pillar_utils.py"),
     Path("salt/_utils/volume_utils.py"),

salt/_modules/metalk8s_drain.py

@@ -522,9 +522,7 @@ def evict_pod(name, namespace="default", grace_period=1, **kwargs):
 
     kubeconfig, context = __salt__["metalk8s_kubernetes.get_kubeconfig"](**kwargs)
 
-    client = kubernetes.dynamic.DynamicClient(
-        kubernetes.config.new_client_from_config(kubeconfig, context)
-    )
+    client = __utils__["metalk8s_kubernetes.get_client"](kubeconfig, context)
 
     # DynamicClient does not handle Pod eviction, so compute the path manually
     path = (

salt/_modules/metalk8s_kubernetes.py

@@ -22,7 +22,6 @@
 
 try:
     import kubernetes.client as k8s_client
-    import kubernetes
     from kubernetes.dynamic.exceptions import ResourceNotFoundError
     from kubernetes.client.rest import ApiException
 except ImportError:
@@ -149,9 +148,7 @@ def method(
 
         kubeconfig, context = __salt__["metalk8s_kubernetes.get_kubeconfig"](**kwargs)
 
-        client = kubernetes.dynamic.DynamicClient(
-            kubernetes.config.new_client_from_config(kubeconfig, context)
-        )
+        client = __utils__["metalk8s_kubernetes.get_client"](kubeconfig, context)
 
         try:
             api = client.resources.get(
@@ -379,9 +376,7 @@ def list_objects(
     """
     kubeconfig, context = __salt__["metalk8s_kubernetes.get_kubeconfig"](**kwargs)
 
-    client = kubernetes.dynamic.DynamicClient(
-        kubernetes.config.new_client_from_config(kubeconfig, context)
-    )
+    client = __utils__["metalk8s_kubernetes.get_client"](kubeconfig, context)
     try:
         api = client.resources.get(api_version=apiVersion, kind=kind)
     except ResourceNotFoundError as exc:

salt/_modules/metalk8s_kubernetes_utils.py

@@ -12,7 +12,6 @@
 
 MISSING_DEPS = []
 try:
-    import kubernetes
     from kubernetes.client.rest import ApiException
 except ImportError:
     MISSING_DEPS.append("kubernetes")
@@ -85,9 +84,7 @@ def get_version_info(**kwargs):
     kubeconfig, context = get_kubeconfig(**kwargs)
 
     try:
-        client = kubernetes.dynamic.DynamicClient(
-            kubernetes.config.new_client_from_config(kubeconfig, context)
-        )
+        client = __utils__["metalk8s_kubernetes.get_client"](kubeconfig, context)
         return client.version["kubernetes"]
     except (ApiException, HTTPError) as exc:
         raise CommandExecutionError("Failed to get version info") from exc

salt/_utils/metalk8s_kubernetes.py

@@ -0,0 +1,38 @@
+"""Utility methods for MetalK8s Kubernetes modules.
+"""
+
+import time
+
+MISSING_DEPS = []
+
+try:
+    import kubernetes
+except ImportError:
+    MISSING_DEPS.append("kubernetes")
+
+__virtualname__ = "metalk8s_kubernetes"
+
+
+def __virtual__():
+    if MISSING_DEPS:
+        error_msg = "Missing dependencies: {}".format(", ".join(MISSING_DEPS))
+        return False, error_msg
+
+    return __virtualname__
+
+
+def get_client(kubeconfig, context, attempts=5):
+    """
+    Simple wrapper to retry on DynamicClient creation since it
+    may fail from time to time
+    """
+    while True:
+        try:
+            return kubernetes.dynamic.DynamicClient(
+                kubernetes.config.new_client_from_config(kubeconfig, context)
+            )
+        except Exception:  # pylint: disable=broad-except
+            if attempts < 0:
+                raise
+            attempts -= 1
+            time.sleep(5)

salt/tests/unit/modules/test_metalk8s_drain.py

@@ -116,11 +116,15 @@ def _create_mock(*args, **kwargs):
         dynamic_client_mock = MagicMock()
         dynamic_client_mock.request.side_effect = create_mock
 
-        dynamic_mock = MagicMock()
-        dynamic_mock.DynamicClient.return_value = dynamic_client_mock
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ), capture_logs(metalk8s_drain.log, logging.DEBUG) as captured:
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": MagicMock(
+                return_value=dynamic_client_mock
+            )
+        }
+
+        with patch.dict(metalk8s_drain.__utils__, utils_dict), capture_logs(
+            metalk8s_drain.log, logging.DEBUG
+        ) as captured:
             if raises:
                 self.assertRaisesRegex(
                     CommandExecutionError, result, metalk8s_drain.evict_pod, **kwargs

salt/tests/unit/modules/test_metalk8s_kubernetes.py

@@ -37,10 +37,9 @@ def _resources_get_mock(api_version, kind):
     dynamic_client_mock = MagicMock()
     dynamic_client_mock.resources.get.side_effect = _resources_get_mock
 
-    dynamic_mock = MagicMock()
-    dynamic_mock.DynamicClient.return_value = dynamic_client_mock
+    get_client_mock = MagicMock(return_value=dynamic_client_mock)
 
-    return dynamic_mock
+    return get_client_mock
 
 
 class Metalk8sKubernetesTestCase(TestCase, mixins.LoaderModuleMockMixin):
@@ -94,7 +93,7 @@ def test_virtual_success(self):
 
     @parameterized.expand(
         [
-            "kubernetes",
+            ("kubernetes.client", "kubernetes"),
             ("urllib3.exceptions", "urllib3"),
         ]
     )
@@ -140,9 +139,12 @@ def _create_mock(body, **_):
             return obj
 
         create_mock = MagicMock(side_effect=_create_mock)
-        dynamic_mock = _mock_k8s_dynamic(
-            namespaced=namespaced, action="create", mock=create_mock
-        )
+
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": _mock_k8s_dynamic(
+                namespaced=namespaced, action="create", mock=create_mock
+            )
+        }
 
         manifest_read_mock = MagicMock()
         # None = IOError
@@ -157,9 +159,9 @@ def _create_mock(body, **_):
         salt_dict = {
             "metalk8s_kubernetes.read_and_render_yaml_file": manifest_read_mock
         }
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ), patch.dict(metalk8s_kubernetes.__salt__, salt_dict):
+        with patch.dict(metalk8s_kubernetes.__utils__, utils_dict), patch.dict(
+            metalk8s_kubernetes.__salt__, salt_dict
+        ):
             if raises:
                 self.assertRaisesRegex(
                     Exception, result, metalk8s_kubernetes.create_object, **kwargs
@@ -200,9 +202,12 @@ def _delete_mock(name, **_):
             return res
 
         delete_mock = MagicMock(side_effect=_delete_mock)
-        dynamic_mock = _mock_k8s_dynamic(
-            namespaced=namespaced, action="delete", mock=delete_mock
-        )
+
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": _mock_k8s_dynamic(
+                namespaced=namespaced, action="delete", mock=delete_mock
+            )
+        }
 
         manifest_read_mock = MagicMock()
         # None = IOError
@@ -217,9 +222,9 @@ def _delete_mock(name, **_):
         salt_dict = {
             "metalk8s_kubernetes.read_and_render_yaml_file": manifest_read_mock
         }
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ), patch.dict(metalk8s_kubernetes.__salt__, salt_dict):
+        with patch.dict(metalk8s_kubernetes.__utils__, utils_dict), patch.dict(
+            metalk8s_kubernetes.__salt__, salt_dict
+        ):
             if raises:
                 self.assertRaisesRegex(
                     Exception, result, metalk8s_kubernetes.delete_object, **kwargs
@@ -258,9 +263,12 @@ def _replace_mock(body, **_):
             return obj
 
         replace_mock = MagicMock(side_effect=_replace_mock)
-        dynamic_mock = _mock_k8s_dynamic(
-            namespaced=namespaced, action="replace", mock=replace_mock
-        )
+
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": _mock_k8s_dynamic(
+                namespaced=namespaced, action="replace", mock=replace_mock
+            )
+        }
 
         manifest_read_mock = MagicMock()
         # None = IOError
@@ -275,9 +283,9 @@ def _replace_mock(body, **_):
         salt_dict = {
             "metalk8s_kubernetes.read_and_render_yaml_file": manifest_read_mock
         }
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ), patch.dict(metalk8s_kubernetes.__salt__, salt_dict):
+        with patch.dict(metalk8s_kubernetes.__utils__, utils_dict), patch.dict(
+            metalk8s_kubernetes.__salt__, salt_dict
+        ):
             if raises:
                 self.assertRaisesRegex(
                     Exception, result, metalk8s_kubernetes.replace_object, **kwargs
@@ -320,9 +328,12 @@ def _get_mock(name, **_):
             return res
 
         get_mock = MagicMock(side_effect=_get_mock)
-        dynamic_mock = _mock_k8s_dynamic(
-            namespaced=namespaced, action="get", mock=get_mock
-        )
+
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": _mock_k8s_dynamic(
+                namespaced=namespaced, action="get", mock=get_mock
+            )
+        }
 
         manifest_read_mock = MagicMock()
         # None = IOError
@@ -337,9 +348,9 @@ def _get_mock(name, **_):
         salt_dict = {
             "metalk8s_kubernetes.read_and_render_yaml_file": manifest_read_mock
         }
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ), patch.dict(metalk8s_kubernetes.__salt__, salt_dict):
+        with patch.dict(metalk8s_kubernetes.__utils__, utils_dict), patch.dict(
+            metalk8s_kubernetes.__salt__, salt_dict
+        ):
             if raises:
                 self.assertRaisesRegex(
                     Exception, result, metalk8s_kubernetes.get_object, **kwargs
@@ -377,9 +388,12 @@ def _patch_mock(body, **_):
             return res
 
         patch_mock = MagicMock(side_effect=_patch_mock)
-        dynamic_mock = _mock_k8s_dynamic(
-            namespaced=namespaced, action="patch", mock=patch_mock
-        )
+
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": _mock_k8s_dynamic(
+                namespaced=namespaced, action="patch", mock=patch_mock
+            )
+        }
 
         manifest_read_mock = MagicMock()
         # None = IOError
@@ -394,9 +408,9 @@ def _patch_mock(body, **_):
         salt_dict = {
             "metalk8s_kubernetes.read_and_render_yaml_file": manifest_read_mock
         }
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ), patch.dict(metalk8s_kubernetes.__salt__, salt_dict):
+        with patch.dict(metalk8s_kubernetes.__utils__, utils_dict), patch.dict(
+            metalk8s_kubernetes.__salt__, salt_dict
+        ):
             if raises:
                 self.assertRaisesRegex(
                     Exception, result, metalk8s_kubernetes.update_object, **kwargs
@@ -473,13 +487,14 @@ def _list_mock(**_):
             return res
 
         list_mock = MagicMock(side_effect=_list_mock)
-        dynamic_mock = _mock_k8s_dynamic(
-            namespaced=namespaced, action="get", mock=list_mock
-        )
 
-        with patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ):
+        utils_dict = {
+            "metalk8s_kubernetes.get_client": _mock_k8s_dynamic(
+                namespaced=namespaced, action="get", mock=list_mock
+            )
+        }
+
+        with patch.dict(metalk8s_kubernetes.__utils__, utils_dict):
             if raises:
                 self.assertRaisesRegex(
                     Exception, result, metalk8s_kubernetes.list_objects, **kwargs

salt/tests/unit/modules/test_metalk8s_kubernetes_utils.py

@@ -39,7 +39,7 @@ def test_virtual_success(self):
 
     @parameterized.expand(
         [
-            ("kubernetes"),
+            (("kubernetes.client.rest", "kubernetes")),
             (("urllib3.exceptions", "urllib3")),
         ]
     )
@@ -109,16 +109,16 @@ def test_get_version_info(
         dynamic_client_mock = MagicMock()
         dynamic_client_mock.version = {"kubernetes": result}
 
-        dynamic_mock = MagicMock()
-        dynamic_mock.DynamicClient.return_value = dynamic_client_mock
+        get_client_mock = MagicMock(return_value=dynamic_client_mock)
+
+        utils_dict = {"metalk8s_kubernetes.get_client": get_client_mock}
+
         if k8s_connection_raise:
-            dynamic_mock.DynamicClient.side_effect = HTTPError("Failed to connect")
+            get_client_mock.side_effect = HTTPError("Failed to connect")
 
         with patch.object(
             metalk8s_kubernetes_utils, "get_kubeconfig", kubeconfig_mock
-        ), patch("kubernetes.dynamic", dynamic_mock), patch(
-            "kubernetes.config", MagicMock()
-        ):
+        ), patch.dict(metalk8s_kubernetes_utils.__utils__, utils_dict):
             if raises:
                 self.assertRaisesRegex(
                     CommandExecutionError,