summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGoutham Pacha Ravi <gouthampravi@gmail.com>2018-08-03 14:46:08 -0700
committerTom Barron <tpb@dyncloud.net>2018-09-25 11:55:47 +0000
commitefbc62d9b29e94dd45339fa78da93a6a85532271 (patch)
treefe017d07a994bf4a5a3c101ce63a9df88a8bf3ed
parenta61a245d43ceb42e8192292971788014a66f7542 (diff)
[ZFSOnLinux] Retry unmounting old datasets during manage5.0.2
Add a retry loop to ensure the dataset being renamed is cleanly unmounted before the rename operation. Change-Id: Ie506f237010c415ee9f0d64abbefd5854f776a5f Closes-Bug: #1785180 (cherry picked from commit d7c01efb444b7bf02c1729be3554abebaee473a3) (cherry picked from commit 6d8f47d3c367aa0247dca713146825cfa66b6848) (cherry picked from commit 820638ccef471eb00f2a233e64b05a9c377e170c)
Notes
Notes (review): Code-Review+2: Tom Barron <tpb@dyncloud.net> Code-Review+2: Goutham Pacha Ravi <gouthampravi@gmail.com> Workflow+1: Goutham Pacha Ravi <gouthampravi@gmail.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Thu, 27 Sep 2018 00:19:56 +0000 Reviewed-on: https://review.openstack.org/605050 Project: openstack/manila Branch: refs/heads/stable/pike
-rw-r--r--manila/share/drivers/zfsonlinux/driver.py54
-rw-r--r--manila/tests/share/drivers/zfsonlinux/test_driver.py78
-rw-r--r--releasenotes/notes/bug-1785180-zfsonlinux-retry-unmounting-during-manage-872cf46313c5a4ff.yaml6
3 files changed, 116 insertions, 22 deletions
diff --git a/manila/share/drivers/zfsonlinux/driver.py b/manila/share/drivers/zfsonlinux/driver.py
index 8a9141c..06d9f18 100644
--- a/manila/share/drivers/zfsonlinux/driver.py
+++ b/manila/share/drivers/zfsonlinux/driver.py
@@ -724,15 +724,6 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
724 "username": self.configuration.zfs_ssh_username, 724 "username": self.configuration.zfs_ssh_username,
725 "host": self.service_ip, 725 "host": self.service_ip,
726 } 726 }
727 self.private_storage.update(
728 share["id"], {
729 "entity_type": "share",
730 "dataset_name": new_dataset_name,
731 "ssh_cmd": ssh_cmd, # used in replication
732 "pool_name": actual_pool_name, # used in replication
733 "used_options": " ".join(options),
734 }
735 )
736 727
737 # Perform checks on requested dataset 728 # Perform checks on requested dataset
738 if actual_pool_name != scheduled_pool_name: 729 if actual_pool_name != scheduled_pool_name:
@@ -760,13 +751,25 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
760 "dataset": old_dataset_name, 751 "dataset": old_dataset_name,
761 "zpool": actual_pool_name}) 752 "zpool": actual_pool_name})
762 753
763 # Rename dataset 754 # Unmount the dataset before attempting to rename and mount
764 out, err = self.execute("sudo", "mount") 755 try:
765 if "%s " % old_dataset_name in out: 756 self._unmount_share_with_retry(old_dataset_name)
766 self.zfs_with_retry("umount", "-f", old_dataset_name) 757 except exception.ZFSonLinuxException:
767 time.sleep(1) 758 msg = _("Unable to unmount share before renaming and re-mounting.")
759 raise exception.ZFSonLinuxException(message=msg)
760
761 # Rename the dataset and mount with new name
768 self.zfs_with_retry("rename", old_dataset_name, new_dataset_name) 762 self.zfs_with_retry("rename", old_dataset_name, new_dataset_name)
769 self.zfs("mount", new_dataset_name) 763
764 try:
765 self.zfs("mount", new_dataset_name)
766 except exception.ProcessExecutionError:
767 # Workaround for bug/1785180
768 out, err = self.zfs("mount")
769 mounted = any([new_dataset_name in mountedfs
770 for mountedfs in out.splitlines()])
771 if not mounted:
772 raise
770 773
771 # Apply options to dataset 774 # Apply options to dataset
772 for option in options: 775 for option in options:
@@ -776,6 +779,16 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
776 export_locations = self._get_share_helper( 779 export_locations = self._get_share_helper(
777 share["share_proto"]).get_exports(new_dataset_name) 780 share["share_proto"]).get_exports(new_dataset_name)
778 781
782 self.private_storage.update(
783 share["id"], {
784 "entity_type": "share",
785 "dataset_name": new_dataset_name,
786 "ssh_cmd": ssh_cmd, # used in replication
787 "pool_name": actual_pool_name, # used in replication
788 "used_options": " ".join(options),
789 }
790 )
791
779 return {"size": share["size"], "export_locations": export_locations} 792 return {"size": share["size"], "export_locations": export_locations}
780 793
781 def unmanage(self, share): 794 def unmanage(self, share):
@@ -836,6 +849,17 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver):
836 """Unmanage dataset snapshot.""" 849 """Unmanage dataset snapshot."""
837 self.private_storage.delete(snapshot_instance["snapshot_id"]) 850 self.private_storage.delete(snapshot_instance["snapshot_id"])
838 851
852 @utils.retry(exception.ZFSonLinuxException)
853 def _unmount_share_with_retry(self, share_name):
854 out, err = self.execute("sudo", "mount")
855 if "%s " % share_name not in out:
856 return
857 self.zfs_with_retry("umount", "-f", share_name)
858 out, err = self.execute("sudo", "mount")
859 if "%s " % share_name in out:
860 raise exception.ZFSonLinuxException(
861 _("Unable to unmount dataset %s"), share_name)
862
839 def _get_replication_snapshot_prefix(self, replica): 863 def _get_replication_snapshot_prefix(self, replica):
840 """Returns replica-based snapshot prefix.""" 864 """Returns replica-based snapshot prefix."""
841 replication_snapshot_prefix = "%s_%s" % ( 865 replication_snapshot_prefix = "%s_%s" % (
diff --git a/manila/tests/share/drivers/zfsonlinux/test_driver.py b/manila/tests/share/drivers/zfsonlinux/test_driver.py
index a91c970..36ee0a5 100644
--- a/manila/tests/share/drivers/zfsonlinux/test_driver.py
+++ b/manila/tests/share/drivers/zfsonlinux/test_driver.py
@@ -1138,7 +1138,7 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1138 zfs_driver.share_types, 1138 zfs_driver.share_types,
1139 'get_extra_specs_from_share', 1139 'get_extra_specs_from_share',
1140 mock.Mock(return_value={})) 1140 mock.Mock(return_value={}))
1141 mock_sleep = self.mock_object(zfs_driver.time, "sleep") 1141 self.mock_object(zfs_driver.time, "sleep")
1142 mock__get_dataset_name = self.mock_object( 1142 mock__get_dataset_name = self.mock_object(
1143 self.driver, "_get_dataset_name", 1143 self.driver, "_get_dataset_name",
1144 mock.Mock(return_value=new_dataset_name)) 1144 mock.Mock(return_value=new_dataset_name))
@@ -1148,11 +1148,17 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1148 mock.Mock(return_value=("fake_out", "fake_error"))) 1148 mock.Mock(return_value=("fake_out", "fake_error")))
1149 mock_zfs_with_retry = self.mock_object(self.driver, "zfs_with_retry") 1149 mock_zfs_with_retry = self.mock_object(self.driver, "zfs_with_retry")
1150 1150
1151 mock_execute = self.mock_object(self.driver, "execute") 1151 mock_execute_side_effects = [
1152 ("%s " % old_dataset_name, "fake_err")
1153 if mount_exists else ("foo", "bar")
1154 ] * 3
1152 if mount_exists: 1155 if mount_exists:
1153 mock_execute.return_value = "%s " % old_dataset_name, "fake_err" 1156 # After three retries, assume the mount goes away
1154 else: 1157 mock_execute_side_effects.append((("foo", "bar")))
1155 mock_execute.return_value = ("foo", "bar") 1158 mock_execute = self.mock_object(
1159 self.driver, "execute",
1160 mock.Mock(side_effect=iter(mock_execute_side_effects)))
1161
1156 mock_parse_zfs_answer = self.mock_object( 1162 mock_parse_zfs_answer = self.mock_object(
1157 self.driver, 1163 self.driver,
1158 "parse_zfs_answer", 1164 "parse_zfs_answer",
@@ -1175,9 +1181,11 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1175 self.assertEqual( 1181 self.assertEqual(
1176 mock_helper.return_value.get_exports.return_value, 1182 mock_helper.return_value.get_exports.return_value,
1177 result["export_locations"]) 1183 result["export_locations"])
1184 mock_execute.assert_called_with("sudo", "mount")
1178 if mount_exists: 1185 if mount_exists:
1179 mock_sleep.assert_called_once_with(1) 1186 self.assertEqual(4, mock_execute.call_count)
1180 mock_execute.assert_called_once_with("sudo", "mount") 1187 else:
1188 self.assertEqual(1, mock_execute.call_count)
1181 mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0]) 1189 mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0])
1182 if driver_options.get("size"): 1190 if driver_options.get("size"):
1183 self.assertFalse(mock_get_zfs_option.called) 1191 self.assertFalse(mock_get_zfs_option.called)
@@ -1259,6 +1267,62 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase):
1259 mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0]) 1267 mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0])
1260 mock_get_extra_specs_from_share.assert_called_once_with(share) 1268 mock_get_extra_specs_from_share.assert_called_once_with(share)
1261 1269
1270 def test_manage_unmount_exception(self):
1271 old_ds_name = "foopool/path/to/old/dataset/name"
1272 new_ds_name = "foopool/path/to/new/dataset/name"
1273 share = {
1274 "id": "fake_share_instance_id",
1275 "share_id": "fake_share_id",
1276 "export_locations": [{"path": "1.1.1.1:/%s" % old_ds_name}],
1277 "host": "foobackend@foohost#foopool",
1278 "share_proto": "NFS",
1279 }
1280
1281 mock_get_extra_specs_from_share = self.mock_object(
1282 zfs_driver.share_types,
1283 'get_extra_specs_from_share',
1284 mock.Mock(return_value={}))
1285 self.mock_object(zfs_driver.time, "sleep")
1286 mock__get_dataset_name = self.mock_object(
1287 self.driver, "_get_dataset_name",
1288 mock.Mock(return_value=new_ds_name))
1289 mock_helper = self.mock_object(self.driver, "_get_share_helper")
1290 mock_zfs = self.mock_object(
1291 self.driver, "zfs",
1292 mock.Mock(return_value=("fake_out", "fake_error")))
1293 mock_zfs_with_retry = self.mock_object(self.driver, "zfs_with_retry")
1294
1295 # 10 Retries, would mean 20 calls to check the mount still exists
1296 mock_execute_side_effects = [("%s " % old_ds_name, "fake_err")] * 21
1297 mock_execute = self.mock_object(
1298 self.driver, "execute",
1299 mock.Mock(side_effect=mock_execute_side_effects))
1300
1301 mock_parse_zfs_answer = self.mock_object(
1302 self.driver,
1303 "parse_zfs_answer",
1304 mock.Mock(return_value=[
1305 {"NAME": "some_other_dataset_1"},
1306 {"NAME": old_ds_name},
1307 {"NAME": "some_other_dataset_2"},
1308 ]))
1309 mock_get_zfs_option = self.mock_object(
1310 self.driver, 'get_zfs_option', mock.Mock(return_value="4G"))
1311
1312 self.assertRaises(exception.ZFSonLinuxException,
1313 self.driver.manage_existing,
1314 share, {'size': 10})
1315
1316 self.assertFalse(mock_helper.return_value.get_exports.called)
1317 mock_zfs_with_retry.assert_called_with("umount", "-f", old_ds_name)
1318 mock_execute.assert_called_with("sudo", "mount")
1319 self.assertEqual(10, mock_zfs_with_retry.call_count)
1320 self.assertEqual(20, mock_execute.call_count)
1321 mock_parse_zfs_answer.assert_called_once_with(mock_zfs.return_value[0])
1322 self.assertFalse(mock_get_zfs_option.called)
1323 mock__get_dataset_name.assert_called_once_with(share)
1324 mock_get_extra_specs_from_share.assert_called_once_with(share)
1325
1262 def test_unmanage(self): 1326 def test_unmanage(self):
1263 share = {'id': 'fake_share_id'} 1327 share = {'id': 'fake_share_id'}
1264 self.mock_object(self.driver.private_storage, 'delete') 1328 self.mock_object(self.driver.private_storage, 'delete')
diff --git a/releasenotes/notes/bug-1785180-zfsonlinux-retry-unmounting-during-manage-872cf46313c5a4ff.yaml b/releasenotes/notes/bug-1785180-zfsonlinux-retry-unmounting-during-manage-872cf46313c5a4ff.yaml
new file mode 100644
index 0000000..bcaf21d
--- /dev/null
+++ b/releasenotes/notes/bug-1785180-zfsonlinux-retry-unmounting-during-manage-872cf46313c5a4ff.yaml
@@ -0,0 +1,6 @@
1---
2fixes:
3 - |
4 The ZFSOnLinux driver now retries unmounting zfs shares
5 to perform the manage operation. See `Launchpad bug 1785180
6 <https://bugs.launchpad.net/manila/+bug/1785180>`_ for details.