From eda1dc8de4984dbd7e9ee3ad0cf3184468b89b6f Mon Sep 17 00:00:00 2001
From: Al Bailey <Al.Bailey@windriver.com>
Date: Thu, 21 Nov 2019 14:57:31 -0600
Subject: [PATCH] Update sysinv flake8 to use python3

Running flake8 in python3 exposes bugs that will
impact the conversion to python3.

This also adjusts the unit tests to run py36
rather than py35.

Some of the notable issues are:
 - cmp operator has been deprecated
 - file operator has been deprecated
    (replaced by call to open)

The behaviour in python2 has not changed.

For python3 unit tests should be added for:
 -  sysinv/agent/lldp/plugin.py  (Key)
sort in python3 expects a different method (__lt__)

 -  sysinv/conductor/manager.py (evaluate_app_reapply)
The contents from file open when passed to the md5 routine
should fail unless opened with utf-8 encoding
ie:
TypeError: Unicode-objects must be encoded before hashing

Change-Id: Ifcec7d5539cba2350649f56057db0f5e78ef279d
Story: 2007082
Task: 38130
Signed-off-by: Al Bailey <Al.Bailey@windriver.com>
---
 .zuul.yaml                                    | 12 ++---
 sysinv/sysinv/sysinv/sysinv/agent/disk.py     |  4 +-
 .../sysinv/sysinv/sysinv/agent/lldp/plugin.py |  6 +--
 sysinv/sysinv/sysinv/sysinv/agent/manager.py  |  2 +-
 .../sysinv/api/controllers/v1/profile.py      |  4 +-
 sysinv/sysinv/sysinv/sysinv/common/ceph.py    |  4 +-
 .../sysinv/sysinv/common/service_parameter.py |  2 +-
 sysinv/sysinv/sysinv/sysinv/common/utils.py   |  2 +-
 sysinv/sysinv/sysinv/sysinv/conductor/ceph.py |  2 +-
 .../sysinv/sysinv/conductor/kube_app.py       |  6 +--
 .../sysinv/sysinv/sysinv/conductor/manager.py |  2 +-
 .../sysinv/openstack/common/rpc/impl_kombu.py |  4 +-
 sysinv/sysinv/sysinv/test-requirements.txt    | 11 ++---
 sysinv/sysinv/sysinv/tox.ini                  | 44 ++++++++++++++-----
 14 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/.zuul.yaml b/.zuul.yaml
index 28cc0e0180..61555c1447 100644
--- a/.zuul.yaml
+++ b/.zuul.yaml
@@ -8,7 +8,7 @@
       jobs:
         - openstack-tox-linters
         - sysinv-tox-py27
-        - sysinv-tox-py35
+        - sysinv-tox-py36
         - sysinv-tox-flake8
         - sysinv-tox-pylint
         - sysinv-tox-bandit
@@ -22,7 +22,7 @@
       jobs:
         - openstack-tox-linters
         - sysinv-tox-py27
-        - sysinv-tox-py35
+        - sysinv-tox-py36
         - sysinv-tox-flake8
         - sysinv-tox-pylint
         - sysinv-tox-bandit
@@ -50,11 +50,11 @@
       tox_extra_args: -c sysinv/sysinv/sysinv/tox.ini
 
 - job:
-    name: sysinv-tox-py35
+    name: sysinv-tox-py36
     parent: tox
     description: |
-      Run py35 test for sysinv
-    nodeset: ubuntu-xenial
+      Run py36 test for sysinv
+    nodeset: ubuntu-bionic
     required-projects:
       - starlingx/fault
       - starlingx/update
@@ -62,7 +62,7 @@
     files:
       - sysinv/sysinv/*
     vars:
-      tox_envlist: py35
+      tox_envlist: py36
       tox_extra_args: -c sysinv/sysinv/sysinv/tox.ini
 
 - job:
diff --git a/sysinv/sysinv/sysinv/sysinv/agent/disk.py b/sysinv/sysinv/sysinv/sysinv/agent/disk.py
index 6591188585..8f728129d8 100644
--- a/sysinv/sysinv/sysinv/sysinv/agent/disk.py
+++ b/sysinv/sysinv/sysinv/sysinv/agent/disk.py
@@ -338,11 +338,11 @@ class DiskOperator(object):
                 device_type = device.device_type
 
                 rotation_rate = constants.DEVICE_TYPE_UNDETERMINED
-                if rotational is '1':
+                if rotational == '1':
                     device_type = constants.DEVICE_TYPE_HDD
                     if 'ID_ATA_ROTATION_RATE_RPM' in device:
                         rotation_rate = device['ID_ATA_ROTATION_RATE_RPM']
-                elif rotational is '0':
+                elif rotational == '0':
                     if constants.DEVICE_NAME_NVME in device.device_node:
                         device_type = constants.DEVICE_TYPE_NVME
                     else:
diff --git a/sysinv/sysinv/sysinv/sysinv/agent/lldp/plugin.py b/sysinv/sysinv/sysinv/sysinv/agent/lldp/plugin.py
index 2e7244969f..d1b84a8521 100644
--- a/sysinv/sysinv/sysinv/sysinv/agent/lldp/plugin.py
+++ b/sysinv/sysinv/sysinv/sysinv/agent/lldp/plugin.py
@@ -29,9 +29,9 @@ class Key(object):
         return hash((self.chassisid, self.portid, self.portname))
 
     def __cmp__(self, rhs):
-        return (cmp(self.chassisid, rhs.chassisid) or
-                cmp(self.portid, rhs.portid) or
-                cmp(self.portname, rhs.portname))
+        return ((self.chassisid < rhs.chassisid) or
+                (self.portid < rhs.portid) or
+                (self.portname < rhs.portname))
 
     def __eq__(self, rhs):
         return (self.chassisid == rhs.chassisid and
diff --git a/sysinv/sysinv/sysinv/sysinv/agent/manager.py b/sysinv/sysinv/sysinv/sysinv/agent/manager.py
index d0733d1b40..f029d766a2 100644
--- a/sysinv/sysinv/sysinv/sysinv/agent/manager.py
+++ b/sysinv/sysinv/sysinv/sysinv/agent/manager.py
@@ -760,7 +760,7 @@ class AgentManager(service.PeriodicService):
             except Timeout:
                 LOG.info("get_ihost_by_macs rpc Timeout.")
                 return  # wait for next audit cycle
-            except Exception as ex:
+            except Exception:
                 LOG.warn("Conductor RPC get_ihost_by_macs exception "
                          "response")
 
diff --git a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/profile.py b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/profile.py
index 0c3994a9a1..6870ae8508 100644
--- a/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/profile.py
+++ b/sysinv/sysinv/sysinv/sysinv/api/controllers/v1/profile.py
@@ -1024,7 +1024,7 @@ class ProfileController(rest.RestController):
     @cutils.synchronized(LOCK_NAME)
     @expose('json')
     def import_profile(self, file):
-        class ProfileObj(object):
+        class ProfileObj(object):  # noqa: F823
             display = ""
             proc = None
 
@@ -1470,7 +1470,7 @@ def _create_if_profile(profile_name, profile_node):
 
             try:
                 pecan.request.dbapi.iinterface_update(i.uuid, idict)
-            except Exception as e:
+            except Exception:
                 raise wsme.exc.ClientSideError(_("Failed to link interface uses."))
     except Exception as exc:
         ihost.ethernet_ports = \
diff --git a/sysinv/sysinv/sysinv/sysinv/common/ceph.py b/sysinv/sysinv/sysinv/sysinv/common/ceph.py
index 9abe2f3d26..bce9427fcf 100644
--- a/sysinv/sysinv/sysinv/sysinv/common/ceph.py
+++ b/sysinv/sysinv/sysinv/sysinv/common/ceph.py
@@ -562,7 +562,7 @@ class CephApiOperator(object):
         try:
             resp, body = self._ceph_api.osd_stat(body='json',
                                                  timeout=timeout)
-        except ReadTimeout as e:
+        except ReadTimeout:
             resp = type('Response', (),
                         dict(ok=False,
                              reason=('Ceph API osd_stat() timeout '
@@ -695,7 +695,7 @@ class CephApiOperator(object):
                     if rc:
                         break
 
-        except Exception as e:
+        except Exception:
             pass
 
         return rc
diff --git a/sysinv/sysinv/sysinv/sysinv/common/service_parameter.py b/sysinv/sysinv/sysinv/sysinv/common/service_parameter.py
index 1fa2af2e6e..5543b49e12 100644
--- a/sysinv/sysinv/sysinv/sysinv/common/service_parameter.py
+++ b/sysinv/sysinv/sysinv/sysinv/common/service_parameter.py
@@ -63,7 +63,7 @@ def _validate_float(name, value):
 
 
 def _validate_not_empty(name, value):
-    if not value or value is '':
+    if not value or value == '':
         raise wsme.exc.ClientSideError(_(
             "Parameter '%s' must not be an empty value." % name))
 
diff --git a/sysinv/sysinv/sysinv/sysinv/common/utils.py b/sysinv/sysinv/sysinv/sysinv/common/utils.py
index 378a2cbd27..cee2f342b7 100644
--- a/sysinv/sysinv/sysinv/sysinv/common/utils.py
+++ b/sysinv/sysinv/sysinv/sysinv/common/utils.py
@@ -609,7 +609,7 @@ def file_open(*args, **kwargs):
           be able to provide a stub module that doesn't alter system
           state at all (for unit tests)
     """
-    return file(*args, **kwargs)
+    return open(*args, **kwargs)
 
 
 def get_file_content(filename):
diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/ceph.py b/sysinv/sysinv/sysinv/sysinv/conductor/ceph.py
index 5ef010a806..68208d6427 100644
--- a/sysinv/sysinv/sysinv/sysinv/conductor/ceph.py
+++ b/sysinv/sysinv/sysinv/sysinv/conductor/ceph.py
@@ -845,7 +845,7 @@ class CephOperator(object):
         try:
             resp, body = self._ceph_api.osd_stat(body='json',
                                                  timeout=timeout)
-        except ReadTimeout as e:
+        except ReadTimeout:
             resp = type('Response', (),
                         dict(ok=False,
                              reason=('Ceph API osd_stat() timeout '
diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py
index 1f0f786b44..26660e6b21 100644
--- a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py
+++ b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py
@@ -855,7 +855,7 @@ class AppOperator(object):
                         db_app.id, chart, namespace, {'system_overrides':
                                                       system_overrides})
                 except exception.HelmOverrideNotFound:
-                    LOG.exception(e)
+                    LOG.exception("Helm Override Not Found")
 
     def _validate_labels(self, labels):
         expr = re.compile(r'[a-z0-9]([-a-z0-9]*[a-z0-9])')
@@ -2720,8 +2720,8 @@ class DockerHelper(object):
                 elif request == constants.APP_APPLY_OP:
                     cmd = ("/bin/bash -c 'set -o pipefail; armada apply "
                            "--enable-chart-cleanup --debug {m} {o} {t} | "
-                           "tee {l}'".format(m=manifest_file, o=overrides_str,
-                                             t=tiller_host, l=logfile))
+                           "tee {lf}'".format(m=manifest_file, o=overrides_str,
+                                             t=tiller_host, lf=logfile))
                     LOG.info("Armada apply command = %s" % cmd)
                     (exit_code, exec_logs) = armada_svc.exec_run(cmd)
                     if exit_code == 0:
diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py
index 8be65eaf92..06efc3e71e 100644
--- a/sysinv/sysinv/sysinv/sysinv/conductor/manager.py
+++ b/sysinv/sysinv/sysinv/sysinv/conductor/manager.py
@@ -10498,7 +10498,7 @@ class ConductorManager(service.PeriodicService):
                 with open(f, 'rb') as file:
                     new_hash[f] = hashlib.md5(file.read()).hexdigest()
 
-            if cmp(old_hash, new_hash) != 0:
+            if old_hash != new_hash:
                 LOG.info("There has been an overrides change, setting up "
                          "reapply of %s", app.name)
                 self._app.set_reapply(app.name)
diff --git a/sysinv/sysinv/sysinv/sysinv/openstack/common/rpc/impl_kombu.py b/sysinv/sysinv/sysinv/sysinv/openstack/common/rpc/impl_kombu.py
index 38d614c2cc..fcef36a617 100644
--- a/sysinv/sysinv/sysinv/sysinv/openstack/common/rpc/impl_kombu.py
+++ b/sysinv/sysinv/sysinv/sysinv/openstack/common/rpc/impl_kombu.py
@@ -519,7 +519,7 @@ class Connection(object):
             try:
                 self._connect(params)
                 return
-            except (IOError, self.connection_errors) as e:
+            except (IOError, self.connection_errors) as e:  # noqa: F841
                 pass
             except Exception as e:
                 # NOTE(comstud): Unfortunately it's possible for amqplib
@@ -532,7 +532,7 @@ class Connection(object):
                     raise
 
             log_info = {}
-            log_info['err_str'] = str(e)
+            log_info['err_str'] = str(e)  # noqa: F821
             log_info['max_retries'] = self.max_retries
             log_info.update(params)
 
diff --git a/sysinv/sysinv/sysinv/test-requirements.txt b/sysinv/sysinv/sysinv/test-requirements.txt
index 54eb255d6e..030d094866 100644
--- a/sysinv/sysinv/sysinv/test-requirements.txt
+++ b/sysinv/sysinv/sysinv/test-requirements.txt
@@ -1,11 +1,12 @@
 # The order of packages is significant, because pip processes them in the order
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
-hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0
+hacking>=1.1.0,<=2.0.0 # Apache-2.0
+pycodestyle>=2.0.0 # MIT License
 coverage>=3.6
 discover
-fixtures>=0.3.14
-mock<1.1.0,>=1.0
+fixtures>=3.0.0 # Apache-2.0/BSD
+mock>=2.0.0 # BSD
 passlib>=1.7.0
 psycopg2-binary
 python-barbicanclient<3.1.0,>=3.0.1
@@ -13,8 +14,8 @@ python-subunit>=0.0.18
 requests-mock>=0.6.0 # Apache-2.0
 sphinx!=1.2.0,!=1.3b1,<1.3,>=1.1.2
 oslosphinx<2.6.0,>=2.5.0 # Apache-2.0
-oslotest<1.6.0,>=1.5.1 # Apache-2.0
-stestr
+oslotest>=3.2.0 # Apache-2.0
+stestr>=1.0.0 # Apache-2.0
 testrepository>=0.0.18
 testtools!=1.2.0,>=0.9.36
 tempest-lib<0.5.0,>=0.4.0
diff --git a/sysinv/sysinv/sysinv/tox.ini b/sysinv/sysinv/sysinv/tox.ini
index 65791aaaa1..e4c91e168a 100644
--- a/sysinv/sysinv/sysinv/tox.ini
+++ b/sysinv/sysinv/sysinv/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = flake8,py27,py35,pylint
+envlist = flake8,py27,py36,pylint
 minversion = 1.6
 # skipsdist = True
 #,pip-missing-reqs
@@ -55,11 +55,6 @@ commands =
   find . -type f -name "*.pyc" -delete
 
 [flake8]
-# Note: hacking pulls in flake8 2.5.5 which can not parse an ignore list spanning multiple lines
-# E series are pep8
-# E126 continuation line over-indented for hanging indent
-# E127 continuation line over-indented for visual indent
-# E128 continuation line under-indented for visual indent
 # H series are hacking
 # H101 is TODO
 # H102 is apache license
@@ -73,13 +68,40 @@ commands =
 # H701 Empty localization string
 # H702 Formatting operation should be outside of localization method call
 # H703 Multiple positional placeholders
-ignore = E126,E127,E128,H101,H102,H104,H105,H306,H401,H403,H404,H405,H701,H702,H703
-exclude = build,dist,tools
+
+# B series are bugbear
+# B006 Do not use mutable data structures for argument defaults. Needs to be FIXED.
+# B007 Loop control variable not used within the loop body.
+# B009 Do not call getattr with a constant attribute value
+# B010 Do not call setattr with a constant attribute value
+# B012 return/continue/break inside finally blocks cause exceptions to be silenced
+# B014 Redundant exception types
+# B301 Python 3 does not include `.iter*` methods on dictionaries. (this should be suppressed on a per line basis)
+# B306 `BaseException.message` has been deprecated.  Needs to be FIXED
+
+# W series are warnings
+# W503 line break before binary operator
+# W504 line break after binary operator
+# W605 invalid escape sequence
+
+# E series are pep8
+# E117 over-indented
+# E126 continuation line over-indented for hanging indent
+# E127 continuation line over-indented for visual indent
+# E128 continuation line under-indented for visual indent
+# E402 module level import not at top of file
+
+ignore = H101,H102,H104,H105,H306,H401,H403,H404,H405,H701,H702,H703,
+         B006,B007,B009,B010,B012,B014,B301,B306,
+         W503,W504,W605,
+         E117,E126,E127,E128,E402
+exclude = build,dist,tools,.eggs
 max-line-length=120
 
 [testenv:flake8]
-basepython = python2.7
+basepython = python3
 deps = -r{toxinidir}/test-requirements.txt
+       flake8-bugbear
 commands =
   flake8 {posargs} . \
     scripts/manage-partitions \
@@ -92,8 +114,8 @@ commands =
   stestr run {posargs}
   stestr slowest
 
-[testenv:py35]
-basepython = python3.5
+[testenv:py36]
+basepython = python3.6
 commands =
   {[testenv]commands}
   stestr run {posargs}