openstack/deb-manila
Code Issues Proposed changes

Merge "Bring remote and local executors into accord"

Browse Source
This commit is contained in:
Jenkins 2016-09-26 12:43:10 +00:00 committed by Gerrit Code Review
commit 856d5a3221
4 changed files with 63 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
manila/share/drivers/generic.py
View File

@ -160,6 +160,11 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
ssh_pool.remove(ssh) ssh_pool.remove(ssh)
ssh = ssh_pool.create() ssh = ssh_pool.create()
self.ssh_connections[server['instance_id']] = (ssh_pool, ssh) self.ssh_connections[server['instance_id']] = (ssh_pool, ssh)
# (aovchinnikov): ssh_execute does not behave well when passed
# parameters with spaces.
wrap = lambda token: "\"" + token + "\""
command = [wrap(tkn) if tkn.count(' ') else tkn for tkn in command]
return processutils.ssh_execute(ssh, ' '.join(command), return processutils.ssh_execute(ssh, ' '.join(command),
check_exit_code=check_exit_code) check_exit_code=check_exit_code)
@ -335,10 +340,11 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
volume): volume):
LOG.debug("Mounting '%(dev)s' to path '%(path)s' on " LOG.debug("Mounting '%(dev)s' to path '%(path)s' on "
"server '%(server)s'.", log_data) "server '%(server)s'.", log_data)
mount_cmd = ['sudo mkdir -p', mount_path, '&&'] mount_cmd = ['sudo', 'mkdir', '-p', mount_path, '&&']
mount_cmd.extend(['sudo mount', volume['mountpoint'], mount_cmd.extend(['sudo', 'mount', volume['mountpoint'],
mount_path])
mount_cmd.extend(['&&', 'sudo', 'chmod', '777',
mount_path]) mount_path])
mount_cmd.extend(['&& sudo chmod 777', mount_path])
self._ssh_exec(server_details, mount_cmd) self._ssh_exec(server_details, mount_cmd)
# Add mount permanently # Add mount permanently
@ -365,8 +371,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
if self._is_device_mounted(mount_path, server_details): if self._is_device_mounted(mount_path, server_details):
LOG.debug("Unmounting path '%(path)s' on server " LOG.debug("Unmounting path '%(path)s' on server "
"'%(server)s'.", log_data) "'%(server)s'.", log_data)
unmount_cmd = ['sudo umount', mount_path, '&& sudo rmdir', unmount_cmd = ['sudo', 'umount', mount_path, '&&', 'sudo',
mount_path] 'rmdir', mount_path]
self._ssh_exec(server_details, unmount_cmd) self._ssh_exec(server_details, unmount_cmd)
# Remove mount permanently # Remove mount permanently
self._sync_mount_temp_and_perm_files(server_details) self._sync_mount_temp_and_perm_files(server_details)

31
manila/share/drivers/helpers.py
View File

@ -289,8 +289,8 @@ class NFSHelper(NASHelperBase):
maintenance_file = self._get_maintenance_file_path(share_name) maintenance_file = self._get_maintenance_file_path(share_name)
backup_exports = [ backup_exports = [
'cat', const.NFS_EXPORTS_FILE, 'cat', const.NFS_EXPORTS_FILE,
'| grep', share_name, '|', 'grep', share_name,
'| sudo tee', maintenance_file '|', 'sudo', 'tee', maintenance_file
] ]
self._ssh_exec(server, backup_exports) self._ssh_exec(server, backup_exports)
@ -304,9 +304,9 @@ class NFSHelper(NASHelperBase):
maintenance_file = self._get_maintenance_file_path(share_name) maintenance_file = self._get_maintenance_file_path(share_name)
restore_exports = [ restore_exports = [
'cat', maintenance_file, 'cat', maintenance_file,
'| sudo tee -a', const.NFS_EXPORTS_FILE, '|', 'sudo', 'tee', '-a', const.NFS_EXPORTS_FILE,
'&& sudo exportfs -r', '&&', 'sudo', 'exportfs', '-r',
'&& sudo rm -f', maintenance_file '&&', 'sudo', 'rm', '-f', maintenance_file
] ]
self._ssh_exec(server, restore_exports) self._ssh_exec(server, restore_exports)
@ -324,10 +324,10 @@ class CIFSHelperIPAccess(NASHelperBase):
self.export_format = '\\\\%s\\%s' self.export_format = '\\\\%s\\%s'
self.parameters = { self.parameters = {
'browseable': 'yes', 'browseable': 'yes',
'\"create mask\"': '0755', 'create mask': '0755',
'\"hosts deny\"': '0.0.0.0/0', # deny all by default 'hosts deny': '0.0.0.0/0', # deny all by default
'\"hosts allow\"': '127.0.0.1', 'hosts allow': '127.0.0.1',
'\"read only\"': 'no', 'read only': 'no',
} }
def init_helper(self, server): def init_helper(self, server):
@ -406,13 +406,13 @@ class CIFSHelperIPAccess(NASHelperBase):
def _get_allow_hosts(self, server, share_name): def _get_allow_hosts(self, server, share_name):
(out, _) = self._ssh_exec(server, ['sudo', 'net', 'conf', 'getparm', (out, _) = self._ssh_exec(server, ['sudo', 'net', 'conf', 'getparm',
share_name, '\"hosts allow\"']) share_name, 'hosts allow'])
return out.split() return out.split()
def _set_allow_hosts(self, server, hosts, share_name): def _set_allow_hosts(self, server, hosts, share_name):
value = "\"" + ' '.join(hosts) + "\"" value = ' '.join(hosts) or ' '
self._ssh_exec(server, ['sudo', 'net', 'conf', 'setparm', share_name, self._ssh_exec(server, ['sudo', 'net', 'conf', 'setparm', share_name,
'\"hosts allow\"', value]) 'hosts allow', value])
@staticmethod @staticmethod
def _get_share_group_name_from_export_location(export_location): def _get_share_group_name_from_export_location(export_location):
@ -450,7 +450,8 @@ class CIFSHelperIPAccess(NASHelperBase):
allowed_hosts = " ".join(self._get_allow_hosts(server, share_name)) allowed_hosts = " ".join(self._get_allow_hosts(server, share_name))
backup_exports = [ backup_exports = [
'echo', "'%s'" % allowed_hosts, '| sudo tee', maintenance_file 'echo', "'%s'" % allowed_hosts, '|', 'sudo', 'tee',
maintenance_file
] ]
self._ssh_exec(server, backup_exports) self._ssh_exec(server, backup_exports)
self._set_allow_hosts(server, [], share_name) self._set_allow_hosts(server, [], share_name)
@ -459,7 +460,7 @@ class CIFSHelperIPAccess(NASHelperBase):
maintenance_file = self._get_maintenance_file_path(share_name) maintenance_file = self._get_maintenance_file_path(share_name)
(exports, __) = self._ssh_exec(server, ['cat', maintenance_file]) (exports, __) = self._ssh_exec(server, ['cat', maintenance_file])
self._set_allow_hosts(server, exports.split(), share_name) self._set_allow_hosts(server, exports.split(), share_name)
self._ssh_exec(server, ['sudo rm -f', maintenance_file]) self._ssh_exec(server, ['sudo', 'rm', '-f', maintenance_file])
class CIFSHelperUserAccess(CIFSHelperIPAccess): class CIFSHelperUserAccess(CIFSHelperIPAccess):
@ -512,7 +513,7 @@ class CIFSHelperUserAccess(CIFSHelperIPAccess):
return 'read list' return 'read list'
def _set_valid_users(self, server, users, share_name, access_level): def _set_valid_users(self, server, users, share_name, access_level):
value = "\"" + ' '.join(users) + "\"" value = ' '.join(users)
param = self._get_conf_param(access_level) param = self._get_conf_param(access_level)
self._ssh_exec(server, ['sudo', 'net', 'conf', 'setparm', share_name, self._ssh_exec(server, ['sudo', 'net', 'conf', 'setparm', share_name,
param, value]) param, value])

30
manila/tests/share/drivers/test_generic.py
View File

@ -397,9 +397,9 @@ class GenericShareDriverTestCase(test.TestCase):
server) server)
self._driver._ssh_exec.assert_called_once_with( self._driver._ssh_exec.assert_called_once_with(
server, server,
['sudo mkdir -p', mount_path, ['sudo', 'mkdir', '-p', mount_path,
'&&', 'sudo mount', volume['mountpoint'], mount_path, '&&', 'sudo', 'mount', volume['mountpoint'], mount_path,
'&& sudo chmod 777', mount_path], '&&', 'sudo', 'chmod', '777', mount_path],
) )
def test_mount_device_present(self): def test_mount_device_present(self):
@ -454,7 +454,7 @@ class GenericShareDriverTestCase(test.TestCase):
self.server) self.server)
self._driver._ssh_exec.assert_called_once_with( self._driver._ssh_exec.assert_called_once_with(
self.server, self.server,
['sudo umount', mount_path, '&& sudo rmdir', mount_path], ['sudo', 'umount', mount_path, '&&', 'sudo', 'rmdir', mount_path],
) )
def test_unmount_device_retry_once(self): def test_unmount_device_retry_once(self):
@ -485,8 +485,8 @@ class GenericShareDriverTestCase(test.TestCase):
self._driver._sync_mount_temp_and_perm_files.assert_called_once_with( self._driver._sync_mount_temp_and_perm_files.assert_called_once_with(
self.server) self.server)
self.assertEqual( self.assertEqual(
[mock.call(self.server, ['sudo umount', mount_path, [mock.call(self.server, ['sudo', 'umount', mount_path,
'&& sudo rmdir', mount_path]) '&&', 'sudo', 'rmdir', mount_path])
for i in moves.range(2)], for i in moves.range(2)],
self._driver._ssh_exec.mock_calls, self._driver._ssh_exec.mock_calls,
) )
@ -1366,6 +1366,24 @@ class GenericShareDriverTestCase(test.TestCase):
) )
self.assertEqual(ssh_output, result) self.assertEqual(ssh_output, result)
def test__ssh_exec_check_list_comprehensions_still_work(self):
ssh_output = 'fake_ssh_output'
cmd = ['fake', 'command spaced']
ssh = mock.Mock()
ssh_pool = mock.Mock()
ssh_pool.create = mock.Mock(side_effect=lambda: ssh)
ssh_pool.remove = mock.Mock()
self.mock_object(processutils, 'ssh_execute',
mock.Mock(return_value=ssh_output))
self._driver.ssh_connections = {
self.server['instance_id']: (ssh_pool, ssh)
}
self._driver._ssh_exec(self.server, cmd)
processutils.ssh_execute.assert_called_once_with(
ssh, 'fake "command spaced"', check_exit_code=True)
def test_get_share_stats_refresh_false(self): def test_get_share_stats_refresh_false(self):
self._driver._stats = {'fake_key': 'fake_value'} self._driver._stats = {'fake_key': 'fake_value'}

23
manila/tests/share/drivers/test_helpers.py
View File

@ -241,8 +241,8 @@ class NFSHelperTestCase(test.TestCase):
self._helper._ssh_exec.assert_any_call( self._helper._ssh_exec.assert_any_call(
self.server, self.server,
['cat', const.NFS_EXPORTS_FILE, ['cat', const.NFS_EXPORTS_FILE,
'| grep', self.share_name, '|', 'grep', self.share_name,
'| sudo tee', fake_maintenance_path] '|', 'sudo', 'tee', fake_maintenance_path]
) )
self._helper._ssh_exec.assert_any_call( self._helper._ssh_exec.assert_any_call(
self.server, self.server,
@ -264,8 +264,8 @@ class NFSHelperTestCase(test.TestCase):
self._helper._ssh_exec.assert_called_once_with( self._helper._ssh_exec.assert_called_once_with(
self.server, self.server,
['cat', fake_maintenance_path, ['cat', fake_maintenance_path,
'| sudo tee -a', const.NFS_EXPORTS_FILE, '|', 'sudo', 'tee', '-a', const.NFS_EXPORTS_FILE,
'&& sudo exportfs -r', '&& sudo rm -f', '&&', 'sudo', 'exportfs', '-r', '&&', 'sudo', 'rm', '-f',
fake_maintenance_path] fake_maintenance_path]
) )
@ -448,8 +448,8 @@ class CIFSHelperIPAccessTestCase(test.TestCase):
access_rules, [], []) access_rules, [], [])
self._helper._ssh_exec.assert_called_once_with( self._helper._ssh_exec.assert_called_once_with(
self.server_details, ['sudo', 'net', 'conf', 'setparm', self.server_details, ['sudo', 'net', 'conf', 'setparm',
self.share_name, '"hosts allow"', self.share_name, 'hosts allow',
'"1.1.1.1"']) '1.1.1.1'])
def test_get_allow_hosts(self): def test_get_allow_hosts(self):
self.mock_object(self._helper, '_ssh_exec', self.mock_object(self._helper, '_ssh_exec',
@ -460,7 +460,7 @@ class CIFSHelperIPAccessTestCase(test.TestCase):
self.server_details, self.share_name) self.server_details, self.share_name)
self.assertEqual(expected, result) self.assertEqual(expected, result)
cmd = ['sudo', 'net', 'conf', 'getparm', self.share_name, cmd = ['sudo', 'net', 'conf', 'getparm', self.share_name,
'\"hosts allow\"'] 'hosts allow']
self._helper._ssh_exec.assert_called_once_with( self._helper._ssh_exec.assert_called_once_with(
self.server_details, cmd) self.server_details, cmd)
@ -537,7 +537,8 @@ class CIFSHelperIPAccessTestCase(test.TestCase):
self.server_details, self.share_name) self.server_details, self.share_name)
self._helper._set_allow_hosts.assert_called_once_with( self._helper._set_allow_hosts.assert_called_once_with(
self.server_details, [], self.share_name) self.server_details, [], self.share_name)
valid_cmd = ['echo', "'test test2'", '| sudo tee', maintenance_path] valid_cmd = ['echo', "'test test2'", '|', 'sudo', 'tee',
maintenance_path]
self._helper._ssh_exec.assert_called_once_with( self._helper._ssh_exec.assert_called_once_with(
self.server_details, valid_cmd) self.server_details, valid_cmd)
@ -557,7 +558,7 @@ class CIFSHelperIPAccessTestCase(test.TestCase):
self._helper._ssh_exec.assert_any_call( self._helper._ssh_exec.assert_any_call(
self.server_details, ['cat', fake_maintenance_path]) self.server_details, ['cat', fake_maintenance_path])
self._helper._ssh_exec.assert_any_call( self._helper._ssh_exec.assert_any_call(
self.server_details, ['sudo rm -f', fake_maintenance_path]) self.server_details, ['sudo', 'rm', '-f', fake_maintenance_path])
@ddt.ddt @ddt.ddt
@ -601,10 +602,10 @@ class CIFSHelperUserAccessTestCase(test.TestCase):
self._helper._ssh_exec.assert_has_calls([ self._helper._ssh_exec.assert_has_calls([
mock.call(self.server_details, mock.call(self.server_details,
['sudo', 'net', 'conf', 'setparm', self.share_name, ['sudo', 'net', 'conf', 'setparm', self.share_name,
'valid users', '"user1"']), 'valid users', 'user1']),
mock.call(self.server_details, mock.call(self.server_details,
['sudo', 'net', 'conf', 'setparm', self.share_name, ['sudo', 'net', 'conf', 'setparm', self.share_name,
'read list', '"user2"']) 'read list', 'user2'])
]) ])
def test_update_access_exception_level(self): def test_update_access_exception_level(self):