Browse Source

Fix NFS volume retype with migrate

When migrating a volume to an NFS backend, don't rename the associated
volume file if the original source volume was on the same backend (same
provider_location). Volume migration always deletes the source volume,
so the volume file should *not* be renamed to the original.

Closes-Bug: #1798468
Change-Id: Iadf537521af93bbd28452a54e0e70e7ed18f606c
(cherry picked from commit 33a3a4bf6f)
(cherry picked from commit 6f3bec3c64)
changes/22/679322/1
Alan Bishop 3 weeks ago
parent
commit
058700edc9
2 changed files with 64 additions and 70 deletions
  1. 62
    69
      cinder/tests/unit/volume/drivers/test_nfs.py
  2. 2
    1
      cinder/volume/drivers/nfs.py

+ 62
- 69
cinder/tests/unit/volume/drivers/test_nfs.py View File

@@ -28,6 +28,7 @@ from cinder import context
28 28
 from cinder import exception
29 29
 from cinder.image import image_utils
30 30
 from cinder import test
31
+from cinder.tests.unit import fake_constants as fake
31 32
 from cinder.tests.unit import fake_snapshot
32 33
 from cinder.tests.unit import fake_volume
33 34
 from cinder.volume import configuration as conf
@@ -1385,6 +1386,67 @@ class NfsDriverTestCase(test.TestCase):
1385 1386
         mock_write_info_file.assert_called_with(
1386 1387
             info_path, {'active': snap_file, fake_snap.id: snap_file})
1387 1388
 
1389
+    @ddt.data({'volume_status': 'available',
1390
+               'original_provider': 'original_provider',
1391
+               'rename_side_effect': None},
1392
+              {'volume_status': 'available',
1393
+               'original_provider': 'current_provider',
1394
+               'rename_side_effect': None},
1395
+              {'volume_status': 'in-use',
1396
+               'original_provider': 'original_provider',
1397
+               'rename_side_effect': None},
1398
+              {'volume_status': 'available',
1399
+               'original_provider': 'original_provider',
1400
+               'rename_side_effect': OSError})
1401
+    @ddt.unpack
1402
+    @mock.patch('os.rename')
1403
+    def test_update_migrated_volume(self,
1404
+                                    mock_rename,
1405
+                                    rename_side_effect,
1406
+                                    original_provider,
1407
+                                    volume_status):
1408
+        drv = nfs.NfsDriver(configuration=self.configuration)
1409
+        base_dir = '/dir_base/'
1410
+        current_path = base_dir + fake.VOLUME2_NAME
1411
+        current_provider = 'current_provider'
1412
+        mock_rename.side_effect = rename_side_effect
1413
+
1414
+        volume = fake_volume.fake_volume_obj(
1415
+            self.context,
1416
+            id=fake.VOLUME_ID,
1417
+            provider_location=original_provider,
1418
+            _name_id=None)
1419
+
1420
+        new_volume = fake_volume.fake_volume_obj(
1421
+            self.context,
1422
+            id=fake.VOLUME2_ID,
1423
+            provider_location=current_provider,
1424
+            _name_id=None)
1425
+
1426
+        with mock.patch.object(drv, 'local_path') as local_path:
1427
+            local_path.return_value = current_path
1428
+            update = drv.update_migrated_volume(self.context,
1429
+                                                volume,
1430
+                                                new_volume,
1431
+                                                volume_status)
1432
+
1433
+            if (volume_status == 'available' and
1434
+                    original_provider != current_provider):
1435
+                original_path = base_dir + fake.VOLUME_NAME
1436
+                mock_rename.assert_called_once_with(current_path,
1437
+                                                    original_path)
1438
+            else:
1439
+                mock_rename.assert_not_called()
1440
+
1441
+            if mock_rename.call_count > 0 and rename_side_effect is None:
1442
+                self.assertEqual({'_name_id': None,
1443
+                                  'provider_location': current_provider},
1444
+                                 update)
1445
+            else:
1446
+                self.assertEqual({'_name_id': fake.VOLUME2_ID,
1447
+                                  'provider_location': current_provider},
1448
+                                 update)
1449
+
1388 1450
 
1389 1451
 class NfsDriverDoSetupTestCase(test.TestCase):
1390 1452
 
@@ -1539,75 +1601,6 @@ class NfsDriverDoSetupTestCase(test.TestCase):
1539 1601
                        check_exit_code=False,
1540 1602
                        run_as_root=True)])
1541 1603
 
1542
-    @mock.patch.object(os, 'rename')
1543
-    def test_update_migrated_available_volume(self, rename_volume):
1544
-        self._test_update_migrated_volume('available', rename_volume)
1545
-
1546
-    @mock.patch.object(os, 'rename')
1547
-    def test_update_migrated_available_volume_rename_fail(self, rename_volume):
1548
-        self._test_update_migrated_volume('available', rename_volume,
1549
-                                          rename_exception=True)
1550
-
1551
-    @mock.patch.object(os, 'rename')
1552
-    def test_update_migrated_in_use_volume(self, rename_volume):
1553
-        self._test_update_migrated_volume('in-use', rename_volume)
1554
-
1555
-    def _test_update_migrated_volume(self, volume_status, rename_volume,
1556
-                                     rename_exception=False):
1557
-        drv = nfs.NfsDriver(configuration=self.configuration)
1558
-        fake_volume_id = 'f51b5730-13b7-11e6-a238-fa163e67a298'
1559
-        fake_new_volume_id = '12341234-13b7-11e6-a238-fa163e67a298'
1560
-        fake_provider_source = 'fake_provider_source'
1561
-        fake_provider = 'fake_provider'
1562
-        base_dir = '/dir_base/'
1563
-        volume_name_template = 'volume-%s'
1564
-        original_volume_name = volume_name_template % fake_volume_id
1565
-        current_name = volume_name_template % fake_new_volume_id
1566
-        original_volume_path = base_dir + original_volume_name
1567
-        current_path = base_dir + current_name
1568
-        volume = fake_volume.fake_volume_obj(
1569
-            self.context,
1570
-            id=fake_volume_id,
1571
-            size=1,
1572
-            provider_location=fake_provider_source,
1573
-            _name_id=None)
1574
-
1575
-        new_volume = fake_volume.fake_volume_obj(
1576
-            self.context,
1577
-            id=fake_new_volume_id,
1578
-            size=1,
1579
-            provider_location=fake_provider,
1580
-            _name_id=None)
1581
-
1582
-        with mock.patch.object(drv, 'local_path') as local_path:
1583
-            local_path.return_value = base_dir + current_name
1584
-            if volume_status == 'in-use':
1585
-                update = drv.update_migrated_volume(self.context,
1586
-                                                    volume,
1587
-                                                    new_volume,
1588
-                                                    volume_status)
1589
-                self.assertEqual({'_name_id': fake_new_volume_id,
1590
-                                  'provider_location': fake_provider}, update)
1591
-            elif rename_exception:
1592
-                rename_volume.side_effect = OSError
1593
-                update = drv.update_migrated_volume(self.context,
1594
-                                                    volume,
1595
-                                                    new_volume,
1596
-                                                    volume_status)
1597
-                rename_volume.assert_called_once_with(current_path,
1598
-                                                      original_volume_path)
1599
-                self.assertEqual({'_name_id': fake_new_volume_id,
1600
-                                  'provider_location': fake_provider}, update)
1601
-            else:
1602
-                update = drv.update_migrated_volume(self.context,
1603
-                                                    volume,
1604
-                                                    new_volume,
1605
-                                                    volume_status)
1606
-                rename_volume.assert_called_once_with(current_path,
1607
-                                                      original_volume_path)
1608
-                self.assertEqual({'_name_id': None,
1609
-                                  'provider_location': fake_provider}, update)
1610
-
1611 1604
     def test_retype_is_there(self):
1612 1605
         "Ensure that driver.retype() is there."""
1613 1606
 

+ 2
- 1
cinder/volume/drivers/nfs.py View File

@@ -457,7 +457,8 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed):
457 457
         :returns: model_update to update DB with any needed changes
458 458
         """
459 459
         name_id = None
460
-        if original_volume_status == 'available':
460
+        if (original_volume_status == 'available' and
461
+                volume.provider_location != new_volume.provider_location):
461 462
             current_name = CONF.volume_name_template % new_volume.id
462 463
             original_volume_name = CONF.volume_name_template % volume.id
463 464
             current_path = self.local_path(new_volume)

Loading…
Cancel
Save