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.