Use the loop device in fstab instead of image file

When adding an fstab entry for a loopback device use the explicit
loopback device name rather than the source image file. This prevents
a new loopback device being created implitcitly when mounting the
image.

The unit tests needed updating to reflect that the loopback
device name is used when creating mountpoint names rather than than
the name of the image file. This was pre-existing behaviour.

Change-Id: Ide074310bf7121f1179e0b5237dff6f3da88e24e
Closes-Bug: #1762390

lib/swift_storage_utils.py

@@ -30,6 +30,8 @@ from charmhelpers.core.unitdata import (
     Storage as KVStore,
 )
 
+import charmhelpers.core.fstab
+
 from charmhelpers.core.host import (
     mkdir,
     mount,
@@ -474,12 +476,18 @@ def setup_storage():
 
         options = None
         loopback_device = is_mapped_loopback_device(dev)
-
+        mountpoint = '/srv/node/%s' % basename
         if loopback_device:
-            dev = loopback_device
+            # If an exiting fstab entry exists using the image file as the
+            # source then preserve it, otherwise use the loopback device
+            # directly to avoid a secound implicit loopback device being
+            # created on mount. Bug #1762390
+            fstab = charmhelpers.core.fstab.Fstab()
+            fstab_entry = fstab.get_entry_by_attr('mountpoint', mountpoint)
+            if fstab_entry and loopback_device == fstab_entry.device:
+                dev = loopback_device
             options = "loop,defaults"
 
-        mountpoint = '/srv/node/%s' % basename
         filesystem = "xfs"
 
         mount(dev, mountpoint, filesystem=filesystem)

unit_tests/test_swift_storage_utils.py

@@ -241,12 +241,13 @@ class SwiftStorageUtilsTests(CharmTestCase):
             ['mkfs.xfs', '-f', '-i', 'size=1024', '/dev/sdb']
         )
 
+    @patch.object(swift_utils.charmhelpers.core.fstab, "Fstab")
     @patch.object(swift_utils, 'is_device_in_ring')
     @patch.object(swift_utils, 'clean_storage')
     @patch.object(swift_utils, 'mkfs_xfs')
     @patch.object(swift_utils, 'determine_block_devices')
     def test_setup_storage_no_overwrite(self, determine, mkfs, clean,
-                                        mock_is_device_in_ring):
+                                        mock_is_device_in_ring, mock_Fstab):
         mock_is_device_in_ring.return_value = False
         determine.return_value = ['/dev/vdb']
         swift_utils.setup_storage()
@@ -417,22 +418,31 @@ class SwiftStorageUtilsTests(CharmTestCase):
         for service in services:
             self.assertIn(call(service), self.service_restart.call_args_list)
 
+    @patch.object(swift_utils.charmhelpers.core.fstab, "Fstab")
     @patch.object(swift_utils, "is_device_in_ring")
     @patch.object(swift_utils, "mkfs_xfs")
     @patch.object(swift_utils, "determine_block_devices")
-    def test_setup_storage_img(self, determine, mkfs, mock_is_device_in_ring):
+    def test_setup_storage_img(self, determine, mkfs, mock_is_device_in_ring,
+                               mock_Fstab):
+
+        class MockFstab(object):
+
+            def get_entry_by_attr(self, x, y):
+                return None
+
+        mock_Fstab.return_value = MockFstab()
         mock_is_device_in_ring.return_value = False
-        determine.return_value = ["/srv/test.img", ]
+        determine.return_value = ["/dev/loop0", ]
         self.is_mapped_loopback_device.return_value = "/srv/test.img"
         swift_utils.setup_storage()
         self.mount.assert_called_with(
-            "/srv/test.img",
-            "/srv/node/test.img",
+            "/dev/loop0",
+            "/srv/node/loop0",
             filesystem="xfs",
         )
         self.fstab_add.assert_called_with(
-            '/srv/test.img',
-            '/srv/node/test.img',
+            '/dev/loop0',
+            '/srv/node/loop0',
             'xfs',
             options='loop,defaults'
         )
@@ -440,7 +450,49 @@ class SwiftStorageUtilsTests(CharmTestCase):
         self.mkdir.assert_has_calls([
             call('/srv/node', owner='swift', group='swift',
                  perms=0o755),
-            call('/srv/node/test.img', group='swift', owner='swift')
+            call('/srv/node/loop0', group='swift', owner='swift')
+        ])
+
+    @patch.object(swift_utils.charmhelpers.core.fstab, "Fstab")
+    @patch.object(swift_utils, "is_device_in_ring")
+    @patch.object(swift_utils, "mkfs_xfs")
+    @patch.object(swift_utils, "determine_block_devices")
+    def test_setup_storage_img_reuse_fstab_entry(self, determine, mkfs,
+                                                 mock_is_device_in_ring,
+                                                 mock_Fstab):
+
+        FstabEntry = namedtuple('FstabEntry', ['mountpoint', 'device'])
+        class MockFstab(object):
+
+            def __init__(self):
+                self.device = '/srv/test.img'
+
+            def get_entry_by_attr(self, x, y):
+                return FstabEntry(
+                    mountpoint='/srv/node/test.img',
+                    device='/srv/test.img')
+
+        mock_Fstab.return_value = MockFstab()
+        mock_is_device_in_ring.return_value = False
+        determine.return_value = ["/dev/loop0", ]
+        self.is_mapped_loopback_device.return_value = "/srv/test.img"
+        swift_utils.setup_storage()
+        self.mount.assert_called_with(
+            "/srv/test.img",
+            "/srv/node/loop0",
+            filesystem="xfs",
+        )
+        self.fstab_add.assert_called_with(
+            '/srv/test.img',
+            '/srv/node/loop0',
+            'xfs',
+            options='loop,defaults'
+        )
+
+        self.mkdir.assert_has_calls([
+            call('/srv/node', owner='swift', group='swift',
+                 perms=0o755),
+            call('/srv/node/loop0', group='swift', owner='swift')
         ])
 
     @patch.object(swift_utils.subprocess, "check_output")