From c1d2f79c22e18557e01601660c956f0825c24473 Mon Sep 17 00:00:00 2001 From: utkarshbhatthere Date: Mon, 8 Aug 2022 18:57:23 +0530 Subject: [PATCH] Fixes currently present PEP8 issues Change-Id: Idff27970dc0d41288444fc3ed18585e3f3a1a0ad --- src/charm.py | 28 ++++++++++++----- src/ganesha.py | 57 ++++++++++++++++++++++++---------- src/interface_ceph_nfs_peer.py | 7 +++-- src/manager.py | 4 +-- tests/nfs_ganesha.py | 21 +++++++++---- unit_tests/test_ganesha.py | 10 +++--- 6 files changed, 89 insertions(+), 38 deletions(-) diff --git a/src/charm.py b/src/charm.py index 256e498..01caa84 100755 --- a/src/charm.py +++ b/src/charm.py @@ -88,7 +88,9 @@ class CephNFSContext(object): :returns: Data pool name. :rtype: str """ - return self.charm_instance.config_get('rbd-pool-name', self.charm_instance.app.name) + return self.charm_instance.config_get( + 'rbd-pool-name', self.charm_instance.app.name + ) @property def client_name(self): @@ -293,7 +295,8 @@ class CephNFSCharm( mode=0o750) def daemon_reload_and_restart(service_name): - logging.debug("restarting {} after config change".format(service_name)) + logging.debug("restarting {} after config change" + .format(service_name)) subprocess.check_call(['systemctl', 'daemon-reload']) subprocess.check_call(['systemctl', 'restart', service_name]) @@ -411,7 +414,8 @@ class CephNFSCharm( def create_share_action(self, event): if not self.model.unit.is_leader(): - event.fail("Share creation needs to be run from the application leader") + event.fail("Share creation needs to be run " + "from the application leader") return share_size = event.params.get('size') name = event.params.get('name') @@ -420,7 +424,8 @@ class CephNFSCharm( export_path = self.ganesha_client.create_share( size=share_size, name=name, access_ips=allowed_ips) if not export_path: - event.fail("Failed to create share, check the log for more details") + event.fail("Failed to create share, check the " + "log for more details") return self.peers.trigger_reload() event.set_results({ @@ -431,12 +436,17 @@ class CephNFSCharm( def list_shares_action(self, event): exports = self.ganesha_client.list_shares() event.set_results({ - "exports": [{"id": export.export_id, "name": export.name} for export in exports] + "exports": [ + { + "id": export.export_id, "name": export.name + } for export in exports + ] }) def delete_share_action(self, event): if not self.model.unit.is_leader(): - event.fail("Share creation needs to be run from the application leader") + event.fail("Share creation needs to be run " + "from the application leader") return name = event.params.get('name') purge = event.params.get('purge') @@ -448,7 +458,8 @@ class CephNFSCharm( def grant_access_action(self, event): if not self.model.unit.is_leader(): - event.fail("Share creation needs to be run from the application leader") + event.fail("Share creation needs to be run " + "from the application leader") return name = event.params.get('name') address = event.params.get('client') @@ -463,7 +474,8 @@ class CephNFSCharm( def revoke_access_action(self, event): if not self.model.unit.is_leader(): - event.fail("Share creation needs to be run from the application leader") + event.fail("Share creation needs to be run " + "from the application leader") return name = event.params.get('name') address = event.params.get('client') diff --git a/src/ganesha.py b/src/ganesha.py index 9997488..5c54c11 100644 --- a/src/ganesha.py +++ b/src/ganesha.py @@ -26,7 +26,9 @@ class Export(object): raise RuntimeError('export_options must be a dictionary') self.export_options = export_options if not isinstance(self.export_options['EXPORT']['CLIENT'], list): - self.export_options['EXPORT']['CLIENT'] = [self.export_options['EXPORT']['CLIENT']] + self.export_options['EXPORT']['CLIENT'] = [ + self.export_options['EXPORT']['CLIENT'] + ] def from_export(export: str) -> 'Export': return Export(export_options=manager.parseconf(export)) @@ -52,9 +54,13 @@ class Export(object): clients_by_mode = {'r': [], 'rw': []} for client in self.clients: if client['Access_Type'].lower() == 'r': - clients_by_mode['r'] += [s.strip() for s in client['Clients'].split(',')] + clients_by_mode['r'] += [ + s.strip() for s in client['Clients'].split(',') + ] elif client['Access_Type'].lower() == 'rw': - clients_by_mode['rw'] += [s.strip() for s in client['Clients'].split(',')] + clients_by_mode['rw'] += [ + s.strip() for s in client['Clients'].split(',') + ] else: raise RuntimeError("Invalid access type") return clients_by_mode @@ -84,7 +90,9 @@ class Export(object): def remove_client(self, client: str): clients_by_mode = self.clients_by_mode for (mode, clients) in clients_by_mode.items(): - clients_by_mode[mode] = [old_client for old_client in clients if old_client != client] + clients_by_mode[mode] = [ + old_client for old_client in clients if old_client != client + ] self.export_options['EXPORT']['CLIENT'] = [] for (mode, clients) in clients_by_mode.items(): if clients: @@ -112,7 +120,9 @@ class GaneshaNFS(object): if name is None: name = str(uuid.uuid4()) else: - existing_shares = [share for share in self.list_shares() if share.name == name] + existing_shares = [ + share for share in self.list_shares() if share.name == name + ] if existing_shares: return existing_shares[0].path if size is not None: @@ -179,7 +189,8 @@ class GaneshaNFS(object): def resize_share(self, name: str, size: int): size_in_bytes = size * 1024 * 1024 - self._ceph_subvolume_command('resize', 'ceph-fs', name, str(size_in_bytes), '--no_shrink') + self._ceph_subvolume_command('resize', 'ceph-fs', name, + str(size_in_bytes), '--no_shrink') def delete_share(self, name: str, purge=False): share = [share for share in self.list_shares() if share.name == name] @@ -187,7 +198,8 @@ class GaneshaNFS(object): share = share[0] else: return - logging.info("About to remove export {} ({})".format(share.name, share.export_id)) + logging.info("About to remove export {} ({})" + .format(share.name, share.export_id)) self._ganesha_remove_export(share.export_id) logging.debug("Removing export from index") self._remove_share_from_index(share.export_id) @@ -204,7 +216,8 @@ class GaneshaNFS(object): export_template = share.to_export() logging.debug("Export template::\n{}".format(export_template)) tmp_file = self._tmpfile(export_template) - self._rados_put('ganesha-export-{}'.format(share.export_id), tmp_file.name) + self._rados_put('ganesha-export-{}'.format(share.export_id), + tmp_file.name) self._ganesha_update_export(share.export_id, tmp_file.name) def revoke_access(self, name: str, client: str): @@ -215,7 +228,8 @@ class GaneshaNFS(object): export_template = share.to_export() logging.debug("Export template::\n{}".format(export_template)) tmp_file = self._tmpfile(export_template) - self._rados_put('ganesha-export-{}'.format(share.export_id), tmp_file.name) + self._rados_put('ganesha-export-{}'.format(share.export_id), + tmp_file.name) self._ganesha_update_export(share.export_id, tmp_file.name) def get_share(self, name: str) -> Optional[Export]: @@ -230,7 +244,8 @@ class GaneshaNFS(object): """Add a configured NFS export to Ganesha""" self._dbus_send( 'ExportMgr', 'AddExport', - 'string:{}'.format(tmp_path), 'string:EXPORT(Path={})'.format(export_path)) + 'string:{}'.format(tmp_path), + 'string:EXPORT(Path={})'.format(export_path)) def _ganesha_remove_export(self, share_id: int): """Remove a configured NFS export from Ganesha""" @@ -243,12 +258,14 @@ class GaneshaNFS(object): """Update a configured NFS export in Ganesha""" self._dbus_send( 'ExportMgr', 'UpdateExport', - 'string:{}'.format(tmp_path), 'string:EXPORT(Export_Id={})'.format(share_id)) + 'string:{}'.format(tmp_path), + 'string:EXPORT(Export_Id={})'.format(share_id)) def _dbus_send(self, section: str, action: str, *args): """Send a command to Ganesha via Dbus""" cmd = [ - 'dbus-send', '--print-reply', '--system', '--dest=org.ganesha.nfsd', + 'dbus-send', '--print-reply', '--system', + '--dest=org.ganesha.nfsd', '/org/ganesha/nfsd/{}'.format(section), 'org.ganesha.nfsd.exportmgr.{}'.format(action)] + [*args] logging.debug("About to call: {}".format(cmd)) @@ -275,7 +292,8 @@ class GaneshaNFS(object): """ try: if size_in_bytes is not None: - self._ceph_subvolume_command('create', 'ceph-fs', name, str(size_in_bytes)) + self._ceph_subvolume_command('create', 'ceph-fs', + name, str(size_in_bytes)) else: self._ceph_subvolume_command('create', 'ceph-fs', name) except subprocess.CalledProcessError: @@ -297,7 +315,9 @@ class GaneshaNFS(object): logging.error("failed to get path") return False - def _ceph_subvolume_command(self, *cmd: List[str]) -> subprocess.CompletedProcess: + def _ceph_subvolume_command( + self, *cmd: List[str] + ) -> subprocess.CompletedProcess: """Run a ceph fs subvolume command""" return self._ceph_fs_command('subvolume', *cmd) @@ -317,7 +337,10 @@ class GaneshaNFS(object): def _ceph_command(self, *cmd: List[str]) -> subprocess.CompletedProcess: """Run a ceph command""" - cmd = ["ceph", "--id", self.client_name, "--conf=/etc/ceph/ceph.conf"] + [*cmd] + cmd = [ + "ceph", "--id", self.client_name, + "--conf=/etc/ceph/ceph.conf" + ] + [*cmd] return subprocess.check_output(cmd, stderr=subprocess.DEVNULL) def _get_next_export_id(self) -> int: @@ -386,7 +409,9 @@ class GaneshaNFS(object): def _add_share_to_index(self, export_id: int): """Add an export RADOS object's URL to the RADOS URL index.""" index_data = self._rados_get(self.export_index) - url = '%url rados://{}/ganesha-export-{}'.format(self.ceph_pool, export_id) + url = '%url rados://{}/ganesha-export-{}'.format( + self.ceph_pool, export_id + ) rados_urls = index_data.split('\n') if url not in rados_urls: rados_urls.append(url) diff --git a/src/interface_ceph_nfs_peer.py b/src/interface_ceph_nfs_peer.py index 8e371b3..f00d54a 100644 --- a/src/interface_ceph_nfs_peer.py +++ b/src/interface_ceph_nfs_peer.py @@ -54,7 +54,8 @@ class CephNFSPeers(Object): def on_changed(self, event): logging.info("CephNFSPeers on_changed") logging.debug('pool_initialised: {}'.format(self.pool_initialised)) - if self.pool_initialised == 'True' and not self._stored.pool_initialised: + if self.pool_initialised == 'True' and \ + not self._stored.pool_initialised: logging.info("emiting pool initialised") self.on.pool_initialised.emit() self._stored.pool_initialised = True @@ -75,7 +76,9 @@ class CephNFSPeers(Object): self.on.pool_initialised.emit() def trigger_reload(self): - self.peer_rel.data[self.peer_rel.app]['reload_nonce'] = str(uuid.uuid4()) + self.peer_rel.data[ + self.peer_rel.app + ]['reload_nonce'] = str(uuid.uuid4()) self.on.reload_nonce.emit() @property diff --git a/src/manager.py b/src/manager.py index fd625ed..c21578b 100644 --- a/src/manager.py +++ b/src/manager.py @@ -94,8 +94,8 @@ def _conf2json(conf): word = ":" elif word == ";": word = ',' - elif (word in ['{', '}'] or - re.search(r'\A-?[1-9]\d*(\.\d+)?\Z', word)): + elif word in ['{', '}'] or \ + re.search(r'\A-?[1-9]\d*(\.\d+)?\Z', word): pass else: word = json.dumps(word) diff --git a/tests/nfs_ganesha.py b/tests/nfs_ganesha.py index 100c221..845700b 100644 --- a/tests/nfs_ganesha.py +++ b/tests/nfs_ganesha.py @@ -76,7 +76,8 @@ class NfsGaneshaTest(unittest.TestCase): }) self.assertEqual(action.status, 'completed') - def _mount_share(self, unit_name: str, share_ip: str, export_path: str, retry: bool = True): + def _mount_share(self, unit_name: str, share_ip: str, + export_path: str, retry: bool = True): self._install_dependencies(unit_name) ssh_cmd = ( 'sudo mkdir -p {0} && ' @@ -88,7 +89,8 @@ class NfsGaneshaTest(unittest.TestCase): if retry: for attempt in tenacity.Retrying( stop=tenacity.stop_after_attempt(5), - wait=tenacity.wait_exponential(multiplier=3, min=2, max=10)): + wait=tenacity.wait_exponential(multiplier=3, + min=2, max=10)): with attempt: zaza.utilities.generic.run_via_ssh( unit_name=unit_name, @@ -117,7 +119,9 @@ class NfsGaneshaTest(unittest.TestCase): stop=tenacity.stop_after_attempt(5), wait=tenacity.wait_exponential(multiplier=3, min=2, max=10)) def _verify_testing_file_on_instance(self, instance_name: str): - run_with_juju_ssh = zaza.utilities.installers.make_juju_ssh_fn('ubuntu/1', sudo=True) + run_with_juju_ssh = zaza.utilities.installers.make_juju_ssh_fn( + 'ubuntu/1', sudo=True + ) output = run_with_juju_ssh( 'sudo cat {}/test'.format(self.mount_dir)) logging.info("Verification output: {}".format(output)) @@ -126,8 +130,12 @@ class NfsGaneshaTest(unittest.TestCase): def test_create_share(self): logging.info("Creating a share") # Todo - enable ACL testing - ubuntu_0_ip = zaza.model.get_unit_public_address(zaza.model.get_unit_from_name('ubuntu/0')) - ubuntu_1_ip = zaza.model.get_unit_public_address(zaza.model.get_unit_from_name('ubuntu/1')) + ubuntu_0_ip = zaza.model.get_unit_public_address( + zaza.model.get_unit_from_name('ubuntu/0') + ) + ubuntu_1_ip = zaza.model.get_unit_public_address( + zaza.model.get_unit_from_name('ubuntu/1') + ) share = self._create_share('test_ganesha_share', access_ip=ubuntu_0_ip) # share = self._create_share('test_ganesha_share') zaza.model.wait_for_application_states(states={ @@ -164,4 +172,5 @@ class NfsGaneshaTest(unittest.TestCase): logging.debug("Action results: {}".format(results)) logging.debug("exports: {}".format(results['exports'])) exports = yaml.safe_load(results['exports']) - self.assertIn('test_ganesha_list_share', [export['name'] for export in exports]) + self.assertIn('test_ganesha_list_share', + [export['name'] for export in exports]) diff --git a/unit_tests/test_ganesha.py b/unit_tests/test_ganesha.py index 2ad4008..ba2dc19 100644 --- a/unit_tests/test_ganesha.py +++ b/unit_tests/test_ganesha.py @@ -9,7 +9,7 @@ EXPORT { # The directory in the exported file system this export # is rooted on. - Path = '/volumes/_nogroup/test_ganesha_share/e12a49ef-1b2b-40b3-ba6c-7e6695bcc950'; + Path = '/volumes/_nogroup/test_ganesha_share/e12a49ef-1b2b-40b3-ba6c'; # FSAL, Ganesha's module component FSAL { @@ -20,7 +20,7 @@ EXPORT { } # Path of export in the NFSv4 pseudo filesystem - Pseudo = '/volumes/_nogroup/test_ganesha_share/e12a49ef-1b2b-40b3-ba6c-7e6695bcc950'; + Pseudo = '/volumes/_nogroup/test_ganesha_share/e12a49ef-1b2b-40b3-ba6c'; SecType = "sys"; CLIENT { @@ -38,7 +38,8 @@ class ExportTest(unittest.TestCase): def test_parser(self): export = ganesha.Export.from_export(EXAMPLE_EXPORT) self.assertEqual(export.export_id, 1000) - self.assertEqual(export.clients, [{'Access_Type': 'rw', 'Clients': '0.0.0.0'}]) + self.assertEqual(export.clients, + [{'Access_Type': 'rw', 'Clients': '0.0.0.0'}]) self.assertEqual(export.name, 'test_ganesha_share') def test_add_client(self): @@ -57,7 +58,8 @@ class ExportTest(unittest.TestCase): self.assertEqual( export.clients, [{ - 'Access_Type': 'rw', 'Clients': '0.0.0.0, 10.0.0.0/8, 192.168.0.0/16' + 'Access_Type': 'rw', + 'Clients': '0.0.0.0, 10.0.0.0/8, 192.168.0.0/16' }]) def test_remove_client(self):