From 6346a7458b7cf5c8bb89873dc833f22115fe31de Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 10 Apr 2018 10:49:44 +0000 Subject: [PATCH] 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 | 14 ++++-- unit_tests/test_swift_storage_utils.py | 68 +++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/lib/swift_storage_utils.py b/lib/swift_storage_utils.py index 2d57fe2..f9a34d8 100644 --- a/lib/swift_storage_utils.py +++ b/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) diff --git a/unit_tests/test_swift_storage_utils.py b/unit_tests/test_swift_storage_utils.py index 58d32a2..03a61d9 100644 --- a/unit_tests/test_swift_storage_utils.py +++ b/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")