From 8650b602091afd3a110d53a9d7610965fa0b9b87 Mon Sep 17 00:00:00 2001 From: Peter Razumovsky Date: Wed, 12 Sep 2018 16:59:31 +0400 Subject: [PATCH] Raise ValueError when command has incorrect type Command arg should be a tuple or a list. Add raising exception instead of omitting it. --- tests/unit/test_service.py | 325 +++++++++++++---------------------- xrally_kubernetes/service.py | 44 ++++- 2 files changed, 151 insertions(+), 218 deletions(-) diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 73e97fe..226fe42 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -547,33 +547,20 @@ def test_create_pod_with_incorrect_command(self): self.k8s_client.generate_random_name = mock.MagicMock() self.k8s_client.generate_random_name.return_value = "name" - self.k8s_client.create_pod( + ex = self.assertRaises( + ValueError, + self.k8s_client.create_pod, image="test/image", namespace="ns", command="ls", - status_wait=False) + status_wait=False + ) - expected = { - "apiVersion": "v1", - "kind": "Pod", - "metadata": { - "name": "name", - "labels": { - "role": "name" - } - }, - "spec": { - "containers": [ - { - "name": "name", - "image": "test/image" - } - ] - } - } - self.client.create_namespaced_pod.assert_called_once_with( - body=expected, - namespace="ns" + self.assertEqual("'command' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_pod.call_count ) def test_create_pod_with_sa(self): @@ -892,47 +879,22 @@ def test_create_replication_controller_with_incorrect_command(self): self.k8s_client.generate_random_name = mock.MagicMock() self.k8s_client.generate_random_name.return_value = "name" - self.k8s_client.create_rc( + ex = self.assertRaises( + ValueError, + self.k8s_client.create_rc, image="test/image", replicas=2, namespace="ns", command="ls", - status_wait=False) + status_wait=False + ) - expected = { - "apiVersion": "v1", - "kind": "ReplicationController", - "metadata": { - "name": "name", - }, - "spec": { - "replicas": 2, - "selector": { - "app": mock.ANY - }, - "template": { - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "containers": [ - { - "name": "name", - "image": "test/image" - } - ] - } - } - } - } - (self.client.create_namespaced_replication_controller - .assert_called_once_with( - body=expected, - namespace="ns" - )) + self.assertEqual("'command' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_replication_controller.call_count + ) def test_create_and_wait_replication_controller_success(self): self.config_cls.reset_mock() @@ -1262,52 +1224,22 @@ def test_create_replicaset_with_incorrect_command(self): self.k8s_client.generate_random_name = mock.MagicMock() self.k8s_client.generate_random_name.return_value = "name" - self.k8s_client.create_replicaset( + ex = self.assertRaises( + ValueError, + self.k8s_client.create_replicaset, image="test/image", replicas=2, namespace="ns", command="ls", - status_wait=False) + status_wait=False + ) - expected = { - "apiVersion": "extensions/v1beta1", - "kind": "ReplicaSet", - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "replicas": 2, - "selector": { - "matchLabels": { - "app": mock.ANY - } - }, - "template": { - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "containers": [ - { - "name": "name", - "image": "test/image" - } - ] - } - } - } - } - (self.client.create_namespaced_replica_set - .assert_called_once_with( - body=expected, - namespace="ns" - )) + self.assertEqual("'command' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_replica_set.call_count + ) def test_create_and_wait_replicaset_success(self): self.config_cls.reset_mock() @@ -2022,47 +1954,72 @@ def test_create_deployment_with_incorrect_command(self): self.k8s_client.generate_random_name = mock.MagicMock() self.k8s_client.generate_random_name.return_value = "name" - self.k8s_client.create_deployment( + ex = self.assertRaises( + ValueError, + self.k8s_client.create_deployment, image="test/image", replicas=2, namespace="ns", command="ls", - status_wait=False) + status_wait=False + ) - expected = { - "apiVersion": "extensions/v1beta1", - "kind": "Deployment", - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "replicas": 2, - "template": { - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "containers": [ - { - "name": "name", - "image": "test/image" - } - ] - } - } - } - } - (self.client.create_namespaced_deployment - .assert_called_once_with( - body=expected, - namespace="ns" - )) + self.assertEqual("'command' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_deployment.call_count + ) + + def test_create_deployment_with_incorrect_env(self): + self.config_cls.reset_mock() + self.api_cls.reset_mock() + self.client_cls.reset_mock() + + self.k8s_client.generate_random_name = mock.MagicMock() + self.k8s_client.generate_random_name.return_value = "name" + ex = self.assertRaises( + ValueError, + self.k8s_client.create_deployment, + image="test/image", + replicas=2, + namespace="ns", + command=["ls"], + env="VAR", + status_wait=False + ) + + self.assertEqual("'env' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_deployment.call_count + ) + + def test_create_deployment_with_incorrect_resources(self): + self.config_cls.reset_mock() + self.api_cls.reset_mock() + self.client_cls.reset_mock() + + self.k8s_client.generate_random_name = mock.MagicMock() + self.k8s_client.generate_random_name.return_value = "name" + ex = self.assertRaises( + ValueError, + self.k8s_client.create_deployment, + image="test/image", + replicas=2, + namespace="ns", + command=["ls"], + resources=("limit", "unlimited"), + status_wait=False + ) + + self.assertEqual("'resources' argument should be dict type, found " + "", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_deployment.call_count + ) def test_create_and_wait_deployment_success(self): self.config_cls.reset_mock() @@ -2626,52 +2583,22 @@ def test_create_statefulset_with_incorrect_command(self): self.k8s_client.generate_random_name = mock.MagicMock() self.k8s_client.generate_random_name.return_value = "name" - self.k8s_client.create_statefulset( + ex = self.assertRaises( + ValueError, + self.k8s_client.create_statefulset, image="test/image", replicas=2, namespace="ns", command="ls", - status_wait=False) + status_wait=False + ) - expected = { - "apiVersion": "apps/v1", - "kind": "StatefulSet", - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "selector": { - "matchLabels": { - "app": mock.ANY - } - }, - "replicas": 2, - "template": { - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "containers": [ - { - "image": "test/image", - "name": "name" - } - ] - } - } - } - } - (self.client.create_namespaced_stateful_set - .assert_called_once_with( - body=expected, - namespace="ns" - )) + self.assertEqual("'command' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_stateful_set.call_count + ) def test_create_and_wait_statefulset_success(self): self.config_cls.reset_mock() @@ -3349,43 +3276,21 @@ def test_create_daemonset_with_incorrect_command(self): self.k8s_client.generate_random_name = mock.MagicMock() self.k8s_client.generate_random_name.return_value = "name" - self.k8s_client.create_daemonset( + ex = self.assertRaises( + ValueError, + self.k8s_client.create_daemonset, image="test/image", namespace="ns", command="ls", node_labels=None, - status_wait=False) - - expected = expected = { - "apiVersion": "extensions/v1beta1", - "kind": "DaemonSet", - "metadata": { - "name": "name" - }, - "spec": { - "template": { - "metadata": { - "name": "name", - "labels": { - "app": mock.ANY - } - }, - "spec": { - "containers": [ - { - "image": "test/image", - "name": "name" - } - ] - } - } - } - } - (self.client.create_namespaced_daemon_set - .assert_called_once_with( - body=expected, - namespace="ns" - )) + status_wait=False + ) + self.assertEqual("'command' argument should be list or tuple type, " + "found ", str(ex)) + self.assertEqual( + 0, + self.client.create_namespaced_daemon_set.call_count + ) def test_create_and_wait_daemonset_success_node_no_filter(self): self.config_cls.reset_mock() diff --git a/xrally_kubernetes/service.py b/xrally_kubernetes/service.py index fcfbad7..4a898f1 100644 --- a/xrally_kubernetes/service.py +++ b/xrally_kubernetes/service.py @@ -387,7 +387,10 @@ def create_pod(self, image, namespace, command=None, volume=None, "name": name, "image": image } - if command is not None and isinstance(command, (list, tuple)): + if command is not None: + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) container_spec["command"] = list(command) if volume and volume.get("mount_path"): container_spec["volumeMounts"] = volume["mount_path"] @@ -504,7 +507,10 @@ def create_rc(self, replicas, image, namespace, command=None, "name": name, "image": image } - if command is not None and isinstance(command, (list, tuple)): + if command is not None: + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) container_spec["command"] = list(command) manifest = { @@ -626,7 +632,10 @@ def create_replicaset(self, namespace, replicas, image, command=None, "name": name, "image": image } - if command is not None and isinstance(command, (list, tuple)): + if command is not None: + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) container_spec["command"] = list(command) manifest = { @@ -745,11 +754,20 @@ def create_deployment(self, namespace, replicas, image, resources=None, "name": name, "image": image } - if command is not None and isinstance(command, (list, tuple)): + if command is not None: + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) container_spec["command"] = list(command) - if env is not None and isinstance(env, (list, tuple)): + if env is not None: + if not isinstance(env, (list, tuple)): + raise ValueError("'env' argument should be list or tuple " + "type, found %s" % type(env)) container_spec["env"] = list(env) - if resources is not None and isinstance(resources, dict): + if resources is not None: + if not isinstance(resources, dict): + raise ValueError("'resources' argument should be dict type, " + "found %s" % type(resources)) container_spec["resources"] = resources manifest = { @@ -912,6 +930,10 @@ def create_job(self, namespace, image, command, name=None, """ name = name or self.generate_random_name() + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) + manifest = { "apiVersion": "batch/v1", "kind": "Job", @@ -1019,7 +1041,10 @@ def create_statefulset(self, namespace, replicas, image, command=None, "name": name, "image": image } - if command is not None and isinstance(command, (list, tuple)): + if command is not None: + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) container_spec["command"] = list(command) manifest = { @@ -1163,7 +1188,10 @@ def create_daemonset(self, image, namespace, command=None, "name": name, "image": image } - if command is not None and isinstance(command, (list, tuple)): + if command is not None: + if not isinstance(command, (list, tuple)): + raise ValueError("'command' argument should be list or tuple " + "type, found %s" % type(command)) container_spec["command"] = list(command) manifest = {