openstack/charm-ceph-nfs
Code Issues Proposed changes

Fixes currently present PEP8 issues

Browse Source 
Change-Id: Idff27970dc0d41288444fc3ed18585e3f3a1a0ad
This commit is contained in:
utkarshbhatthere 2022-08-08 18:57:23 +05:30
parent 79f488332c
commit c1d2f79c22
No known key found for this signature in database
GPG Key ID: 8AFC279E7CD87430
6 changed files with 89 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/charm.py
View File

@ -88,7 +88,9 @@ class CephNFSContext(object):
:returns: Data pool name. :returns: Data pool name.
:rtype: str :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 @property
def client_name(self): def client_name(self):
@ -293,7 +295,8 @@ class CephNFSCharm(
mode=0o750) mode=0o750)
def daemon_reload_and_restart(service_name): 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', 'daemon-reload'])
subprocess.check_call(['systemctl', 'restart', service_name]) subprocess.check_call(['systemctl', 'restart', service_name])
@ -411,7 +414,8 @@ class CephNFSCharm(
def create_share_action(self, event): def create_share_action(self, event):
if not self.model.unit.is_leader(): 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 return
share_size = event.params.get('size') share_size = event.params.get('size')
name = event.params.get('name') name = event.params.get('name')
@ -420,7 +424,8 @@ class CephNFSCharm(
export_path = self.ganesha_client.create_share( export_path = self.ganesha_client.create_share(
size=share_size, name=name, access_ips=allowed_ips) size=share_size, name=name, access_ips=allowed_ips)
if not export_path: 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 return
self.peers.trigger_reload() self.peers.trigger_reload()
event.set_results({ event.set_results({
@ -431,12 +436,17 @@ class CephNFSCharm(
def list_shares_action(self, event): def list_shares_action(self, event):
exports = self.ganesha_client.list_shares() exports = self.ganesha_client.list_shares()
event.set_results({ 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): def delete_share_action(self, event):
if not self.model.unit.is_leader(): 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 return
name = event.params.get('name') name = event.params.get('name')
purge = event.params.get('purge') purge = event.params.get('purge')
@ -448,7 +458,8 @@ class CephNFSCharm(
def grant_access_action(self, event): def grant_access_action(self, event):
if not self.model.unit.is_leader(): 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 return
name = event.params.get('name') name = event.params.get('name')
address = event.params.get('client') address = event.params.get('client')
@ -463,7 +474,8 @@ class CephNFSCharm(
def revoke_access_action(self, event): def revoke_access_action(self, event):
if not self.model.unit.is_leader(): 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 return
name = event.params.get('name') name = event.params.get('name')
address = event.params.get('client') address = event.params.get('client')

57
src/ganesha.py
View File

@ -26,7 +26,9 @@ class Export(object):
raise RuntimeError('export_options must be a dictionary') raise RuntimeError('export_options must be a dictionary')
self.export_options = export_options self.export_options = export_options
if not isinstance(self.export_options['EXPORT']['CLIENT'], list): 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': def from_export(export: str) -> 'Export':
return Export(export_options=manager.parseconf(export)) return Export(export_options=manager.parseconf(export))
@ -52,9 +54,13 @@ class Export(object):
clients_by_mode = {'r': [], 'rw': []} clients_by_mode = {'r': [], 'rw': []}
for client in self.clients: for client in self.clients:
if client['Access_Type'].lower() == 'r': 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': 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: else:
raise RuntimeError("Invalid access type") raise RuntimeError("Invalid access type")
return clients_by_mode return clients_by_mode
@ -84,7 +90,9 @@ class Export(object):
def remove_client(self, client: str): def remove_client(self, client: str):
clients_by_mode = self.clients_by_mode clients_by_mode = self.clients_by_mode
for (mode, clients) in clients_by_mode.items(): 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'] = [] self.export_options['EXPORT']['CLIENT'] = []
for (mode, clients) in clients_by_mode.items(): for (mode, clients) in clients_by_mode.items():
if clients: if clients:
@ -112,7 +120,9 @@ class GaneshaNFS(object):
if name is None: if name is None:
name = str(uuid.uuid4()) name = str(uuid.uuid4())
else: 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: if existing_shares:
return existing_shares[0].path return existing_shares[0].path
if size is not None: if size is not None:
@ -179,7 +189,8 @@ class GaneshaNFS(object):
def resize_share(self, name: str, size: int): def resize_share(self, name: str, size: int):
size_in_bytes = size * 1024 * 1024 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): def delete_share(self, name: str, purge=False):
share = [share for share in self.list_shares() if share.name == name] share = [share for share in self.list_shares() if share.name == name]
@ -187,7 +198,8 @@ class GaneshaNFS(object):
share = share[0] share = share[0]
else: else:
return 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) self._ganesha_remove_export(share.export_id)
logging.debug("Removing export from index") logging.debug("Removing export from index")
self._remove_share_from_index(share.export_id) self._remove_share_from_index(share.export_id)
@ -204,7 +216,8 @@ class GaneshaNFS(object):
export_template = share.to_export() export_template = share.to_export()
logging.debug("Export template::\n{}".format(export_template)) logging.debug("Export template::\n{}".format(export_template))
tmp_file = self._tmpfile(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) self._ganesha_update_export(share.export_id, tmp_file.name)
def revoke_access(self, name: str, client: str): def revoke_access(self, name: str, client: str):
@ -215,7 +228,8 @@ class GaneshaNFS(object):
export_template = share.to_export() export_template = share.to_export()
logging.debug("Export template::\n{}".format(export_template)) logging.debug("Export template::\n{}".format(export_template))
tmp_file = self._tmpfile(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) self._ganesha_update_export(share.export_id, tmp_file.name)
def get_share(self, name: str) -> Optional[Export]: def get_share(self, name: str) -> Optional[Export]:
@ -230,7 +244,8 @@ class GaneshaNFS(object):
"""Add a configured NFS export to Ganesha""" """Add a configured NFS export to Ganesha"""
self._dbus_send( self._dbus_send(
'ExportMgr', 'AddExport', '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): def _ganesha_remove_export(self, share_id: int):
"""Remove a configured NFS export from Ganesha""" """Remove a configured NFS export from Ganesha"""
@ -243,12 +258,14 @@ class GaneshaNFS(object):
"""Update a configured NFS export in Ganesha""" """Update a configured NFS export in Ganesha"""
self._dbus_send( self._dbus_send(
'ExportMgr', 'UpdateExport', '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): def _dbus_send(self, section: str, action: str, *args):
"""Send a command to Ganesha via Dbus""" """Send a command to Ganesha via Dbus"""
cmd = [ 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/{}'.format(section),
'org.ganesha.nfsd.exportmgr.{}'.format(action)] + [*args] 'org.ganesha.nfsd.exportmgr.{}'.format(action)] + [*args]
logging.debug("About to call: {}".format(cmd)) logging.debug("About to call: {}".format(cmd))
@ -275,7 +292,8 @@ class GaneshaNFS(object):
""" """
try: try:
if size_in_bytes is not None: 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: else:
self._ceph_subvolume_command('create', 'ceph-fs', name) self._ceph_subvolume_command('create', 'ceph-fs', name)
except subprocess.CalledProcessError: except subprocess.CalledProcessError:
@ -297,7 +315,9 @@ class GaneshaNFS(object):
logging.error("failed to get path") logging.error("failed to get path")
return False 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""" """Run a ceph fs subvolume command"""
return self._ceph_fs_command('subvolume', *cmd) return self._ceph_fs_command('subvolume', *cmd)
@ -317,7 +337,10 @@ class GaneshaNFS(object):
def _ceph_command(self, *cmd: List[str]) -> subprocess.CompletedProcess: def _ceph_command(self, *cmd: List[str]) -> subprocess.CompletedProcess:
"""Run a ceph command""" """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) return subprocess.check_output(cmd, stderr=subprocess.DEVNULL)
def _get_next_export_id(self) -> int: def _get_next_export_id(self) -> int:
@ -386,7 +409,9 @@ class GaneshaNFS(object):
def _add_share_to_index(self, export_id: int): def _add_share_to_index(self, export_id: int):
"""Add an export RADOS object's URL to the RADOS URL index.""" """Add an export RADOS object's URL to the RADOS URL index."""
index_data = self._rados_get(self.export_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') rados_urls = index_data.split('\n')
if url not in rados_urls: if url not in rados_urls:
rados_urls.append(url) rados_urls.append(url)

7
src/interface_ceph_nfs_peer.py
View File

@ -54,7 +54,8 @@ class CephNFSPeers(Object):
def on_changed(self, event): def on_changed(self, event):
logging.info("CephNFSPeers on_changed") logging.info("CephNFSPeers on_changed")
logging.debug('pool_initialised: {}'.format(self.pool_initialised)) 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") logging.info("emiting pool initialised")
self.on.pool_initialised.emit() self.on.pool_initialised.emit()
self._stored.pool_initialised = True self._stored.pool_initialised = True
@ -75,7 +76,9 @@ class CephNFSPeers(Object):
self.on.pool_initialised.emit() self.on.pool_initialised.emit()
def trigger_reload(self): 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() self.on.reload_nonce.emit()
@property @property

4
src/manager.py
View File

@ -94,8 +94,8 @@ def _conf2json(conf):
word = ":" word = ":"
elif word == ";": elif word == ";":
word = ',' word = ','
elif (word in ['{', '}'] or elif word in ['{', '}'] or \
re.search(r'\A-?[1-9]\d*(\.\d+)?\Z', word)): re.search(r'\A-?[1-9]\d*(\.\d+)?\Z', word):
pass pass
else: else:
word = json.dumps(word) word = json.dumps(word)

21
tests/nfs_ganesha.py
View File

@ -76,7 +76,8 @@ class NfsGaneshaTest(unittest.TestCase):
}) })
self.assertEqual(action.status, 'completed') 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) self._install_dependencies(unit_name)
ssh_cmd = ( ssh_cmd = (
'sudo mkdir -p {0} && ' 'sudo mkdir -p {0} && '
@ -88,7 +89,8 @@ class NfsGaneshaTest(unittest.TestCase):
if retry: if retry:
for attempt in tenacity.Retrying( for attempt in tenacity.Retrying(
stop=tenacity.stop_after_attempt(5), 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: with attempt:
zaza.utilities.generic.run_via_ssh( zaza.utilities.generic.run_via_ssh(
unit_name=unit_name, unit_name=unit_name,
@ -117,7 +119,9 @@ class NfsGaneshaTest(unittest.TestCase):
stop=tenacity.stop_after_attempt(5), 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))
def _verify_testing_file_on_instance(self, instance_name: str): 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( output = run_with_juju_ssh(
'sudo cat {}/test'.format(self.mount_dir)) 'sudo cat {}/test'.format(self.mount_dir))
logging.info("Verification output: {}".format(output)) logging.info("Verification output: {}".format(output))
@ -126,8 +130,12 @@ class NfsGaneshaTest(unittest.TestCase):
def test_create_share(self): def test_create_share(self):
logging.info("Creating a share") logging.info("Creating a share")
# Todo - enable ACL testing # Todo - enable ACL testing
ubuntu_0_ip = zaza.model.get_unit_public_address(zaza.model.get_unit_from_name('ubuntu/0')) ubuntu_0_ip = zaza.model.get_unit_public_address(
ubuntu_1_ip = zaza.model.get_unit_public_address(zaza.model.get_unit_from_name('ubuntu/1')) 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', access_ip=ubuntu_0_ip)
# share = self._create_share('test_ganesha_share') # share = self._create_share('test_ganesha_share')
zaza.model.wait_for_application_states(states={ zaza.model.wait_for_application_states(states={
@ -164,4 +172,5 @@ class NfsGaneshaTest(unittest.TestCase):
logging.debug("Action results: {}".format(results)) logging.debug("Action results: {}".format(results))
logging.debug("exports: {}".format(results['exports'])) logging.debug("exports: {}".format(results['exports']))
exports = yaml.safe_load(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])

10
unit_tests/test_ganesha.py
View File

@ -9,7 +9,7 @@ EXPORT {
# The directory in the exported file system this export # The directory in the exported file system this export
# is rooted on. # 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, Ganesha's module component
FSAL { FSAL {
@ -20,7 +20,7 @@ EXPORT {
} }
# Path of export in the NFSv4 pseudo filesystem # 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"; SecType = "sys";
CLIENT { CLIENT {
@ -38,7 +38,8 @@ class ExportTest(unittest.TestCase):
def test_parser(self): def test_parser(self):
export = ganesha.Export.from_export(EXAMPLE_EXPORT) export = ganesha.Export.from_export(EXAMPLE_EXPORT)
self.assertEqual(export.export_id, 1000) 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') self.assertEqual(export.name, 'test_ganesha_share')
def test_add_client(self): def test_add_client(self):
@ -57,7 +58,8 @@ class ExportTest(unittest.TestCase):
self.assertEqual( self.assertEqual(
export.clients, 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): def test_remove_client(self):