From ec63caba7bb1c2959b1d350e8baabded0d4aeae2 Mon Sep 17 00:00:00 2001 From: Erick Daniszewski Date: Mon, 23 Mar 2020 11:32:35 -0400 Subject: [PATCH] feat: allow multiple objects to be defined in loaded manifests --- .../get-all-pods-via-fixture/test_examples.py | 2 +- kubetest/client.py | 164 +++++++++++++++--- kubetest/objects/api_object.py | 59 ++++++- tests/data/multi-obj-manifest.yaml | 53 ++++++ tests/objects/test_api_object.py | 91 ++++++++++ tests/test_manifest.py | 38 ++++ 6 files changed, 377 insertions(+), 30 deletions(-) create mode 100644 tests/data/multi-obj-manifest.yaml create mode 100644 tests/objects/test_api_object.py diff --git a/examples/get-all-pods-via-fixture/test_examples.py b/examples/get-all-pods-via-fixture/test_examples.py index 654f7b6..aef1c0b 100644 --- a/examples/get-all-pods-via-fixture/test_examples.py +++ b/examples/get-all-pods-via-fixture/test_examples.py @@ -5,7 +5,7 @@ @pytest.mark.applymanifest(os.path.join(os.path.dirname(__file__), - 'manifests/deployment-redis.yaml')) + 'manifests/deployment-redis.yaml')) def test_pods_from_deployment_loaded_from_marker(kube): """Get the Pods for a Deployment which is loaded via the kubetest 'applymanifest' marker. diff --git a/kubetest/client.py b/kubetest/client.py index a2614c1..157b854 100644 --- a/kubetest/client.py +++ b/kubetest/client.py @@ -6,7 +6,7 @@ """ import logging -from typing import Dict, Union +from typing import Dict, Optional, Union from kubernetes import client @@ -74,19 +74,33 @@ def refresh(obj: objects.ApiObject) -> None: # ****** Manifest Loaders ****** @staticmethod - def load_clusterrolebinding(path: str) -> objects.ClusterRoleBinding: + def load_clusterrolebinding( + path: str, + name: Optional[str] = None, + ) -> objects.ClusterRoleBinding: """Load a manifest YAML into a ClusterRoleBinding object. Args: path: The path to the ClusterRoleBinding manifest. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The ClusterRoleBinding for the specified manifest. """ log.info(f'loading clusterrolebinding from path: {path}') - return objects.ClusterRoleBinding.load(path) + return objects.ClusterRoleBinding.load(path, name=name) - def load_configmap(self, path: str, set_namespace: bool = True) -> objects.ConfigMap: + def load_configmap( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.ConfigMap: """Load a manifest YAML into a ConfigMap object. By default, this will augment the ConfigMap object with the generated @@ -97,17 +111,28 @@ def load_configmap(self, path: str, set_namespace: bool = True) -> objects.Confi path: The path to the ConfigMap manifest. set_namespace: Enable/disable the automatic augmentation of the ConfigMap namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The ConfigMap for the specified manifest. """ log.info(f'loading configmap from path: {path}') - configmap = objects.ConfigMap.load(path) + configmap = objects.ConfigMap.load(path, name=name) if set_namespace: configmap.namespace = self.namespace return configmap - def load_daemonset(self, path: str, set_namespace: bool = True) -> objects.DaemonSet: + def load_daemonset( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.DaemonSet: """Load a manifest YAML into a DaemonSet object. By default, this will augment the DaemonSet object with the generated test @@ -117,17 +142,28 @@ def load_daemonset(self, path: str, set_namespace: bool = True) -> objects.Daemo path: The path to the DaemonSet manifest. set_namespace: Enable/disable the automatic augmentation of the DaemonSet namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The DaemonSet for the specified manifest. """ log.info(f'loading daemonset from path: {path}') - daemonset = objects.DaemonSet.load(path) + daemonset = objects.DaemonSet.load(path, name=name) if set_namespace: daemonset.namespace = self.namespace return daemonset - def load_deployment(self, path: str, set_namespace: bool = True) -> objects.Deployment: + def load_deployment( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.Deployment: """Load a manifest YAML into a Deployment object. By default, this will augment the Deployment object with the generated @@ -138,17 +174,28 @@ def load_deployment(self, path: str, set_namespace: bool = True) -> objects.Depl path: The path to the Deployment manifest. set_namespace: Enable/disable the automatic augmentation of the Deployment namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The Deployment for the specified manifest. """ log.info(f'loading deployment from path: {path}') - deployment = objects.Deployment.load(path) + deployment = objects.Deployment.load(path, name=name) if set_namespace: deployment.namespace = self.namespace return deployment - def load_pod(self, path: str, set_namespace: bool = True) -> objects.Pod: + def load_pod( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.Pod: """Load a manifest YAML into a Pod object. By default, this will augment the Pod object with the generated test case @@ -158,17 +205,28 @@ def load_pod(self, path: str, set_namespace: bool = True) -> objects.Pod: path: The path to the Pod manifest. set_namespace: Enable/disable the automatic augmentation of the Pod namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The Pod for the specified manifest. """ log.info(f'loading pod from path: {path}') - pod = objects.Pod.load(path) + pod = objects.Pod.load(path, name=name) if set_namespace: pod.namespace = self.namespace return pod - def load_rolebinding(self, path: str, set_namespace: bool = True) -> objects.RoleBinding: + def load_rolebinding( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.RoleBinding: """Load a manifest YAML into a RoleBinding object. By default, this will augment the RoleBinding object with the generated @@ -179,17 +237,28 @@ def load_rolebinding(self, path: str, set_namespace: bool = True) -> objects.Rol path: The path to the RoleBinding manifest. set_namespace: Enable/disable the automatic augmentation of the RoleBinding namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The RoleBinding for the specified manifest. """ log.info(f'loading rolebinding from path: {path}') - rolebinding = objects.RoleBinding.load(path) + rolebinding = objects.RoleBinding.load(path, name=name) if set_namespace: rolebinding.namespace = self.namespace return rolebinding - def load_secret(self, path: str, set_namespace: bool = True) -> objects.Secret: + def load_secret( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.Secret: """Load a manifest YAML into a Secret object. By default, this will augment the Secret object with the generated @@ -200,17 +269,28 @@ def load_secret(self, path: str, set_namespace: bool = True) -> objects.Secret: path: The path to the Secret manifest. set_namespace: Enable/disable the automatic augmentation of the Secret namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The Secret for the specified manifest. """ log.info(f'loading secret from path: {path}') - secret = objects.Secret.load(path) + secret = objects.Secret.load(path, name=name) if set_namespace: secret.namespace = self.namespace return secret - def load_service(self, path: str, set_namespace: bool = True) -> objects.Service: + def load_service( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.Service: """Load a manifest YAML into a Service object. By default, this will augment the Service object with the generated @@ -221,17 +301,28 @@ def load_service(self, path: str, set_namespace: bool = True) -> objects.Service path: The path to the Service manifest. set_namespace: Enable/disable the automatic augmentation of the Service namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The Service for the specified manifest. """ log.info(f'loading service from path: {path}') - service = objects.Service.load(path) + service = objects.Service.load(path, name=name) if set_namespace: service.namespace = self.namespace return service - def load_persistentvolumeclaim(self, path, set_namespace=True) -> objects.PersistentVolumeClaim: + def load_persistentvolumeclaim( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.PersistentVolumeClaim: """Load a manifest YAML into a PersistentVolumeClaim object. By default, this will augment the PersistentVolumeClaim object with @@ -242,18 +333,29 @@ def load_persistentvolumeclaim(self, path, set_namespace=True) -> objects.Persis path (str): The path to the PersistentVolumeClaim manifest. set_namespace (bool): Enable/disable the automatic augmentation of the PersistentVolumeClaim namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: objects.PersistentVolumeClaim: The PersistentVolumeClaim for the specified manifest. """ log.info('loading persistentvolumeclaim from path: %s', path) - persistentvolumeclaim = objects.PersistentVolumeClaim.load(path) + persistentvolumeclaim = objects.PersistentVolumeClaim.load(path, name=name) if set_namespace: persistentvolumeclaim.namespace = self.namespace return persistentvolumeclaim - def load_ingress(self, path, set_namespace=True) -> objects.Ingress: + def load_ingress( + self, + path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.Ingress: """Load a manifest YAML into a Ingress object. By default, this will augment the Ingress object with @@ -264,18 +366,28 @@ def load_ingress(self, path, set_namespace=True) -> objects.Ingress: path (str): The path to the Ingress manifest. set_namespace (bool): Enable/disable the automatic augmentation of the Ingress namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: objects.Ingress: The ingress for the specified manifest. """ log.info('loading ingress from path: %s', path) - ingress = objects.Ingress.load(path) + ingress = objects.Ingress.load(path, name=name) if set_namespace: ingress.namespace = self.namespace return ingress - def load_statefulset(self, path: str, set_namespace: bool = True) -> objects.StatefulSet: + def load_statefulset( + self, path: str, + set_namespace: bool = True, + name: Optional[str] = None, + ) -> objects.StatefulSet: """Load a manifest YAML into a StatefulSet object. By default, this will augment the StatefulSet object with the generated @@ -286,12 +398,18 @@ def load_statefulset(self, path: str, set_namespace: bool = True) -> objects.Sta path: The path to the StatefulSet manifest. set_namespace: Enable/disable the automatic augmentation of the StatefulSet namespace. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The StatefulSet for the specified manifest. """ log.info(f'loading statefulset from path: {path}') - statefulset = objects.StatefulSet.load(path) + statefulset = objects.StatefulSet.load(path, name=name) if set_namespace: statefulset.namespace = self.namespace return statefulset diff --git a/kubetest/objects/api_object.py b/kubetest/objects/api_object.py index 351778e..502066d 100644 --- a/kubetest/objects/api_object.py +++ b/kubetest/objects/api_object.py @@ -2,7 +2,7 @@ import abc import logging -from typing import Union +from typing import Optional, Union from kubernetes import client from kubernetes.client.rest import ApiException @@ -198,7 +198,7 @@ def deleted_fn(): ) @classmethod - def load(cls, path: str) -> 'ApiObject': + def load(cls, path: str, name: Optional[str] = None) -> 'ApiObject': """Load the Kubernetes resource from file. Generally, this is used to load the Kubernetes manifest files @@ -207,18 +207,65 @@ def load(cls, path: str) -> 'ApiObject': Args: path: The path to the YAML config file (manifest) containing the configuration for the resource. + name: The name of the resource to load. If the manifest file + contains a single object definition for the type being + loaded, it is not necessary to specify the name. If the + manifest has multiple definitions containing the same + type, a name is required to differentiate between them. + If no name is specified in such case, an error is raised. Returns: The API object wrapper corresponding to the configuration loaded from manifest YAML file. + + Raises: + ValueError: Multiple objects of the desired type were found in + the manifest file and no name was specified to differentiate between + them. """ objs = load_file(path) - if len(objs) != 1: + + # There is only one object defined in the manifest, so load it. + # If the defined object does not match the type of the class being used + # to load the definition, this will fail. + if len(objs) == 1: + return cls(objs[0]) + + # Otherwise, there are multiple definitions in the manifest. Some of + # these definitions may not match with the type we are trying to load, + # so filter the loaded objects to only those which we care about. + filtered = [] + for o in objs: + if isinstance(o, cls.obj_type): + filtered.append(o) + + if len(filtered) == 0: + raise ValueError( + 'Unable to load resource from file - no resource definitions found ' + f'with type {cls.obj_type}.' + ) + + if len(filtered) == 1: + return cls(filtered[0]) + + # If we get here, there are multiple objects of the same type defined. We + # need to check that a name is provided and return the object whose name + # matches. + if not name: raise ValueError( - 'Unable to load resource from file - multiple resources found ' - 'in specified file.' + 'Unable to load resource from file - multiple resource definitions ' + f'found for {cls.obj_type}, but no name specified to select which one.' ) - return cls(objs[0]) + + for o in filtered: + api_obj = cls(o) + if api_obj.name == name: + return api_obj + + raise ValueError( + 'Unable to load resource from file - multiple resource definitions found ' + f'for {cls.obj_type}, but none match specified name: {name}' + ) @abc.abstractmethod def create(self, namespace: str = None) -> None: diff --git a/tests/data/multi-obj-manifest.yaml b/tests/data/multi-obj-manifest.yaml new file mode 100644 index 0000000..bef1c44 --- /dev/null +++ b/tests/data/multi-obj-manifest.yaml @@ -0,0 +1,53 @@ +# Manifest file used for unit tests which includes one Deployment +# definition and two Service definitions. + +apiVersion: v1 +kind: Service +metadata: + name: service-a +spec: + ports: + - name: http + port: 80 + selector: + deployment: kubetest-test-app + type: ClusterIP +--- +apiVersion: v1 +kind: Service +metadata: + name: service-b +spec: + ports: + - name: http + port: 8080 + selector: + deployment: kubetest-test-app + type: ClusterIP +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: kubetest-test-app + labels: + deployment: kubetest-test-app +spec: + replicas: 1 + selector: + matchLabels: + deployment: kubetest-test-app + template: + metadata: + labels: + deployment: kubetest-test-app + spec: + containers: + - name: busybox + image: busybox + imagePullPolicy: Always + ports: + - name: http + containerPort: 5000 + command: + - 'sleep' + - '3600' diff --git a/tests/objects/test_api_object.py b/tests/objects/test_api_object.py new file mode 100644 index 0000000..1b73a1a --- /dev/null +++ b/tests/objects/test_api_object.py @@ -0,0 +1,91 @@ +"""Unit tests for the kubetest.objects.api_object module.""" + +import os + +import pytest + +from kubetest.objects import ConfigMap, Deployment, Service + + +class TestApiObject: + + def test_load_obj_from_manifest_one_definition(self, manifest_dir): + """Load an object from a manifest file which only defines the definition + for the object. + """ + + obj = Deployment.load( + os.path.join(manifest_dir, 'simple-deployment.yaml'), + ) + + assert isinstance(obj, Deployment) + assert obj.name == 'nginx-deployment' + + def test_load_obj_from_manifest_many_definitions(self, manifest_dir): + """Load an object from a manifest file which defines multiple objects. + + In this case, the manifest only contains a single object definition for + the type that we are trying to load. + """ + + obj = Deployment.load( + os.path.join(manifest_dir, 'multi-obj-manifest.yaml'), + ) + + assert isinstance(obj, Deployment) + assert obj.name == 'kubetest-test-app' + + def test_load_obj_from_manifest_many_definitions_no_identifier(self, manifest_dir): + """Load an object from a manifest file which defines multiple objects. + + In this case, the manifest contains multiple object definitions for + the type that we are trying to load and no identifier is passed to the + load function. + """ + + with pytest.raises(ValueError): + Service.load( + os.path.join(manifest_dir, 'multi-obj-manifest.yaml'), + ) + + def test_load_obj_from_manifest_many_definitions_with_identifier(self, manifest_dir): + """Load an object from a manifest file which defines multiple objects. + + In this case, the manifest contains multiple object definitions for + the type that we are trying to load and a valid identifier is passed to the + load function. + """ + + obj = Service.load( + os.path.join(manifest_dir, 'multi-obj-manifest.yaml'), + name='service-b', + ) + + assert isinstance(obj, Service) + assert obj.name == 'service-b' + + def test_load_obj_from_manifest_many_definitions_no_match(self, manifest_dir): + """Load an object from a manifest file which defines multiple objects. + + In this case, the manifest contains multiple objects, all of which do not match + the type of object we are trying to load. + """ + + with pytest.raises(ValueError): + ConfigMap.load( + os.path.join(manifest_dir, 'multi-obj-manifest.yaml'), + ) + + def test_load_obj_from_manifest_many_definitions_name_no_match(self, manifest_dir): + """Load an object from a manifest file which defines multiple objects. + + In this case, the manifest contains multiple object definitions for + the type that we are trying to load and an identifier which does not match + any of the objects is passed to the load function. + """ + + with pytest.raises(ValueError): + Service.load( + os.path.join(manifest_dir, 'multi-obj-manifest.yaml'), + name='service-c' + ) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 72dc9dd..e039b96 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -315,3 +315,41 @@ def test_invalid_yaml(self, manifest_dir): with pytest.raises(yaml.YAMLError): manifest.load_path(manifest_dir) + + +class TestLoadFile: + """Tests for kubetest.manifest.load_file.""" + + def test_ok_single(self, manifest_dir): + """Load manifest file with single object definition.""" + + objs = manifest.load_file( + os.path.join(manifest_dir, 'simple-deployment.yaml') + ) + + assert len(objs) == 1 + assert isinstance(objs[0], client.models.v1_deployment.V1Deployment) + + def test_ok_multi(self, manifest_dir): + """Load manifest file with multiple object definitions.""" + + objs = manifest.load_file( + os.path.join(manifest_dir, 'multi-obj-manifest.yaml') + ) + + assert len(objs) == 3 + assert isinstance(objs[0], client.models.v1_service.V1Service) + assert isinstance(objs[1], client.models.v1_service.V1Service) + assert isinstance(objs[2], client.models.v1_deployment.V1Deployment) + + def test_no_file(self, manifest_dir): + """Load manifest file which does not exist.""" + + with pytest.raises(FileNotFoundError): + manifest.load_file(os.path.join(manifest_dir, 'file-does-not-exist.yaml')) + + def test_invalid_yaml(self, manifest_dir): + """Load manifest file with invalid YAML.""" + + with pytest.raises(yaml.YAMLError): + manifest.load_file(os.path.join(manifest_dir, 'invalid.yaml'))