black and tidy

src/lib/charm/openstack/trilio_dm.py

@@ -13,7 +13,6 @@
 # limitations under the License.
 
 import collections
-import subprocess
 import shutil
 
 import charmhelpers.core.hookenv as hookenv
@@ -23,12 +22,9 @@ import charms_openstack.charm
 import charms_openstack.adapters as os_adapters
 
 
-DM_USR = 'nova'
-DM_GRP = 'nova'
-VALID_BACKUP_TARGETS = [
-    'nfs',
-    's3'
-]
+DM_USR = "nova"
+DM_GRP = "nova"
+VALID_BACKUP_TARGETS = ["nfs", "s3"]
 
 
 class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
@@ -37,9 +33,9 @@ class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
 
     adapters_class = os_adapters.OpenStackAPIRelationAdapters
 
-    data_mover_conf = '/etc/tvault-contego/tvault-contego.conf'
+    data_mover_conf = "/etc/tvault-contego/tvault-contego.conf"
     logrotate_conf = "/etc/logrotate.d/tvault-contego"
-    
+
     # First release supported
     release = "stein"
 
@@ -59,7 +55,7 @@ class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
 
     def get_amqp_credentials(self):
         return ("datamover", "openstack")
-    
+
     def configure_source(self):
         with open("/etc/apt/sources.list.d/trilio-wlm.list", "w") as tsources:
             tsources.write(hookenv.config("triliovault-pkg-source"))
@@ -67,21 +63,19 @@ class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
 
     @property
     def packages(self):
-        if hookenv.config('python-version') == 2:
-            return ['tvault-contego', 'nfs-common']
-        return ['python3-tvault-contego', 'nfs-common']
+        if hookenv.config("python-version") == 2:
+            return ["tvault-contego", "nfs-common"]
+        return ["python3-tvault-contego", "nfs-common"]
 
     @property
     def services(self):
-        if hookenv.config('backup-target-type') == 's3':
+        if hookenv.config("backup-target-type") == "s3":
             return ["tvault-contego", "tvault-object-store"]
         return ["tvault-contego"]
 
     @property
     def restart_map(self):
-        return {
-            self.data_mover_conf: self.services,
-        }
+        return {self.data_mover_conf: self.services}
 
     # TODO: drop once packaging is updated
     def install(self):
@@ -90,21 +84,41 @@ class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
         self.install_files()
         self.configure_nova_user()
 
+    # TODO: drop once packaging is updated
+    def upgrade_charm(self):
+        super().upgrade_charm()
+        self.ensure_dirs()
+
     # TODO: drop once included in packages
     def ensure_dirs(self):
         """
         Ensures all the required directories are present
         and have appropriate permissions.
         """
-        ch_host.mkdir(hookenv.config('tv-data-dir'),
-                      owner=DM_USR, group=DM_GRP, perms=0o770, force=True)
+        ch_host.mkdir(
+            hookenv.config("tv-data-dir"),
+            owner=DM_USR,
+            group=DM_GRP,
+            perms=0o770,
+            force=True,
+        )
         # TODO: review this?
         # os.system('rm -rf {}'.format(hookenv.config('tv-data-dir-old')))
-        ch_host.mkdir(hookenv.config('tv-data-dir-old'),
-                      owner=DM_USR, group=DM_GRP, perms=0o770, force=True)
-        ch_host.mkdir('/etc/tvault-contego',
-                      owner='root', group=DM_GRP, perms=0o750, force=True)
-        
+        ch_host.mkdir(
+            hookenv.config("tv-data-dir-old"),
+            owner=DM_USR,
+            group=DM_GRP,
+            perms=0o770,
+            force=True,
+        )
+        ch_host.mkdir(
+            "/etc/tvault-contego",
+            owner="root",
+            group=DM_GRP,
+            perms=0o750,
+            force=True,
+        )
+
     # TODO: drop once included in packages
     def install_files(self):
         """
@@ -114,14 +128,10 @@ class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
         # "files/trilio/tvault-object-store.service":
         # "/etc/systemd/system",
         _file_map = {
-            "files/trilio/tvault-contego.service":
-                "/etc/systemd/system",
-            "files/trilio/trilio.filters":
-                "/etc/nova/rootwrap.d",
-            "files/trilio/trilio_sudoers":
-                "/etc/sudoers.d/",
-            "files/trilio/tvault-contego":
-                "/etc/logrotate.d/",
+            "files/trilio/tvault-contego.service": "/etc/systemd/system",
+            "files/trilio/trilio.filters": "/etc/nova/rootwrap.d",
+            "files/trilio/trilio_sudoers": "/etc/sudoers.d/",
+            "files/trilio/tvault-contego": "/etc/logrotate.d/",
         }
         for file, target in _file_map.items():
             shutil.copy(file, target)
@@ -129,5 +139,5 @@ class TrilioDataMoverCharm(charms_openstack.charm.OpenStackCharm):
     # TODO: review why this is required
     def configure_nova_user(self):
         """Add nova user to kvm and disk groups"""
-        for grp in ('kvm', 'disk'):
+        for grp in ("kvm", "disk"):
             ch_host.add_user_to_group(DM_USR, grp)

src/reactive/data_mover_handlers.py

@@ -17,11 +17,7 @@ import charms.reactive as reactive
 # trilio_dm
 import charm.openstack.trilio_dm as trilio_dm  # noqa
 
-charm.use_defaults(
-    "charm.installed",
-    "config.changed",
-    "update-status",
-)
+charm.use_defaults("charm.installed", "config.changed", "update-status")
 
 
 @reactive.when("amqp.available")
@@ -36,7 +32,7 @@ def render_config(*args):
 
 
 # NOTE(jamespage): default handler is in api layer which is to much
-@reactive.when('amqp.connected')
+@reactive.when("amqp.connected")
 def default_amqp_connection(amqp):
     """Handle the default amqp connection.
 
@@ -46,4 +42,4 @@ def default_amqp_connection(amqp):
     with charm.provide_charm_instance() as instance:
         user, vhost = instance.get_amqp_credentials()
         amqp.request_access(username=user, vhost=vhost)
-        instance.assess_status()
+        instance.assess_status()

unit_tests/__init__.py

@@ -1,4 +1,4 @@
 import sys
 
-sys.path.append('src')
-sys.path.append('src/reactive')
+sys.path.append("src")
+sys.path.append("src/reactive")

unit_tests/test_data_mover.py

@@ -7,9 +7,7 @@ _when_not_args = {}
 
 
 def mock_hook_factory(d):
-
     def mock_hook(*args, **kwargs):
-
         def inner(f):
             # remember what we were passed.  Note that we can't actually
             # determine the class we're attached to, as the decorator only gets
@@ -19,18 +17,22 @@ def mock_hook_factory(d):
             except KeyError:
                 d[f.__name__] = [dict(args=args, kwargs=kwargs)]
             return f
+
         return inner
+
     return mock_hook
 
 
 class Test(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
-        cls._patched_when = mock.patch('charms.reactive.when',
-                                       mock_hook_factory(_when_args))
+        cls._patched_when = mock.patch(
+            "charms.reactive.when", mock_hook_factory(_when_args)
+        )
         cls._patched_when_started = cls._patched_when.start()
-        cls._patched_when_not = mock.patch('charms.reactive.when_not',
-                                           mock_hook_factory(_when_not_args))
+        cls._patched_when_not = mock.patch(
+            "charms.reactive.when_not", mock_hook_factory(_when_not_args)
+        )
         cls._patched_when_not_started = cls._patched_when_not.start()
         # force requires to rerun the mock_hook decorator:
         # try except is Python2/Python3 compatibility as Python3 has moved
@@ -39,6 +41,7 @@ class Test(unittest.TestCase):
             reload(datamover)
         except NameError:
             import importlib
+
             importlib.reload(datamover)
 
     @classmethod
@@ -54,6 +57,7 @@ class Test(unittest.TestCase):
             reload(datamover)
         except NameError:
             import importlib
+
             importlib.reload(datamover)
 
     def setUp(self):
@@ -81,109 +85,109 @@ class Test(unittest.TestCase):
         # are meaningful for this interface: this is to handle regressions.
         # The keys are the function names that the hook attaches to.
         when_patterns = {
-            'stop_tvault_contego_plugin': ('tvault-contego.stopping', ),
+            "stop_tvault_contego_plugin": ("tvault-contego.stopping",)
         }
         when_not_patterns = {
-            'install_tvault_contego_plugin': (
-                'tvault-contego.installed', ), }
+            "install_tvault_contego_plugin": ("tvault-contego.installed",)
+        }
         # check the when hooks are attached to the expected functions
-        for t, p in [(_when_args, when_patterns),
-                     (_when_not_args, when_not_patterns)]:
+        for t, p in [
+            (_when_args, when_patterns),
+            (_when_not_args, when_not_patterns),
+        ]:
             for f, args in t.items():
                 # check that function is in patterns
-                self.assertTrue(f in p.keys(),
-                                "{} not found".format(f))
+                self.assertTrue(f in p.keys(), "{} not found".format(f))
                 # check that the lists are equal
                 lists = []
                 for a in args:
-                    lists += a['args'][:]
-                self.assertEqual(sorted(lists), sorted(p[f]),
-                                 "{}: incorrect state registration".format(f))
+                    lists += a["args"][:]
+                self.assertEqual(
+                    sorted(lists),
+                    sorted(p[f]),
+                    "{}: incorrect state registration".format(f),
+                )
 
     def test_install_plugin(self):
-         self.patch(datamover, 'install_plugin')
-         datamover.install_plugin('pkg_name')
-         self.install_plugin.assert_called_once_with('pkg_name')
+        self.patch(datamover, "install_plugin")
+        datamover.install_plugin("pkg_name")
+        self.install_plugin.assert_called_once_with("pkg_name")
 
     def test_uninstall_plugin(self):
-         self.patch(datamover, 'uninstall_plugin')
-         datamover.uninstall_plugin()
-         self.uninstall_plugin.assert_called_once_with()
+        self.patch(datamover, "uninstall_plugin")
+        datamover.uninstall_plugin()
+        self.uninstall_plugin.assert_called_once_with()
 
     def test_install_tvault_contego_plugin(self):
-         self.patch(datamover, 'install_tvault_contego_plugin')
-         datamover.install_tvault_contego_plugin()
-         self.install_tvault_contego_plugin.assert_called_once_with()
+        self.patch(datamover, "install_tvault_contego_plugin")
+        datamover.install_tvault_contego_plugin()
+        self.install_tvault_contego_plugin.assert_called_once_with()
 
     def test_stop_tvault_contego_plugin(self):
-         self.patch(datamover, 'config')
-         self.patch(datamover, 'status_set')
-         self.patch(datamover, 'remove_state')
-         self.patch(datamover, 'uninstall_plugin')
-         self.uninstall_plugin.return_value = True
-         datamover.stop_tvault_contego_plugin()
-         self.status_set.assert_called_with(
-             'maintenance', 'Stopping...')
-         self.remove_state.assert_called_with('tvault-contego.stopping')
+        self.patch(datamover, "config")
+        self.patch(datamover, "status_set")
+        self.patch(datamover, "remove_state")
+        self.patch(datamover, "uninstall_plugin")
+        self.uninstall_plugin.return_value = True
+        datamover.stop_tvault_contego_plugin()
+        self.status_set.assert_called_with("maintenance", "Stopping...")
+        self.remove_state.assert_called_with("tvault-contego.stopping")
 
     def test_s3_object_storage_fail(self):
-         self.patch(datamover, 'config')
-         self.config.return_value = 's3'
-         self.patch(datamover, 'apt_update')
-         self.patch(datamover, 'status_set')
-         self.patch(datamover, 'validate_backup')
-         self.validate_backup.return_value = True
-         self.patch(datamover, 'add_users')
-         self.add_users.return_value = True
-         self.patch(datamover, 'create_virt_env')
-         self.create_virt_env.return_value = True
-         self.patch(datamover, 'ensure_files')
-         self.ensure_files.return_value = True
-         self.patch(datamover, 'create_conf')
-         self.create_conf.return_value = True
-         self.patch(datamover, 'ensure_data_dir')
-         self.ensure_data_dir.return_value = True
-         self.patch(datamover, 'create_service_file')
-         self.create_service_file.return_value = True
-         self.patch(datamover, 'create_object_storage_service')
-         self.create_object_storage_service.return_value = False
-         self.patch(datamover.os, 'system')
-         self.patch(datamover, 'log')
-         datamover.install_tvault_contego_plugin()
-         self.status_set.assert_called_with(
-             'blocked',
-             'Failed while creating ObjectStore service file')
+        self.patch(datamover, "config")
+        self.config.return_value = "s3"
+        self.patch(datamover, "apt_update")
+        self.patch(datamover, "status_set")
+        self.patch(datamover, "validate_backup")
+        self.validate_backup.return_value = True
+        self.patch(datamover, "add_users")
+        self.add_users.return_value = True
+        self.patch(datamover, "create_virt_env")
+        self.create_virt_env.return_value = True
+        self.patch(datamover, "ensure_files")
+        self.ensure_files.return_value = True
+        self.patch(datamover, "create_conf")
+        self.create_conf.return_value = True
+        self.patch(datamover, "ensure_data_dir")
+        self.ensure_data_dir.return_value = True
+        self.patch(datamover, "create_service_file")
+        self.create_service_file.return_value = True
+        self.patch(datamover, "create_object_storage_service")
+        self.create_object_storage_service.return_value = False
+        self.patch(datamover.os, "system")
+        self.patch(datamover, "log")
+        datamover.install_tvault_contego_plugin()
+        self.status_set.assert_called_with(
+            "blocked", "Failed while creating ObjectStore service file"
+        )
 
     def test_s3_object_storage_pass(self):
-         self.patch(datamover, 'config')
-         self.patch(datamover, 'apt_update')
-         self.patch(datamover, 'status_set')
-         self.patch(datamover, 'validate_backup')
-         self.validate_backup.return_value = True
-         self.patch(datamover, 'add_users')
-         self.add_users.return_value = True
-         self.patch(datamover, 'create_virt_env')
-         self.create_virt_env.return_value = True
-         self.patch(datamover, 'ensure_files')
-         self.ensure_files.return_value = True
-         self.patch(datamover, 'create_conf')
-         self.create_conf.return_value = True
-         self.patch(datamover, 'ensure_data_dir')
-         self.ensure_data_dir.return_value = True
-         self.patch(datamover, 'create_service_file')
-         self.create_service_file.return_value = True
-         self.patch(datamover, 'create_object_storage_service')
-         self.create_object_storage_service.return_value = True
-         self.patch(datamover, 'service_restart')
-         self.patch(datamover, 'set_flag')
-         self.patch(datamover, 'application_version_set')
-         self.patch(datamover, 'get_new_version')
-         self.patch(datamover.os, 'system')
-         datamover.install_tvault_contego_plugin()
-         self.service_restart.assert_called_with(
-             'tvault-contego')
-         self.status_set.assert_called_with(
-             'active', 'Ready...')
-         self.application_version_set.assert_called_once()
-         self.set_flag.assert_called_with(
-             'tvault-contego.installed')
+        self.patch(datamover, "config")
+        self.patch(datamover, "apt_update")
+        self.patch(datamover, "status_set")
+        self.patch(datamover, "validate_backup")
+        self.validate_backup.return_value = True
+        self.patch(datamover, "add_users")
+        self.add_users.return_value = True
+        self.patch(datamover, "create_virt_env")
+        self.create_virt_env.return_value = True
+        self.patch(datamover, "ensure_files")
+        self.ensure_files.return_value = True
+        self.patch(datamover, "create_conf")
+        self.create_conf.return_value = True
+        self.patch(datamover, "ensure_data_dir")
+        self.ensure_data_dir.return_value = True
+        self.patch(datamover, "create_service_file")
+        self.create_service_file.return_value = True
+        self.patch(datamover, "create_object_storage_service")
+        self.create_object_storage_service.return_value = True
+        self.patch(datamover, "service_restart")
+        self.patch(datamover, "set_flag")
+        self.patch(datamover, "application_version_set")
+        self.patch(datamover, "get_new_version")
+        self.patch(datamover.os, "system")
+        datamover.install_tvault_contego_plugin()
+        self.service_restart.assert_called_with("tvault-contego")
+        self.status_set.assert_called_with("active", "Ready...")
+        self.application_version_set.assert_called_once()
+        self.set_flag.assert_called_with("tvault-contego.installed")