From 453b56bf074cf9acbc71bc8626c4688e17020cd9 Mon Sep 17 00:00:00 2001 From: Kenny Woodson Date: Tue, 24 Jan 2017 17:08:31 -0500 Subject: Fixing tests and linting. --- roles/lib_openshift/library/oc_scale.py | 36 +++++++++------- roles/lib_openshift/src/class/oc_scale.py | 4 ++ roles/lib_openshift/src/doc/scale | 4 +- roles/lib_openshift/src/lib/deploymentconfig.py | 26 +++++------ .../lib_openshift/src/lib/replicationcontroller.py | 4 +- .../src/test/integration/oc_scale.yml | 48 ++++++++++----------- roles/lib_openshift/src/test/unit/oc_scale.py | 35 ++------------- roles/lib_openshift/src/test/unit/oc_version.py | 50 +++++++++++----------- 8 files changed, 96 insertions(+), 111 deletions(-) diff --git a/roles/lib_openshift/library/oc_scale.py b/roles/lib_openshift/library/oc_scale.py index 39afa6ba8..4e6717ec0 100644 --- a/roles/lib_openshift/library/oc_scale.py +++ b/roles/lib_openshift/library/oc_scale.py @@ -49,10 +49,10 @@ description: options: state: description: - - State represents whether to create, modify, delete, or list + - State represents whether to scale or list the current replicas required: true default: present - choices: ["present", "absent", "list"] + choices: ["present", "list"] aliases: [] kubeconfig: description: @@ -1191,6 +1191,7 @@ class OpenShiftCLIConfig(object): return rval + # pylint: disable=too-many-public-methods class DeploymentConfig(Yedit): ''' Class to wrap the oc command line tools ''' @@ -1251,7 +1252,7 @@ spec: volume_mounts_path = "spec.template.spec.containers[0].volumeMounts" def __init__(self, content=None): - ''' Constructor for OpenshiftOC ''' + ''' Constructor for deploymentconfig ''' if not content: content = DeploymentConfig.default_deployment_config @@ -1401,7 +1402,7 @@ spec: exist_volumes = self.get_volumes() del_idx = None for idx, exist_volume in enumerate(exist_volumes): - if exist_volume.has_key('name') and exist_volume['name'] == volume['name']: + if 'name' in exist_volume and exist_volume['name'] == volume['name']: del_idx = idx break @@ -1411,7 +1412,7 @@ spec: del_idx = None for idx, exist_volume_mount in enumerate(exist_volume_mounts): - if exist_volume_mount.has_key('name') and exist_volume_mount['name'] == volume['name']: + if 'name' in exist_volume_mount and exist_volume_mount['name'] == volume['name']: del_idx = idx break @@ -1478,7 +1479,7 @@ spec: # update the volume mount for exist_vol_mount in exist_volume_mounts: if exist_vol_mount['name'] == volume_mount['name']: - if exist_vol_mount.has_key('mountPath') and \ + if 'mountPath' in exist_vol_mount and \ str(exist_vol_mount['mountPath']) != str(volume_mount['mountPath']): exist_vol_mount['mountPath'] = volume_mount['mountPath'] modified = True @@ -1497,27 +1498,27 @@ spec: results = [] results.append(exist_volume['name'] == volume['name']) - if volume.has_key('secret'): - results.append(exist_volume.has_key('secret')) + if 'secret' in volume: + results.append('secret' in exist_volume) results.append(exist_volume['secret']['secretName'] == volume['secret']['secretName']) results.append(exist_volume_mount['name'] == volume_mount['name']) results.append(exist_volume_mount['mountPath'] == volume_mount['mountPath']) - elif volume.has_key('emptyDir'): + elif 'emptyDir' in volume: results.append(exist_volume_mount['name'] == volume['name']) results.append(exist_volume_mount['mountPath'] == volume_mount['mountPath']) - elif volume.has_key('persistentVolumeClaim'): + elif 'persistentVolumeClaim' in volume: pvc = 'persistentVolumeClaim' - results.append(exist_volume.has_key(pvc)) + results.append(pvc in exist_volume) if results[-1]: results.append(exist_volume[pvc]['claimName'] == volume[pvc]['claimName']) - if volume[pvc].has_key('claimSize'): + if 'claimSize' in volume[pvc]: results.append(exist_volume[pvc]['claimSize'] == volume[pvc]['claimSize']) - elif volume.has_key('hostpath'): - results.append(exist_volume.has_key('hostPath')) + elif 'hostpath' in volume: + results.append('hostPath' in exist_volume) results.append(exist_volume['hostPath']['path'] == volume_mount['mountPath']) return not all(results) @@ -1527,6 +1528,7 @@ spec: current_reps = self.get(DeploymentConfig.replicas_path) return not current_reps == replicas + # pylint: disable=too-many-public-methods class ReplicationController(DeploymentConfig): ''' Class to wrap the oc command line tools ''' @@ -1537,7 +1539,7 @@ class ReplicationController(DeploymentConfig): volume_mounts_path = "spec.template.spec.containers[0].volumeMounts" def __init__(self, content): - ''' Constructor for OpenshiftOC ''' + ''' Constructor for ReplicationController ''' super(ReplicationController, self).__init__(content=content) # pylint: disable=too-many-instance-attributes @@ -1580,9 +1582,13 @@ class OCScale(OpenShiftCLI): vol = self._get(self.kind, self.name) if vol['returncode'] == 0: if self.kind == 'dc': + # The resource returned from a query could be an rc or dc. + # pylint: disable=redefined-variable-type self.resource = DeploymentConfig(content=vol['results'][0]) vol['results'] = [self.resource.get_replicas()] if self.kind == 'rc': + # The resource returned from a query could be an rc or dc. + # pylint: disable=redefined-variable-type self.resource = ReplicationController(content=vol['results'][0]) vol['results'] = [self.resource.get_replicas()] diff --git a/roles/lib_openshift/src/class/oc_scale.py b/roles/lib_openshift/src/class/oc_scale.py index 68dc6ffd4..19fba3af5 100644 --- a/roles/lib_openshift/src/class/oc_scale.py +++ b/roles/lib_openshift/src/class/oc_scale.py @@ -41,9 +41,13 @@ class OCScale(OpenShiftCLI): vol = self._get(self.kind, self.name) if vol['returncode'] == 0: if self.kind == 'dc': + # The resource returned from a query could be an rc or dc. + # pylint: disable=redefined-variable-type self.resource = DeploymentConfig(content=vol['results'][0]) vol['results'] = [self.resource.get_replicas()] if self.kind == 'rc': + # The resource returned from a query could be an rc or dc. + # pylint: disable=redefined-variable-type self.resource = ReplicationController(content=vol['results'][0]) vol['results'] = [self.resource.get_replicas()] diff --git a/roles/lib_openshift/src/doc/scale b/roles/lib_openshift/src/doc/scale index b2ffc77f9..6427d38af 100644 --- a/roles/lib_openshift/src/doc/scale +++ b/roles/lib_openshift/src/doc/scale @@ -10,10 +10,10 @@ description: options: state: description: - - State represents whether to create, modify, delete, or list + - State represents whether to scale or list the current replicas required: true default: present - choices: ["present", "absent", "list"] + choices: ["present", "list"] aliases: [] kubeconfig: description: diff --git a/roles/lib_openshift/src/lib/deploymentconfig.py b/roles/lib_openshift/src/lib/deploymentconfig.py index c0e9af0a1..e060d3707 100644 --- a/roles/lib_openshift/src/lib/deploymentconfig.py +++ b/roles/lib_openshift/src/lib/deploymentconfig.py @@ -1,4 +1,6 @@ # pylint: skip-file +# flake8: noqa + # pylint: disable=too-many-public-methods class DeploymentConfig(Yedit): @@ -60,7 +62,7 @@ spec: volume_mounts_path = "spec.template.spec.containers[0].volumeMounts" def __init__(self, content=None): - ''' Constructor for OpenshiftOC ''' + ''' Constructor for deploymentconfig ''' if not content: content = DeploymentConfig.default_deployment_config @@ -210,7 +212,7 @@ spec: exist_volumes = self.get_volumes() del_idx = None for idx, exist_volume in enumerate(exist_volumes): - if exist_volume.has_key('name') and exist_volume['name'] == volume['name']: + if 'name' in exist_volume and exist_volume['name'] == volume['name']: del_idx = idx break @@ -220,7 +222,7 @@ spec: del_idx = None for idx, exist_volume_mount in enumerate(exist_volume_mounts): - if exist_volume_mount.has_key('name') and exist_volume_mount['name'] == volume['name']: + if 'name' in exist_volume_mount and exist_volume_mount['name'] == volume['name']: del_idx = idx break @@ -287,7 +289,7 @@ spec: # update the volume mount for exist_vol_mount in exist_volume_mounts: if exist_vol_mount['name'] == volume_mount['name']: - if exist_vol_mount.has_key('mountPath') and \ + if 'mountPath' in exist_vol_mount and \ str(exist_vol_mount['mountPath']) != str(volume_mount['mountPath']): exist_vol_mount['mountPath'] = volume_mount['mountPath'] modified = True @@ -306,27 +308,27 @@ spec: results = [] results.append(exist_volume['name'] == volume['name']) - if volume.has_key('secret'): - results.append(exist_volume.has_key('secret')) + if 'secret' in volume: + results.append('secret' in exist_volume) results.append(exist_volume['secret']['secretName'] == volume['secret']['secretName']) results.append(exist_volume_mount['name'] == volume_mount['name']) results.append(exist_volume_mount['mountPath'] == volume_mount['mountPath']) - elif volume.has_key('emptyDir'): + elif 'emptyDir' in volume: results.append(exist_volume_mount['name'] == volume['name']) results.append(exist_volume_mount['mountPath'] == volume_mount['mountPath']) - elif volume.has_key('persistentVolumeClaim'): + elif 'persistentVolumeClaim' in volume: pvc = 'persistentVolumeClaim' - results.append(exist_volume.has_key(pvc)) + results.append(pvc in exist_volume) if results[-1]: results.append(exist_volume[pvc]['claimName'] == volume[pvc]['claimName']) - if volume[pvc].has_key('claimSize'): + if 'claimSize' in volume[pvc]: results.append(exist_volume[pvc]['claimSize'] == volume[pvc]['claimSize']) - elif volume.has_key('hostpath'): - results.append(exist_volume.has_key('hostPath')) + elif 'hostpath' in volume: + results.append('hostPath' in exist_volume) results.append(exist_volume['hostPath']['path'] == volume_mount['mountPath']) return not all(results) diff --git a/roles/lib_openshift/src/lib/replicationcontroller.py b/roles/lib_openshift/src/lib/replicationcontroller.py index 7dafc60f1..ae585a986 100644 --- a/roles/lib_openshift/src/lib/replicationcontroller.py +++ b/roles/lib_openshift/src/lib/replicationcontroller.py @@ -1,4 +1,6 @@ # pylint: skip-file +# flake8: noqa + # pylint: disable=too-many-public-methods class ReplicationController(DeploymentConfig): @@ -10,5 +12,5 @@ class ReplicationController(DeploymentConfig): volume_mounts_path = "spec.template.spec.containers[0].volumeMounts" def __init__(self, content): - ''' Constructor for OpenshiftOC ''' + ''' Constructor for ReplicationController ''' super(ReplicationController, self).__init__(content=content) diff --git a/roles/lib_openshift/src/test/integration/oc_scale.yml b/roles/lib_openshift/src/test/integration/oc_scale.yml index ccc3d05de..e96e16820 100755 --- a/roles/lib_openshift/src/test/integration/oc_scale.yml +++ b/roles/lib_openshift/src/test/integration/oc_scale.yml @@ -5,30 +5,30 @@ gather_facts: no user: root tasks: -# - name: list oc scale for default router dc -# oc_scale: -# state: list -# name: router -# namespace: default -# kind: dc -# register: scaleout -# - debug: var=scaleout -# -# - assert: -# that: -# - "'result' in scaleout" -# - scaleout.result > 0 -# msg: "Did not find 'result' in returned value or result not > 0." -# -# - name: get the rc for router -# oc_obj: -# state: list -# kind: dc -# namespace: default -# selector: router=router -# register: rcout -# - debug: -# msg: "{{ rcout.results.results[0]['items'][-1]['metadata']['name'] }}" + - name: list oc scale for default router dc + oc_scale: + state: list + name: router + namespace: default + kind: dc + register: scaleout + - debug: var=scaleout + + - assert: + that: + - "'result' in scaleout" + - scaleout.result > 0 + msg: "Did not find 'result' in returned value or result not > 0." + + - name: get the rc for router + oc_obj: + state: list + kind: dc + namespace: default + selector: router=router + register: rcout + - debug: + msg: "{{ rcout.results.results[0]['items'][-1]['metadata']['name'] }}" - name: scale dc to 1 oc_scale: diff --git a/roles/lib_openshift/src/test/unit/oc_scale.py b/roles/lib_openshift/src/test/unit/oc_scale.py index 02f96c165..c523592de 100755 --- a/roles/lib_openshift/src/test/unit/oc_scale.py +++ b/roles/lib_openshift/src/test/unit/oc_scale.py @@ -10,10 +10,10 @@ # # OK -import mock import os import sys import unittest +import mock # Removing invalid variable names for tests so that I can # keep them brief @@ -26,25 +26,6 @@ sys.path.insert(0, module_path) from oc_scale import OCScale # noqa: E402 -# pylint: disable=unused-argument -def oc_cmd_mock(cmd, oadm=False, output=False, output_type='json', input_data=None): - '''mock command for openshift_cmd''' - version = '''oc v3.4.0.39 -kubernetes v1.4.0+776c994 -features: Basic-Auth GSSAPI Kerberos SPNEGO - -Server https://internal.api.opstest.openshift.com -openshift v3.4.0.39 -kubernetes v1.4.0+776c994 -''' - if 'version' in cmd: - return {'stderr': None, - 'stdout': version, - 'returncode': 0, - 'results': version, - 'cmd': cmd} - - class OCScaleTest(unittest.TestCase): ''' Test class for OCVersion @@ -65,7 +46,6 @@ class OCScaleTest(unittest.TestCase): 'kubeconfig': '/etc/origin/master/admin.kubeconfig', 'debug': False} - dc = '''{"kind": "DeploymentConfig", "apiVersion": "v1", "metadata": { @@ -85,13 +65,10 @@ class OCScaleTest(unittest.TestCase): } }''' - mock_openshift_cmd.side_effect = [ {"cmd": '/usr/bin/oc get dc router -n default', 'results': dc, - 'returncode': 0, - } - ] + 'returncode': 0}] results = OCScale.run_ansible(params, False) @@ -109,7 +86,6 @@ class OCScaleTest(unittest.TestCase): 'kubeconfig': '/etc/origin/master/admin.kubeconfig', 'debug': False} - dc = '''{"kind": "DeploymentConfig", "apiVersion": "v1", "metadata": { @@ -129,16 +105,13 @@ class OCScaleTest(unittest.TestCase): } }''' - mock_openshift_cmd.side_effect = [ {"cmd": '/usr/bin/oc get dc router -n default', 'results': dc, - 'returncode': 0, - }, + 'returncode': 0}, {"cmd": '/usr/bin/oc create -f /tmp/router -n default', 'results': '', - 'returncode': 0, - }, + 'returncode': 0} ] results = OCScale.run_ansible(params, False) diff --git a/roles/lib_openshift/src/test/unit/oc_version.py b/roles/lib_openshift/src/test/unit/oc_version.py index 8d9128187..f927948be 100755 --- a/roles/lib_openshift/src/test/unit/oc_version.py +++ b/roles/lib_openshift/src/test/unit/oc_version.py @@ -13,6 +13,7 @@ import os import sys import unittest +import mock # Removing invalid variable names for tests so that I can # keep them brief @@ -25,25 +26,6 @@ sys.path.insert(0, module_path) from oc_version import OCVersion # noqa: E402 -# pylint: disable=unused-argument -def oc_cmd_mock(cmd, oadm=False, output=False, output_type='json', input_data=None): - '''mock command for openshift_cmd''' - version = '''oc v3.4.0.39 -kubernetes v1.4.0+776c994 -features: Basic-Auth GSSAPI Kerberos SPNEGO - -Server https://internal.api.opstest.openshift.com -openshift v3.4.0.39 -kubernetes v1.4.0+776c994 -''' - if 'version' in cmd: - return {'stderr': None, - 'stdout': version, - 'returncode': 0, - 'results': version, - 'cmd': cmd} - - class OCVersionTest(unittest.TestCase): ''' Test class for OCVersion @@ -51,15 +33,31 @@ class OCVersionTest(unittest.TestCase): def setUp(self): ''' setup method will create a file and set to known configuration ''' - self.oc_ver = OCVersion(None, False) - self.oc_ver.openshift_cmd = oc_cmd_mock + pass - def test_get(self): + @mock.patch('oc_version.OCVersion.openshift_cmd') + def test_get(self, mock_openshift_cmd): ''' Testing a get ''' - results = self.oc_ver.get() - self.assertEqual(results['oc_short'], '3.4') - self.assertEqual(results['oc_numeric'], '3.4.0.39') - self.assertEqual(results['kubernetes_numeric'], '1.4.0') + params = {'kubeconfig': '/etc/origin/master/admin.kubeconfig', + 'state': 'list', + 'debug': False} + + mock_openshift_cmd.side_effect = [ + {"cmd": "oc version", + "results": "oc v3.4.0.39\nkubernetes v1.4.0+776c994\n" + + "features: Basic-Auth GSSAPI Kerberos SPNEGO\n\n" + + "Server https://internal.api.opstest.openshift.com" + + "openshift v3.4.0.39\n" + + "kubernetes v1.4.0+776c994\n", + "returncode": 0} + ] + + results = OCVersion.run_ansible(params) + + self.assertFalse(results['changed']) + self.assertEqual(results['results']['oc_short'], '3.4') + self.assertEqual(results['results']['oc_numeric'], '3.4.0.39') + self.assertEqual(results['results']['kubernetes_numeric'], '1.4.0') def tearDown(self): '''TearDown method''' -- cgit v1.2.3