Fixing tests and linting.

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'''