From b2ddad27522a79e7d18e5a6c74776c82faf12fc6 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 15 May 2017 15:42:46 +0300 Subject: [PATCH] Windows iSCSI: use configured iSCSI portals At the moment, the Windows iSCSI driver ignores the configured iSCSI ip addresses, picking up one of the available ones. This change ensures that we only use the configured addresses when reporting iSCSI portals, also ensuring that those are in fact available. As a side effect, this change enables multipath support. Closes-Bug: #1369471 Change-Id: Ibfa8de9e3f27f4c7485abb9c4764b6daa5d5d6a8 --- cinder/tests/unit/windows/test_windows.py | 74 +++++++++++++++---- cinder/volume/drivers/windows/windows.py | 46 ++++++++++-- ...iscsi-config-portals-51895294228d7883.yaml | 9 +++ 3 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/win-iscsi-config-portals-51895294228d7883.yaml diff --git a/cinder/tests/unit/windows/test_windows.py b/cinder/tests/unit/windows/test_windows.py index 8c396e38f75..4ba4916e281 100644 --- a/cinder/tests/unit/windows/test_windows.py +++ b/cinder/tests/unit/windows/test_windows.py @@ -18,13 +18,15 @@ Unit tests for Windows Server 2012 OpenStack Cinder volume driver """ -import mock import os +import ddt +import mock from oslo_utils import fileutils from oslo_utils import units from cinder import context +from cinder import exception from cinder.image import image_utils from cinder import test from cinder.tests.unit import fake_snapshot @@ -34,16 +36,17 @@ from cinder.volume import configuration as conf from cinder.volume.drivers.windows import windows +@ddt.ddt class TestWindowsDriver(test.TestCase): @mock.patch.object(windows, 'utilsfactory') def setUp(self, mock_utilsfactory): super(TestWindowsDriver, self).setUp() - configuration = conf.Configuration(None) - configuration.append_config_values(windows.windows_opts) + self.configuration = conf.Configuration(None) + self.configuration.append_config_values(windows.windows_opts) self.flags(windows_iscsi_lun_path='fake_iscsi_lun_path') self.flags(image_conversion_dir='fake_image_conversion_dir') - self._driver = windows.WindowsDriver(configuration=configuration) + self._driver = windows.WindowsDriver(configuration=self.configuration) @mock.patch.object(fileutils, 'ensure_tree') def test_do_setup(self, mock_ensure_tree): @@ -53,30 +56,66 @@ class TestWindowsDriver(test.TestCase): [mock.call('fake_iscsi_lun_path'), mock.call('fake_image_conversion_dir')]) - def test_check_for_setup_error(self): + @mock.patch.object(windows.WindowsDriver, '_get_portals') + def test_check_for_setup_error(self, mock_get_portals): self._driver.check_for_setup_error() - self._driver._tgt_utils.get_portal_locations.assert_called_once_with( - available_only=True, fail_if_none_found=True) + mock_get_portals.assert_called_once_with() + @ddt.data(True, False) + def test_get_portals(self, portals_available=True): + iscsi_port = mock.sentinel.iscsi_port + available_ips = ['fake_ip0', 'fake_ip1', 'fake_unrequested_ip'] + requested_ips = available_ips[:-1] + ['fake_inexistent_ips'] + + available_portals = ([":".join([ip_addr, str(iscsi_port)]) + for ip_addr in available_ips] + if portals_available else []) + + self._driver.configuration = mock.Mock() + self._driver.configuration.iscsi_port = iscsi_port + self._driver.configuration.iscsi_ip_address = requested_ips[0] + self._driver.configuration.iscsi_secondary_ip_addresses = ( + requested_ips[1:]) + + self._driver._tgt_utils.get_portal_locations.return_value = ( + available_portals) + + if portals_available: + portals = self._driver._get_portals() + self.assertEqual(set(available_portals[:-1]), set(portals)) + else: + self.assertRaises(exception.VolumeDriverException, + self._driver._get_portals) + + self._driver._tgt_utils.get_portal_locations.assert_called_once_with( + available_only=True, + fail_if_none_found=True) + + @ddt.data(True, False) + @mock.patch.object(windows.WindowsDriver, '_get_portals') @mock.patch.object(windows.WindowsDriver, '_get_target_name') - def test_get_host_information(self, mock_get_target_name): + def test_get_host_information(self, multipath, mock_get_target_name, + mock_get_portals): tgt_utils = self._driver._tgt_utils fake_auth_meth = 'CHAP' fake_chap_username = 'fake_chap_username' fake_chap_password = 'fake_chap_password' - fake_host_info = {'fake_prop': 'fake_value'} + fake_target_iqn = 'fake_target_iqn' + fake_host_info = {'target_iqn': 'fake_target_iqn', + 'fake_prop': 'fake_value'} fake_provider_auth = "%s %s %s" % (fake_auth_meth, fake_chap_username, fake_chap_password) + fake_portals = [mock.sentinel.portal_location0, + mock.sentinel.portal_location1] volume = fake_volume.fake_volume_obj(mock.sentinel.context, provider_auth=fake_provider_auth) mock_get_target_name.return_value = mock.sentinel.target_name - tgt_utils.get_portal_locations.return_value = [ - mock.sentinel.portal_location] + mock_get_portals.return_value = fake_portals tgt_utils.get_target_information.return_value = fake_host_info expected_host_info = dict(fake_host_info, @@ -84,16 +123,20 @@ class TestWindowsDriver(test.TestCase): auth_username=fake_chap_username, auth_password=fake_chap_password, target_discovered=False, - target_portal=mock.sentinel.portal_location, + target_portal=fake_portals[0], target_lun=0, volume_id=volume.id) + if multipath: + expected_host_info['target_portals'] = fake_portals + expected_host_info['target_iqns'] = [fake_target_iqn] * 2 + expected_host_info['target_luns'] = [0] * 2 - host_info = self._driver._get_host_information(volume) + host_info = self._driver._get_host_information(volume, multipath) self.assertEqual(expected_host_info, host_info) mock_get_target_name.assert_called_once_with(volume) - tgt_utils.get_portal_locations.assert_called_once_with() + mock_get_portals.assert_called_once_with() tgt_utils.get_target_information.assert_called_once_with( mock.sentinel.target_name) @@ -103,6 +146,7 @@ class TestWindowsDriver(test.TestCase): volume = fake_volume.fake_volume_obj(mock.sentinel.fake_context) fake_initiator = db_fakes.get_fake_connector_info() + fake_initiator['multipath'] = mock.sentinel.multipath fake_host_info = {'fake_host_prop': 'fake_value'} mock_get_host_info.return_value = fake_host_info @@ -113,6 +157,8 @@ class TestWindowsDriver(test.TestCase): fake_initiator) self.assertEqual(expected_conn_info, conn_info) + mock_get_host_info.assert_called_once_with( + volume, mock.sentinel.multipath) mock_associate = tgt_utils.associate_initiator_with_iscsi_target mock_associate.assert_called_once_with( fake_initiator['initiator'], diff --git a/cinder/volume/drivers/windows/windows.py b/cinder/volume/drivers/windows/windows.py index 0a1abd006b3..2638e709656 100644 --- a/cinder/volume/drivers/windows/windows.py +++ b/cinder/volume/drivers/windows/windows.py @@ -29,6 +29,7 @@ from oslo_utils import fileutils from oslo_utils import units from oslo_utils import uuidutils +from cinder import exception from cinder.image import image_utils from cinder.volume import driver from cinder.volume import utils @@ -74,16 +75,37 @@ class WindowsDriver(driver.ISCSIDriver): def check_for_setup_error(self): """Check that the driver is working and can communicate.""" - self._tgt_utils.get_portal_locations(available_only=True, - fail_if_none_found=True) + self._get_portals() - def _get_host_information(self, volume): + def _get_portals(self): + available_portals = set(self._tgt_utils.get_portal_locations( + available_only=True, + fail_if_none_found=True)) + LOG.debug("Available iSCSI portals: %s", available_portals) + + iscsi_port = self.configuration.iscsi_port + iscsi_ips = ([self.configuration.iscsi_ip_address] + + self.configuration.iscsi_secondary_ip_addresses) + requested_portals = {':'.join([iscsi_ip, str(iscsi_port)]) + for iscsi_ip in iscsi_ips} + + unavailable_portals = requested_portals - available_portals + if unavailable_portals: + LOG.warning("The following iSCSI portals were requested but " + "are not available: %s.", unavailable_portals) + + selected_portals = requested_portals & available_portals + if not selected_portals: + err_msg = "None of the configured iSCSI portals are available." + raise exception.VolumeDriverException(err_msg) + + return list(selected_portals) + + def _get_host_information(self, volume, multipath=False): """Getting the portal and port information.""" - # TODO(lpetrut): properly handle multiple existing portals, also - # use the iSCSI traffic addresses config options. target_name = self._get_target_name(volume) - available_portal_location = self._tgt_utils.get_portal_locations()[0] + available_portals = self._get_portals() properties = self._tgt_utils.get_target_information(target_name) # Note(lpetrut): the WT_Host CHAPSecret field cannot be accessed @@ -95,11 +117,18 @@ class WindowsDriver(driver.ISCSIDriver): properties['auth_username'] = auth_username properties['auth_password'] = auth_secret + properties['target_portal'] = available_portals[0] properties['target_discovered'] = False - properties['target_portal'] = available_portal_location properties['target_lun'] = 0 properties['volume_id'] = volume.id + if multipath: + properties['target_portals'] = available_portals + properties['target_iqns'] = [properties['target_iqn'] + for portal in available_portals] + properties['target_luns'] = [properties['target_lun'] + for portal in available_portals] + return properties def initialize_connection(self, volume, connector): @@ -110,7 +139,8 @@ class WindowsDriver(driver.ISCSIDriver): self._tgt_utils.associate_initiator_with_iscsi_target(initiator_name, target_name) - properties = self._get_host_information(volume) + properties = self._get_host_information(volume, + connector.get('multipath')) return { 'driver_volume_type': 'iscsi', diff --git a/releasenotes/notes/win-iscsi-config-portals-51895294228d7883.yaml b/releasenotes/notes/win-iscsi-config-portals-51895294228d7883.yaml new file mode 100644 index 00000000000..1e13cd5efdc --- /dev/null +++ b/releasenotes/notes/win-iscsi-config-portals-51895294228d7883.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + The Windows iSCSI driver now returns multiple portals when available + and multipath is requested. +fixes: + - | + The Windows iSCSI driver now honors the configured iSCSI addresses, + ensuring that only those addresses will be used for iSCSI traffic.