iscsi: Add _get_device_link retry when waiting for /dev/disk/by-id/ to populate
Bug #1820007 documents failures to find /dev/disk/by-id/ symlinks associated with encrypted volumes both in real world and CI environments. These failures appear to be due to udev on these slow or overloaded hosts failing to populate the required /dev/disk/by-id/ symlinks in time after the iSCSI volume has been connected. This change seeks to avoid such failures by simply decorating _get_device_link with the @utils.retry to hopefully allow udev time to create the required symlinks under /dev/disk/by-id/. Closes-Bug: #1820007 Change-Id: Ib9c8ebae7a6051e18538920139fecd123682a474 (cherry picked from commit331316827a
) (cherry picked from commitd8ed21e7d2
) (cherry picked from commitd1f84dfd89
)
This commit is contained in:
parent
34a3ca23b3
commit
1f811c412a
|
@ -514,6 +514,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
|
||||||
with excutils.save_and_reraise_exception():
|
with excutils.save_and_reraise_exception():
|
||||||
self._cleanup_connection(connection_properties, force=True)
|
self._cleanup_connection(connection_properties, force=True)
|
||||||
|
|
||||||
|
@utils.retry(exceptions=(exception.VolumeDeviceNotFound))
|
||||||
def _get_device_link(self, wwn, device, mpath):
|
def _get_device_link(self, wwn, device, mpath):
|
||||||
# These are the default symlinks that should always be there
|
# These are the default symlinks that should always be there
|
||||||
if mpath:
|
if mpath:
|
||||||
|
|
|
@ -1555,17 +1555,67 @@ Setting up iSCSI targets: unused
|
||||||
mock.call('/dev/disk/by-id/dm-...'),
|
mock.call('/dev/disk/by-id/dm-...'),
|
||||||
mock.call('/dev/disk/by-id/scsi-wwn')])
|
mock.call('/dev/disk/by-id/scsi-wwn')])
|
||||||
|
|
||||||
|
@mock.patch('time.sleep')
|
||||||
@mock.patch('os.path.realpath', return_value='/dev/sdz')
|
@mock.patch('os.path.realpath', return_value='/dev/sdz')
|
||||||
@mock.patch('os.listdir', return_value=['dm-...', 'scsi-...'])
|
@mock.patch('os.listdir', return_value=['dm-...', 'scsi-...'])
|
||||||
def test__get_device_link_not_found(self, listdir_mock, realpath_mock):
|
def test__get_device_link_not_found(self, listdir_mock, realpath_mock,
|
||||||
|
mock_time):
|
||||||
self.assertRaises(exception.VolumeDeviceNotFound,
|
self.assertRaises(exception.VolumeDeviceNotFound,
|
||||||
self.connector._get_device_link,
|
self.connector._get_device_link,
|
||||||
'wwn', '/dev/sda', None)
|
'wwn', '/dev/sda', None)
|
||||||
listdir_mock.assert_called_once_with('/dev/disk/by-id/')
|
listdir_mock.assert_has_calls(3 * [mock.call('/dev/disk/by-id/')])
|
||||||
realpath_mock.assert_has_calls([
|
self.assertEqual(3, listdir_mock.call_count)
|
||||||
mock.call('/dev/disk/by-id/scsi-wwn'),
|
realpath_mock.assert_has_calls(
|
||||||
mock.call('/dev/disk/by-id/dm-...'),
|
3 * [mock.call('/dev/disk/by-id/scsi-wwn'),
|
||||||
mock.call('/dev/disk/by-id/scsi-...')])
|
mock.call('/dev/disk/by-id/dm-...'),
|
||||||
|
mock.call('/dev/disk/by-id/scsi-...')])
|
||||||
|
self.assertEqual(9, realpath_mock.call_count)
|
||||||
|
|
||||||
|
@mock.patch('time.sleep')
|
||||||
|
@mock.patch('os.path.realpath')
|
||||||
|
@mock.patch('os.listdir', return_value=['dm-...', 'scsi-...'])
|
||||||
|
def test__get_device_link_symlink_found_after_retry(self, mock_listdir,
|
||||||
|
mock_realpath,
|
||||||
|
mock_time):
|
||||||
|
# Return the expected realpath on the third retry
|
||||||
|
mock_realpath.side_effect = [
|
||||||
|
None, None, None, None, None, None, '/dev/sda']
|
||||||
|
|
||||||
|
# Assert that VolumeDeviceNotFound isn't raised
|
||||||
|
self.connector._get_device_link('wwn', '/dev/sda', None)
|
||||||
|
|
||||||
|
# Assert that listdir and realpath have been called correctly
|
||||||
|
mock_listdir.assert_has_calls(2 * [mock.call('/dev/disk/by-id/')])
|
||||||
|
self.assertEqual(2, mock_listdir.call_count)
|
||||||
|
mock_realpath.assert_has_calls(
|
||||||
|
2 * [mock.call('/dev/disk/by-id/scsi-wwn'),
|
||||||
|
mock.call('/dev/disk/by-id/dm-...'),
|
||||||
|
mock.call('/dev/disk/by-id/scsi-...')]
|
||||||
|
+ [mock.call('/dev/disk/by-id/scsi-wwn')])
|
||||||
|
self.assertEqual(7, mock_realpath.call_count)
|
||||||
|
|
||||||
|
@mock.patch('time.sleep')
|
||||||
|
@mock.patch('os.path.realpath')
|
||||||
|
@mock.patch('os.listdir', return_value=['dm-...', 'scsi-...'])
|
||||||
|
def test__get_device_link_symlink_found_after_retry_by_listdir(
|
||||||
|
self, mock_listdir, mock_realpath, mock_time):
|
||||||
|
|
||||||
|
# Return the expected realpath on the second retry while looping over
|
||||||
|
# the devices returned by listdir
|
||||||
|
mock_realpath.side_effect = [
|
||||||
|
None, None, None, None, None, '/dev/sda']
|
||||||
|
|
||||||
|
# Assert that VolumeDeviceNotFound isn't raised
|
||||||
|
self.connector._get_device_link('wwn', '/dev/sda', None)
|
||||||
|
|
||||||
|
# Assert that listdir and realpath have been called correctly
|
||||||
|
mock_listdir.assert_has_calls(2 * [mock.call('/dev/disk/by-id/')])
|
||||||
|
self.assertEqual(2, mock_listdir.call_count)
|
||||||
|
mock_realpath.assert_has_calls(
|
||||||
|
2 * [mock.call('/dev/disk/by-id/scsi-wwn'),
|
||||||
|
mock.call('/dev/disk/by-id/dm-...'),
|
||||||
|
mock.call('/dev/disk/by-id/scsi-...')])
|
||||||
|
self.assertEqual(6, mock_realpath.call_count)
|
||||||
|
|
||||||
@mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare')
|
@mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare')
|
||||||
def test_get_node_startup_values(self, run_iscsiadm_bare_mock):
|
def test_get_node_startup_values(self, run_iscsiadm_bare_mock):
|
||||||
|
|
Loading…
Reference in New Issue