Add: Run pylint in check; fix problems it finds

16 files changed, 64 insertions, 57 deletions

diff --git a/check b/check
index 919342f..6cf23af 100755
--- a/check
+++ b/check
@@ -3,6 +3,7 @@
 set -eu
 
 python -m CoverageTestRunner --ignore-missing-from=without-tests yarns vmdb
+pylint --rcfile pylint.conf vmdb
 yarn \
     --shell=python2 \
     --shell-arg '' \
diff --git a/pylint.conf b/pylint.conf
new file mode 100644
index 0000000..45d0a08
--- /dev/null
+++ b/pylint.conf
@@ -0,0 +1,39 @@
+[MASTER]
+persistent=no
+
+[MESSAGES CONTROL]
+disable=
+    abstract-class-little-used,
+    abstract-class-not-used,
+    attribute-defined-outside-init,
+    cyclic-import,
+    fixme,
+    global-statement,
+    interface-not-implemented,
+    invalid-name,
+    locally-disabled,
+    missing-docstring,
+    no-member,
+    no-self-use,
+    protected-access,
+    redefined-builtin,
+    redefined-variable-type,
+    star-args,
+    too-few-public-methods,
+    too-many-arguments,
+    too-many-branches,
+    too-many-instance-attributes,
+    too-many-lines,
+    too-many-locals,
+    too-many-public-methods,
+    too-many-statements,
+    unidiomatic-typecheck,
+    unsubscriptable-object,
+    unsupported-membership-test,
+    unused-argument
+
+[REPORTS]
+reports=no
+
+[SIMILARITIES]
+min-similarity-lines=999
diff --git a/vmdb/app.py b/vmdb/app.py
index 302e8b7..e5072ee 100644
--- a/vmdb/app.py
+++ b/vmdb/app.py
@@ -91,7 +91,7 @@ class Vmdb2(cliapp.Application):
                 else:
                     method = getattr(runner, method_name)
                     method(expanded_step, self.settings, state)
-            except Exception as e:
+            except BaseException as e:
                 vmdb.error(str(e))
                 core_meltdown = True
                 if not keep_going:
diff --git a/vmdb/plugins/ansible_plugin.py b/vmdb/plugins/ansible_plugin.py
index 83a989e..40564ad 100644
--- a/vmdb/plugins/ansible_plugin.py
+++ b/vmdb/plugins/ansible_plugin.py
@@ -17,7 +17,6 @@
 
 
 
-import logging
 import os
 import tempfile
 
@@ -45,7 +44,7 @@ class AnsibleStepRunner(vmdb.StepRunnerInterface):
         vmdb.progress(
             'Running ansible playbook {} on filesystem at {} ({})'.format(
                 playbook, mount_point, fstag))
-        
+
         state.ansible_inventory = self.create_inventory(mount_point)
         vmdb.progress(
             'Created {} for Ansible inventory'.format(state.ansible_inventory))
diff --git a/vmdb/plugins/chroot_plugin.py b/vmdb/plugins/chroot_plugin.py
index 7be6992..5f17f01 100644
--- a/vmdb/plugins/chroot_plugin.py
+++ b/vmdb/plugins/chroot_plugin.py
@@ -17,9 +17,7 @@
 
 
 
-import logging
 import os
-import sys
 
 import cliapp
 
@@ -31,7 +29,7 @@ class ChrootPlugin(cliapp.Plugin):
     def enable(self):
         self.app.step_runners.add(ChrootStepRunner())
         self.app.step_runners.add(ShellStepRunner())
-        
+
 
 class ChrootStepRunner(vmdb.StepRunnerInterface):
 
@@ -47,7 +45,7 @@ class ChrootStepRunner(vmdb.StepRunnerInterface):
         vmdb.progress(
             'chroot {} to {}'.format(mount_point, ' '.join(shell.split('\n'))))
         vmdb.runcmd(['chroot', mount_point, 'sh', '-c', shell])
-        
+
 
 class ShellStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/debootstrap_plugin.py b/vmdb/plugins/debootstrap_plugin.py
index da7f1fc..61625ef 100644
--- a/vmdb/plugins/debootstrap_plugin.py
+++ b/vmdb/plugins/debootstrap_plugin.py
@@ -17,9 +17,6 @@
 
 
 
-import logging
-import sys
-
 import cliapp
 
 import vmdb
@@ -29,7 +26,7 @@ class DebootstrapPlugin(cliapp.Plugin):
 
     def enable(self):
         self.app.step_runners.add(DebootstrapStepRunner())
-        
+
 
 class DebootstrapStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/echo_plugin.py b/vmdb/plugins/echo_plugin.py
index 26f8bab..c09a387 100644
--- a/vmdb/plugins/echo_plugin.py
+++ b/vmdb/plugins/echo_plugin.py
@@ -18,7 +18,6 @@
 
 
 import logging
-import sys
 
 import cliapp
 
@@ -29,7 +28,7 @@ class EchoPlugin(cliapp.Plugin):
 
     def enable(self):
         self.app.step_runners.add(EchoStepRunner())
-        
+
 
 class EchoStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/error_plugin.py b/vmdb/plugins/error_plugin.py
index 73db1bc..e40e1ec 100644
--- a/vmdb/plugins/error_plugin.py
+++ b/vmdb/plugins/error_plugin.py
@@ -17,9 +17,6 @@
 
 
 
-import logging
-import sys
-
 import cliapp
 
 import vmdb
@@ -29,7 +26,7 @@ class ErrorPlugin(cliapp.Plugin):
 
     def enable(self):
         self.app.step_runners.add(ErrorStepRunner())
-        
+
 
 class ErrorStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/grub_plugin.py b/vmdb/plugins/grub_plugin.py
index 9de94d2..2a2029b 100644
--- a/vmdb/plugins/grub_plugin.py
+++ b/vmdb/plugins/grub_plugin.py
@@ -72,10 +72,8 @@
 # The grub step will take of the rest.
 
 
-import logging
 import os
 import re
-import sys
 
 import cliapp
 
@@ -120,7 +118,6 @@ class GrubStepRunner(vmdb.StepRunnerInterface):
         self.install_grub(step, settings, state, grub_package, grub_target)
 
     def install_grub(self, step, settings, state, grub_package, grub_target):
-        device = step['device']
         console = step.get('console', None)
 
         rootfs = step['root-fs']
@@ -195,7 +192,7 @@ class GrubStepRunner(vmdb.StepRunnerInterface):
         # We get /dev/mappers/loopXpY and return /dev/loopX
         assert partition_device.startswith('/dev/mapper/loop')
 
-        m = re.match('^/dev/mapper/(?P<loop>loop\d+)p\d+$', partition_device)
+        m = re.match(r'^/dev/mapper/(?P<loop>loop\d+)p\d+$', partition_device)
         assert m is not None
 
         loop = m.group('loop')
@@ -229,7 +226,7 @@ class GrubStepRunner(vmdb.StepRunnerInterface):
         env = os.environ.copy()
         env['DEBIAN_FRONTEND'] = 'noninteractive'
         self.chroot(
-            chroot, 
+            chroot,
             ['apt-get', '-y', '--no-show-progress', 'install', package],
             env=env)
 
diff --git a/vmdb/plugins/mkfs_plugin.py b/vmdb/plugins/mkfs_plugin.py
index 82fd1d6..5aca499 100644
--- a/vmdb/plugins/mkfs_plugin.py
+++ b/vmdb/plugins/mkfs_plugin.py
@@ -17,9 +17,6 @@
 
 
 
-import logging
-import sys
-
 import cliapp
 
 import vmdb
@@ -29,7 +26,7 @@ class MkfsPlugin(cliapp.Plugin):
 
     def enable(self):
         self.app.step_runners.add(MkfsStepRunner())
-        
+
 
 class MkfsStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/mkimg_plugin.py b/vmdb/plugins/mkimg_plugin.py
index 6b1749c..ea4830f 100644
--- a/vmdb/plugins/mkimg_plugin.py
+++ b/vmdb/plugins/mkimg_plugin.py
@@ -17,9 +17,6 @@
 
 
 
-import logging
-import sys
-
 import cliapp
 
 import vmdb
@@ -33,7 +30,7 @@ class MkimgPlugin(cliapp.Plugin):
             ['size'],
             'size of output image',
             default='1GiB')
-        
+
 
 class MkimgStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/mount_plugin.py b/vmdb/plugins/mount_plugin.py
index c563181..da038ad 100644
--- a/vmdb/plugins/mount_plugin.py
+++ b/vmdb/plugins/mount_plugin.py
@@ -19,7 +19,6 @@
 
 import logging
 import os
-import sys
 import tempfile
 
 import cliapp
@@ -31,7 +30,7 @@ class MountPlugin(cliapp.Plugin):
 
     def enable(self):
         self.app.step_runners.add(MountStepRunner())
-        
+
 
 class MountStepRunner(vmdb.StepRunnerInterface):
 
@@ -53,7 +52,7 @@ class MountStepRunner(vmdb.StepRunnerInterface):
         self.mount_virtuals(rootfs, state)
 
     def teardown(self, step, settings, state):
-        self.unmount_virtuals()
+        self.unmount_virtuals(state)
         self.unmount_rootfs(step, settings, state)
 
     def mount_rootfs(self, step, settings, state):
@@ -68,21 +67,15 @@ class MountStepRunner(vmdb.StepRunnerInterface):
         device = state.parts[part_tag]
         mount_point = tempfile.mkdtemp()
 
-        vmdb.progress(
-            'Mounting {} ({}) on {}'.format(device, fs_tag, mount_point))
         vmdb.runcmd(['mount', device, mount_point])
         state.mounts[fs_tag] = mount_point
 
         return mount_point
 
     def unmount_rootfs(self, step, settings, state):
-        part_tag = step['mount']
-        device = state.parts[part_tag]
         fs_tag = step['fs-tag']
         mount_point = state.mounts[fs_tag]
-        
-        vmdb.progress(
-            'Unmounting {} ({}) from {}'.format(mount_point, fs_tag, device))
+
         vmdb.runcmd(['umount', mount_point])
         os.rmdir(mount_point)
 
@@ -92,19 +85,15 @@ class MountStepRunner(vmdb.StepRunnerInterface):
 
         for device, mount_point, fstype in self.virtuals:
             path = os.path.join(rootfs, './' + mount_point)
-            vmdb.progress(
-                'Mounting virtual {} ({}) on {}'.format(device, fstype, path))
             if not os.path.exists(path):
                 os.mkdir(path)
             vmdb.runcmd(['mount', '-t', fstype, device, path])
             state.virtuals.append(path)
         logging.debug('mounted virtuals: %r', state.virtuals)
 
-    def unmount_virtuals(self):
+    def unmount_virtuals(self, state):
         logging.debug('unmounting virtuals: %r', state.virtuals)
         for mount_point in reversed(state.virtuals):
-            vmdb.progress(
-                'Unmounting virtual {}'.format(mount_point))
             try:
                 vmdb.runcmd(['umount', mount_point])
             except cliapp.AppException:
diff --git a/vmdb/plugins/partition_plugin.py b/vmdb/plugins/partition_plugin.py
index f88b07e..cf5b317 100644
--- a/vmdb/plugins/partition_plugin.py
+++ b/vmdb/plugins/partition_plugin.py
@@ -17,10 +17,6 @@
 
 
 
-import logging
-import os
-import sys
-
 import cliapp
 
 import vmdb
@@ -31,7 +27,7 @@ class PartitionPlugin(cliapp.Plugin):
     def enable(self):
         self.app.step_runners.add(MklabelStepRunner())
         self.app.step_runners.add(MkpartStepRunner())
-        
+
 
 class MklabelStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/plugins/rootfs_cache_plugin.py b/vmdb/plugins/rootfs_cache_plugin.py
index 89eb3ec..5be833a 100644
--- a/vmdb/plugins/rootfs_cache_plugin.py
+++ b/vmdb/plugins/rootfs_cache_plugin.py
@@ -17,9 +17,7 @@
 
 
 
-import logging
 import os
-import sys
 
 import cliapp
 
@@ -36,7 +34,7 @@ class RootFSCachePlugin(cliapp.Plugin):
 
         self.app.step_runners.add(MakeCacheStepRunner())
         self.app.step_runners.add(UnpackCacheStepRunner())
-        
+
 
 class MakeCacheStepRunner(vmdb.StepRunnerInterface):
 
@@ -51,7 +49,7 @@ class MakeCacheStepRunner(vmdb.StepRunnerInterface):
             vmdb.progress(
                 'Caching contents of {} to {}'.format(rootdir, tar_path))
             vmdb.runcmd(['tar', '-C', rootdir, '-caf', tar_path, '.'])
-        
+
 
 class UnpackCacheStepRunner(vmdb.StepRunnerInterface):
 
diff --git a/vmdb/step_list.py b/vmdb/step_list.py
index b5a7c7c..a3f3a89 100644
--- a/vmdb/step_list.py
+++ b/vmdb/step_list.py
@@ -81,4 +81,4 @@ class NoMatchingRunner(cliapp.AppException):
     def __init__(self, keys):
         super(NoMatchingRunner, self).__init__(
             'No runner implements step with keys {}'.format(
-            ', '.join(keys)))
+                ', '.join(keys)))
diff --git a/vmdb/step_list_tests.py b/vmdb/step_list_tests.py
index 9ca228d..4d052d7 100644
--- a/vmdb/step_list_tests.py
+++ b/vmdb/step_list_tests.py
@@ -37,7 +37,7 @@ class StepRunnerListTests(unittest.TestCase):
         steps = vmdb.StepRunnerList()
         runner = DummyStepRunner()
         steps.add(runner)
-        found = steps.find({ 'foo': None, 'bar': None })
+        found = steps.find({'foo': None, 'bar': None})
         self.assertEqual(runner, found)
 
     def test_raises_error_if_runner_not_found(self):
@@ -45,10 +45,13 @@ class StepRunnerListTests(unittest.TestCase):
         runner = DummyStepRunner()
         steps.add(runner)
         with self.assertRaises(vmdb.NoMatchingRunner):
-            steps.find({ 'foo': None })
+            steps.find({'foo': None})
 
 
 class DummyStepRunner(vmdb.StepRunnerInterface):
 
+    def run(self, *args):
+        pass
+
     def get_required_keys(self):
         return ['foo', 'bar']