diff --git a/clusterman/kubernetes/util.py b/clusterman/kubernetes/util.py index 0e9253a5f..d88f1b73b 100644 --- a/clusterman/kubernetes/util.py +++ b/clusterman/kubernetes/util.py @@ -30,6 +30,7 @@ from kubernetes.client.models.v1_node_selector_requirement import V1NodeSelectorRequirement from kubernetes.client.models.v1_node_selector_term import V1NodeSelectorTerm from kubernetes.client.models.v1_pod import V1Pod as KubernetesPod +from kubernetes.config.config_exception import ConfigException from clusterman.util import ClustermanResources @@ -53,33 +54,38 @@ class KubeApiClientWrapper: + # CLUSTERMAN-812: By making client a class variable we are avoiding re-creating kubernetes + # client object multiple times ( due to call to reload_state which calls reload_client in return) + _client = None + def __init__(self, kubeconfig_path: str, client_class: Type) -> None: """Init k8s API client :param str kubeconfig_path: k8s configuration path :param Type client_class: k8s client class to initialize """ - try: - """ - https://kubernetes.io/docs/concepts/containers/container-environment/#container-environment - Every pod in k8s gets some default environment variable injected, including KUBERNETES_SERVICE_HOST - which points to default kuberbetes service. We are using this variable to distinguise between - when cluterman is started in a pod vs when it's started on host. For clusterman instances running inside - a k8s cluster, we prioritise using K8s Service account since that let us avoid creating any kubeconfig - in advance. For clusterman CLI invocation we continue using provided KUBECONFIG file - """ - if os.getenv("KUBERNETES_SERVICE_HOST"): - kubernetes.config.load_incluster_config() - else: - kubernetes.config.load_kube_config(kubeconfig_path, context=os.getenv("KUBECONTEXT")) - except TypeError: - error_msg = "Could not load KUBECONFIG; is this running on Kubernetes master?" - if "yelpcorp" in socket.getfqdn(): - error_msg += "\nHint: try using the clusterman-k8s- wrapper script!" - logger.error(error_msg) - raise - - self._client = client_class() + if self._client is None: + try: + """ + https://kubernetes.io/docs/concepts/containers/container-environment/#container-environment + Every pod in k8s gets some default environment variable injected, including KUBERNETES_SERVICE_HOST + which points to default kuberbetes service. We are using this variable to distinguise between + when cluterman is started in a pod vs when it's started on host. For clusterman instances running inside + a k8s cluster, we prioritise using K8s Service account since that let us avoid creating any kubeconfig + in advance. For clusterman CLI invocation we continue using provided KUBECONFIG file + """ + if os.getenv("KUBERNETES_SERVICE_HOST"): + kubernetes.config.load_incluster_config() + else: + kubernetes.config.load_kube_config(kubeconfig_path, context=os.getenv("KUBECONTEXT")) + except (TypeError, ConfigException): + error_msg = "Could not load KUBECONFIG; is this running on Kubernetes master?" + if "yelpcorp" in socket.getfqdn(): + error_msg += "\nHint: try using the clusterman-k8s- wrapper script!" + logger.error(error_msg) + raise + + KubeApiClientWrapper._client = client_class() def __getattr__(self, attr): return getattr(self._client, attr) diff --git a/tests/kubernetes/util_test.py b/tests/kubernetes/util_test.py index 3316a4660..a39e24677 100644 --- a/tests/kubernetes/util_test.py +++ b/tests/kubernetes/util_test.py @@ -6,7 +6,7 @@ from kubernetes.client.models.v1_node_selector_requirement import V1NodeSelectorRequirement from kubernetes.client.models.v1_node_selector_term import V1NodeSelectorTerm -from clusterman.kubernetes.util import CachedCoreV1Api +from clusterman.kubernetes.util import CachedCoreV1Api, KubeApiClientWrapper from clusterman.kubernetes.util import ConciseCRDApi from clusterman.kubernetes.util import get_node_kernel_version from clusterman.kubernetes.util import get_node_lsbrelease @@ -16,6 +16,8 @@ @pytest.fixture def mock_cached_core_v1_api(): + # Make sure we always reset the client before each test + KubeApiClientWrapper._client = None with mock.patch("clusterman.kubernetes.util.kubernetes"): yield CachedCoreV1Api("/foo/bar/admin.conf") @@ -41,6 +43,16 @@ def test_cached_corev1_api_use_load_kubeconfig_config_when_running_as_cli(): assert mock_load_kube_config.called +def test_client_initialization_happen_only_once(): + with mock.patch.dict(os.environ, {"KUBERNETES_SERVICE_HOST": "ABC"}): + with mock.patch( + "clusterman.kubernetes.util.kubernetes.config.load_incluster_config" + ) as mock_load_incluster_config: + _ = CachedCoreV1Api("/foo/bar/admin.conf") + _ = CachedCoreV1Api("/foo/bar/admin1.conf") + assert mock_load_incluster_config.call_count == 1 + + def test_cached_corev1_api_caches_non_cached_function(mock_cached_core_v1_api): mock_cached_core_v1_api.list_namespace() assert mock_cached_core_v1_api._client.list_namespace.call_count == 1