diff --git a/etc/manila/rootwrap.d/share.filters b/etc/manila/rootwrap.d/share.filters index 416b466bc0..72345916ee 100644 --- a/etc/manila/rootwrap.d/share.filters +++ b/etc/manila/rootwrap.d/share.filters @@ -60,6 +60,9 @@ vgs: CommandFilter, vgs, root # manila/share/drivers/lvm.py: 'tune2fs', '-U', 'random', '%volume-snapshot%' tune2fs: CommandFilter, tune2fs, root +# manila/share/drivers/generic.py: 'sed', '-i', '\'/%s/d\'', '%s' +sed: CommandFilter, sed, root + # manila/share/drivers/glusterfs.py: 'mkdir', '%s' # manila/share/drivers/ganesha/manager.py: 'mkdir', '-p', '%s' mkdir: CommandFilter, mkdir, root diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index e398ccb3e1..f4c88c0ec5 100644 --- a/manila/share/drivers/generic.py +++ b/manila/share/drivers/generic.py @@ -285,16 +285,19 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): return True return False - def _sync_mount_temp_and_perm_files(self, server_details): - """Sync temporary and permanent files for mounted filesystems.""" + def _add_mount_permanently(self, share_id, server_details): + """Add mount permanently for mounted filesystems.""" try: self._ssh_exec( server_details, - ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE], + ['grep', share_id, const.MOUNT_FILE_TEMP, + '|', 'sudo', 'tee', '-a', const.MOUNT_FILE], ) except exception.ProcessExecutionError as e: - LOG.error("Failed to sync mount files on server '%s'.", - server_details['instance_id']) + LOG.error("Failed to add 'Share-%(share_id)s' mount " + "permanently on server '%(instance_id)s'.", + {"share_id": share_id, + "instance_id": server_details['instance_id']}) raise exception.ShareBackendException(msg=six.text_type(e)) try: # Remount it to avoid postponed point of failure @@ -302,6 +305,20 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): except exception.ProcessExecutionError as e: LOG.error("Failed to mount all shares on server '%s'.", server_details['instance_id']) + + def _remove_mount_permanently(self, share_id, server_details): + """Remove mount permanently from mounted filesystems.""" + try: + self._ssh_exec( + server_details, + ['sudo', 'sed', '-i', '\'/%s/d\'' % share_id, + const.MOUNT_FILE], + ) + except exception.ProcessExecutionError as e: + LOG.error("Failed to remove 'Share-%(share_id)s' mount " + "permanently on server '%(instance_id)s'.", + {"share_id": share_id, + "instance_id": server_details['instance_id']}) raise exception.ShareBackendException(msg=six.text_type(e)) def _mount_device(self, share, server_details, volume): @@ -342,9 +359,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): '&&', 'sudo', 'mount', device_path, mount_path, ) self._ssh_exec(server_details, mount_cmd) - - # Add mount permanently - self._sync_mount_temp_and_perm_files(server_details) + self._add_mount_permanently(share.id, server_details) else: LOG.warning("Mount point '%(path)s' already exists on " "server '%(server)s'.", log_data) @@ -370,8 +385,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): unmount_cmd = ['sudo', 'umount', mount_path, '&&', 'sudo', 'rmdir', mount_path] self._ssh_exec(server_details, unmount_cmd) - # Remove mount permanently - self._sync_mount_temp_and_perm_files(server_details) + self._remove_mount_permanently(share.id, server_details) else: LOG.warning("Mount point '%(path)s' does not exist on " "server '%(server)s'.", log_data) diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py index 942af3b4c2..a40d4a577f 100644 --- a/manila/tests/share/drivers/test_generic.py +++ b/manila/tests/share/drivers/test_generic.py @@ -307,7 +307,7 @@ class GenericShareDriverTestCase(test.TestCase): volume = {'mountpoint': 'fake_mount_point'} self.mock_object(self._driver, '_is_device_mounted', mock.Mock(return_value=False)) - self.mock_object(self._driver, '_sync_mount_temp_and_perm_files') + self.mock_object(self._driver, '_add_mount_permanently') self.mock_object(self._driver, '_ssh_exec', mock.Mock(return_value=('', ''))) @@ -315,8 +315,8 @@ class GenericShareDriverTestCase(test.TestCase): self._driver._is_device_mounted.assert_called_once_with( mount_path, server, volume) - self._driver._sync_mount_temp_and_perm_files.assert_called_once_with( - server) + self._driver._add_mount_permanently.assert_called_once_with( + self.share.id, server) self._driver._ssh_exec.assert_called_once_with( server, ( 'sudo', 'mkdir', '-p', mount_path, @@ -365,7 +365,7 @@ class GenericShareDriverTestCase(test.TestCase): mount_path = '/fake/mount/path' self.mock_object(self._driver, '_is_device_mounted', mock.Mock(return_value=True)) - self.mock_object(self._driver, '_sync_mount_temp_and_perm_files') + self.mock_object(self._driver, '_remove_mount_permanently') self.mock_object(self._driver, '_get_mount_path', mock.Mock(return_value=mount_path)) self.mock_object(self._driver, '_ssh_exec', @@ -376,8 +376,8 @@ class GenericShareDriverTestCase(test.TestCase): self._driver._get_mount_path.assert_called_once_with(self.share) self._driver._is_device_mounted.assert_called_once_with( mount_path, self.server) - self._driver._sync_mount_temp_and_perm_files.assert_called_once_with( - self.server) + self._driver._remove_mount_permanently.assert_called_once_with( + self.share.id, self.server) self._driver._ssh_exec.assert_called_once_with( self.server, ['sudo', 'umount', mount_path, '&&', 'sudo', 'rmdir', mount_path], @@ -394,7 +394,7 @@ class GenericShareDriverTestCase(test.TestCase): mount_path = '/fake/mount/path' self.mock_object(self._driver, '_is_device_mounted', mock.Mock(return_value=True)) - self.mock_object(self._driver, '_sync_mount_temp_and_perm_files') + self.mock_object(self._driver, '_remove_mount_permanently') self.mock_object(self._driver, '_get_mount_path', mock.Mock(return_value=mount_path)) self.mock_object(self._driver, '_ssh_exec', @@ -408,8 +408,8 @@ class GenericShareDriverTestCase(test.TestCase): self.assertEqual([mock.call(mount_path, self.server) for i in moves.range(2)], self._driver._is_device_mounted.mock_calls) - self._driver._sync_mount_temp_and_perm_files.assert_called_once_with( - self.server) + self._driver._remove_mount_permanently.assert_called_once_with( + self.share.id, self.server) self.assertEqual( [mock.call(self.server, ['sudo', 'umount', mount_path, '&&', 'sudo', 'rmdir', mount_path]) @@ -488,45 +488,57 @@ class GenericShareDriverTestCase(test.TestCase): self.server, ['sudo', 'mount']) self.assertFalse(result) - def test_sync_mount_temp_and_perm_files(self): + def test_add_mount_permanently(self): self.mock_object(self._driver, '_ssh_exec') - self._driver._sync_mount_temp_and_perm_files(self.server) + self._driver._add_mount_permanently(self.share.id, self.server) self._driver._ssh_exec.has_calls( mock.call( self.server, - ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]), - mock.call(self.server, ['sudo', 'mount', '-a'])) + ['grep', self.share.id, const.MOUNT_FILE_TEMP, + '|', 'sudo', 'tee', '-a', const.MOUNT_FILE]), + mock.call(self.server, ['sudo', 'mount', '-a']) + ) - def test_sync_mount_temp_and_perm_files_raise_error_on_copy(self): + def test_add_mount_permanently_raise_error_on_add(self): self.mock_object( self._driver, '_ssh_exec', mock.Mock(side_effect=exception.ProcessExecutionError)) self.assertRaises( exception.ShareBackendException, - self._driver._sync_mount_temp_and_perm_files, + self._driver._add_mount_permanently, + self.share.id, self.server ) self._driver._ssh_exec.assert_called_once_with( self.server, - ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]) + ['grep', self.share.id, const.MOUNT_FILE_TEMP, + '|', 'sudo', 'tee', '-a', const.MOUNT_FILE], + ) - def test_sync_mount_temp_and_perm_files_raise_error_on_mount(self): - def raise_error_on_mount(*args, **kwargs): - if args[1][1] == 'cp': - raise exception.ProcessExecutionError() + def test_remove_mount_permanently(self): + self.mock_object(self._driver, '_ssh_exec') + self._driver._remove_mount_permanently(self.share.id, self.server) + self._driver._ssh_exec.assert_called_once_with( + self.server, + ['sudo', 'sed', '-i', '\'/%s/d\'' % self.share.id, + const.MOUNT_FILE], + ) - self.mock_object(self._driver, '_ssh_exec', - mock.Mock(side_effect=raise_error_on_mount)) + def test_remove_mount_permanently_raise_error_on_remove(self): + self.mock_object( + self._driver, '_ssh_exec', + mock.Mock(side_effect=exception.ProcessExecutionError)) self.assertRaises( exception.ShareBackendException, - self._driver._sync_mount_temp_and_perm_files, + self._driver._remove_mount_permanently, + self.share.id, self.server ) - self._driver._ssh_exec.has_calls( - mock.call( - self.server, - ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]), - mock.call(self.server, ['sudo', 'mount', '-a'])) + self._driver._ssh_exec.assert_called_once_with( + self.server, + ['sudo', 'sed', '-i', '\'/%s/d\'' % self.share.id, + const.MOUNT_FILE], + ) def test_get_mount_path(self): result = self._driver._get_mount_path(self.share) diff --git a/releasenotes/notes/bug-1639662-fix-share-service-VM-restart-problem-1110f9133cc294e8.yaml b/releasenotes/notes/bug-1639662-fix-share-service-VM-restart-problem-1110f9133cc294e8.yaml new file mode 100644 index 0000000000..523d38028d --- /dev/null +++ b/releasenotes/notes/bug-1639662-fix-share-service-VM-restart-problem-1110f9133cc294e8.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - Changed sync mount permanently logic in the Generic + driver to select the newly mounted share from /etc/mtab + and insert it into /etc/fstab. Added corresponding + remove mount permanently functionality.