diff --git a/CHANGELOG.md b/CHANGELOG.md index fbcc276d37..939f6d3907 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/buildchain/buildchain/salt_tree.py b/buildchain/buildchain/salt_tree.py index b2e3cbc709..6c416d84d7 100644 --- a/buildchain/buildchain/salt_tree.py +++ b/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"), diff --git a/salt/_modules/metalk8s_drain.py b/salt/_modules/metalk8s_drain.py index 1c243ce7f7..c6421be485 100644 --- a/salt/_modules/metalk8s_drain.py +++ b/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 = ( diff --git a/salt/_modules/metalk8s_kubernetes.py b/salt/_modules/metalk8s_kubernetes.py index af8351fe59..3381bd5b9b 100644 --- a/salt/_modules/metalk8s_kubernetes.py +++ b/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: diff --git a/salt/_modules/metalk8s_kubernetes_utils.py b/salt/_modules/metalk8s_kubernetes_utils.py index e11e63c4b3..2f7454af87 100644 --- a/salt/_modules/metalk8s_kubernetes_utils.py +++ b/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 diff --git a/salt/_utils/metalk8s_kubernetes.py b/salt/_utils/metalk8s_kubernetes.py new file mode 100644 index 0000000000..7c5996de28 --- /dev/null +++ b/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) diff --git a/salt/tests/unit/modules/test_metalk8s_drain.py b/salt/tests/unit/modules/test_metalk8s_drain.py index c4d4261d4b..7d74337dab 100644 --- a/salt/tests/unit/modules/test_metalk8s_drain.py +++ b/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 diff --git a/salt/tests/unit/modules/test_metalk8s_kubernetes.py b/salt/tests/unit/modules/test_metalk8s_kubernetes.py index 05e211fba3..3f00ea6f52 100644 --- a/salt/tests/unit/modules/test_metalk8s_kubernetes.py +++ b/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 diff --git a/salt/tests/unit/modules/test_metalk8s_kubernetes_utils.py b/salt/tests/unit/modules/test_metalk8s_kubernetes_utils.py index 9f7ee05dd0..4910cad530 100644 --- a/salt/tests/unit/modules/test_metalk8s_kubernetes_utils.py +++ b/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,