Browse Source

Fix share-service VM restart problem

The /etc/mtab file may have mount information such as 'nfsd'
that if copied to /etc/fstab will cause the share server to
hang when rebooted.

Update /etc/fstab with exactly the newly mounted or unmounted
shares rather than simply overwriting /etc/fstab with /etc/mtab.

Change-Id: I67602bae1f928769d768008deca7bd0f2fef1ac2
Close-Bug: #1639662
(cherry picked from commit b76934770b)
(cherry picked from commit 344d4aa91c)
(cherry picked from commit 7b306c0676)
tags/4.0.2
Xiaoyang Zhang 2 years ago
parent
commit
891d2deeee

+ 3
- 0
etc/manila/rootwrap.d/share.filters View File

@@ -60,6 +60,9 @@ vgs: CommandFilter, vgs, root
60 60
 # manila/share/drivers/lvm.py: 'tune2fs', '-U', 'random', '%volume-snapshot%'
61 61
 tune2fs: CommandFilter, tune2fs, root
62 62
 
63
+# manila/share/drivers/generic.py: 'sed', '-i', '\'/%s/d\'', '%s'
64
+sed: CommandFilter, sed, root
65
+
63 66
 # manila/share/drivers/glusterfs.py: 'mkdir', '%s'
64 67
 # manila/share/drivers/ganesha/manager.py: 'mkdir', '-p', '%s'
65 68
 mkdir: CommandFilter, mkdir, root

+ 24
- 10
manila/share/drivers/generic.py View File

@@ -285,16 +285,19 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
285 285
                     return True
286 286
         return False
287 287
 
288
-    def _sync_mount_temp_and_perm_files(self, server_details):
289
-        """Sync temporary and permanent files for mounted filesystems."""
288
+    def _add_mount_permanently(self, share_id, server_details):
289
+        """Add mount permanently for mounted filesystems."""
290 290
         try:
291 291
             self._ssh_exec(
292 292
                 server_details,
293
-                ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE],
293
+                ['grep', share_id, const.MOUNT_FILE_TEMP,
294
+                 '|', 'sudo', 'tee', '-a', const.MOUNT_FILE],
294 295
             )
295 296
         except exception.ProcessExecutionError as e:
296
-            LOG.error(_LE("Failed to sync mount files on server '%s'."),
297
-                      server_details['instance_id'])
297
+            LOG.error(_LE("Failed to add 'Share-%(share_id)s' mount "
298
+                      "permanently on server '%(instance_id)s'."),
299
+                      {"share_id": share_id,
300
+                       "instance_id": server_details['instance_id']})
298 301
             raise exception.ShareBackendException(msg=six.text_type(e))
299 302
         try:
300 303
             # Remount it to avoid postponed point of failure
@@ -302,6 +305,20 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
302 305
         except exception.ProcessExecutionError as e:
303 306
             LOG.error(_LE("Failed to mount all shares on server '%s'."),
304 307
                       server_details['instance_id'])
308
+
309
+    def _remove_mount_permanently(self, share_id, server_details):
310
+        """Remove mount permanently from mounted filesystems."""
311
+        try:
312
+            self._ssh_exec(
313
+                server_details,
314
+                ['sudo', 'sed', '-i', '\'/%s/d\'' % share_id,
315
+                 const.MOUNT_FILE],
316
+            )
317
+        except exception.ProcessExecutionError as e:
318
+            LOG.error(_LE("Failed to remove 'Share-%(share_id)s' mount "
319
+                      "permanently on server '%(instance_id)s'."),
320
+                      {"share_id": share_id,
321
+                       "instance_id": server_details['instance_id']})
305 322
             raise exception.ShareBackendException(msg=six.text_type(e))
306 323
 
307 324
     def _mount_device(self, share, server_details, volume):
@@ -342,9 +359,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
342 359
                         '&&', 'sudo', 'mount', device_path, mount_path,
343 360
                     )
344 361
                     self._ssh_exec(server_details, mount_cmd)
345
-
346
-                    # Add mount permanently
347
-                    self._sync_mount_temp_and_perm_files(server_details)
362
+                    self._add_mount_permanently(share.id, server_details)
348 363
                 else:
349 364
                     LOG.warning(_LW("Mount point '%(path)s' already exists on "
350 365
                                     "server '%(server)s'."), log_data)
@@ -370,8 +385,7 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
370 385
                 unmount_cmd = ['sudo', 'umount', mount_path, '&&', 'sudo',
371 386
                                'rmdir', mount_path]
372 387
                 self._ssh_exec(server_details, unmount_cmd)
373
-                # Remove mount permanently
374
-                self._sync_mount_temp_and_perm_files(server_details)
388
+                self._remove_mount_permanently(share.id, server_details)
375 389
             else:
376 390
                 LOG.warning(_LW("Mount point '%(path)s' does not exist on "
377 391
                                 "server '%(server)s'."), log_data)

+ 40
- 28
manila/tests/share/drivers/test_generic.py View File

@@ -307,7 +307,7 @@ class GenericShareDriverTestCase(test.TestCase):
307 307
         volume = {'mountpoint': 'fake_mount_point'}
308 308
         self.mock_object(self._driver, '_is_device_mounted',
309 309
                          mock.Mock(return_value=False))
310
-        self.mock_object(self._driver, '_sync_mount_temp_and_perm_files')
310
+        self.mock_object(self._driver, '_add_mount_permanently')
311 311
         self.mock_object(self._driver, '_ssh_exec',
312 312
                          mock.Mock(return_value=('', '')))
313 313
 
@@ -315,8 +315,8 @@ class GenericShareDriverTestCase(test.TestCase):
315 315
 
316 316
         self._driver._is_device_mounted.assert_called_once_with(
317 317
             mount_path, server, volume)
318
-        self._driver._sync_mount_temp_and_perm_files.assert_called_once_with(
319
-            server)
318
+        self._driver._add_mount_permanently.assert_called_once_with(
319
+            self.share.id, server)
320 320
         self._driver._ssh_exec.assert_called_once_with(
321 321
             server, (
322 322
                 'sudo', 'mkdir', '-p', mount_path,
@@ -365,7 +365,7 @@ class GenericShareDriverTestCase(test.TestCase):
365 365
         mount_path = '/fake/mount/path'
366 366
         self.mock_object(self._driver, '_is_device_mounted',
367 367
                          mock.Mock(return_value=True))
368
-        self.mock_object(self._driver, '_sync_mount_temp_and_perm_files')
368
+        self.mock_object(self._driver, '_remove_mount_permanently')
369 369
         self.mock_object(self._driver, '_get_mount_path',
370 370
                          mock.Mock(return_value=mount_path))
371 371
         self.mock_object(self._driver, '_ssh_exec',
@@ -376,8 +376,8 @@ class GenericShareDriverTestCase(test.TestCase):
376 376
         self._driver._get_mount_path.assert_called_once_with(self.share)
377 377
         self._driver._is_device_mounted.assert_called_once_with(
378 378
             mount_path, self.server)
379
-        self._driver._sync_mount_temp_and_perm_files.assert_called_once_with(
380
-            self.server)
379
+        self._driver._remove_mount_permanently.assert_called_once_with(
380
+            self.share.id, self.server)
381 381
         self._driver._ssh_exec.assert_called_once_with(
382 382
             self.server,
383 383
             ['sudo', 'umount', mount_path, '&&', 'sudo', 'rmdir', mount_path],
@@ -394,7 +394,7 @@ class GenericShareDriverTestCase(test.TestCase):
394 394
         mount_path = '/fake/mount/path'
395 395
         self.mock_object(self._driver, '_is_device_mounted',
396 396
                          mock.Mock(return_value=True))
397
-        self.mock_object(self._driver, '_sync_mount_temp_and_perm_files')
397
+        self.mock_object(self._driver, '_remove_mount_permanently')
398 398
         self.mock_object(self._driver, '_get_mount_path',
399 399
                          mock.Mock(return_value=mount_path))
400 400
         self.mock_object(self._driver, '_ssh_exec',
@@ -408,8 +408,8 @@ class GenericShareDriverTestCase(test.TestCase):
408 408
         self.assertEqual([mock.call(mount_path,
409 409
                                     self.server) for i in moves.range(2)],
410 410
                          self._driver._is_device_mounted.mock_calls)
411
-        self._driver._sync_mount_temp_and_perm_files.assert_called_once_with(
412
-            self.server)
411
+        self._driver._remove_mount_permanently.assert_called_once_with(
412
+            self.share.id, self.server)
413 413
         self.assertEqual(
414 414
             [mock.call(self.server, ['sudo', 'umount', mount_path,
415 415
                                      '&&', 'sudo', 'rmdir', mount_path])
@@ -488,45 +488,57 @@ class GenericShareDriverTestCase(test.TestCase):
488 488
             self.server, ['sudo', 'mount'])
489 489
         self.assertFalse(result)
490 490
 
491
-    def test_sync_mount_temp_and_perm_files(self):
491
+    def test_add_mount_permanently(self):
492 492
         self.mock_object(self._driver, '_ssh_exec')
493
-        self._driver._sync_mount_temp_and_perm_files(self.server)
493
+        self._driver._add_mount_permanently(self.share.id, self.server)
494 494
         self._driver._ssh_exec.has_calls(
495 495
             mock.call(
496 496
                 self.server,
497
-                ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]),
498
-            mock.call(self.server, ['sudo', 'mount', '-a']))
497
+                ['grep', self.share.id, const.MOUNT_FILE_TEMP,
498
+                 '|', 'sudo', 'tee', '-a', const.MOUNT_FILE]),
499
+            mock.call(self.server, ['sudo', 'mount', '-a'])
500
+        )
499 501
 
500
-    def test_sync_mount_temp_and_perm_files_raise_error_on_copy(self):
502
+    def test_add_mount_permanently_raise_error_on_add(self):
501 503
         self.mock_object(
502 504
             self._driver, '_ssh_exec',
503 505
             mock.Mock(side_effect=exception.ProcessExecutionError))
504 506
         self.assertRaises(
505 507
             exception.ShareBackendException,
506
-            self._driver._sync_mount_temp_and_perm_files,
508
+            self._driver._add_mount_permanently,
509
+            self.share.id,
507 510
             self.server
508 511
         )
509 512
         self._driver._ssh_exec.assert_called_once_with(
510 513
             self.server,
511
-            ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE])
514
+            ['grep', self.share.id, const.MOUNT_FILE_TEMP,
515
+             '|', 'sudo', 'tee', '-a', const.MOUNT_FILE],
516
+        )
512 517
 
513
-    def test_sync_mount_temp_and_perm_files_raise_error_on_mount(self):
514
-        def raise_error_on_mount(*args, **kwargs):
515
-            if args[1][1] == 'cp':
516
-                raise exception.ProcessExecutionError()
518
+    def test_remove_mount_permanently(self):
519
+        self.mock_object(self._driver, '_ssh_exec')
520
+        self._driver._remove_mount_permanently(self.share.id, self.server)
521
+        self._driver._ssh_exec.assert_called_once_with(
522
+            self.server,
523
+            ['sudo', 'sed', '-i', '\'/%s/d\'' % self.share.id,
524
+             const.MOUNT_FILE],
525
+        )
517 526
 
518
-        self.mock_object(self._driver, '_ssh_exec',
519
-                         mock.Mock(side_effect=raise_error_on_mount))
527
+    def test_remove_mount_permanently_raise_error_on_remove(self):
528
+        self.mock_object(
529
+            self._driver, '_ssh_exec',
530
+            mock.Mock(side_effect=exception.ProcessExecutionError))
520 531
         self.assertRaises(
521 532
             exception.ShareBackendException,
522
-            self._driver._sync_mount_temp_and_perm_files,
533
+            self._driver._remove_mount_permanently,
534
+            self.share.id,
523 535
             self.server
524 536
         )
525
-        self._driver._ssh_exec.has_calls(
526
-            mock.call(
527
-                self.server,
528
-                ['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]),
529
-            mock.call(self.server, ['sudo', 'mount', '-a']))
537
+        self._driver._ssh_exec.assert_called_once_with(
538
+            self.server,
539
+            ['sudo', 'sed', '-i', '\'/%s/d\'' % self.share.id,
540
+             const.MOUNT_FILE],
541
+        )
530 542
 
531 543
     def test_get_mount_path(self):
532 544
         result = self._driver._get_mount_path(self.share)

+ 6
- 0
releasenotes/notes/bug-1639662-fix-share-service-VM-restart-problem-1110f9133cc294e8.yaml View File

@@ -0,0 +1,6 @@
1
+---
2
+fixes:
3
+  - Changed sync mount permanently logic in the Generic
4
+    driver to select the newly mounted share from /etc/mtab
5
+    and insert it into /etc/fstab. Added corresponding
6
+    remove mount permanently functionality.

Loading…
Cancel
Save