From 3266fb51a53156e4fa103c222e0226b23f785dd3 Mon Sep 17 00:00:00 2001 From: Rikimaru Honjo Date: Tue, 28 Nov 2017 15:16:41 +0900 Subject: [PATCH] Recover node.startup values after discovering os_brick updates node.startup values from default value to "automatic" when it creates iscsi connection. But existing target's node.startup values will be reverted from "automatic" to default value in creating iscsi connection process if multipath is used. When using multipath with a discovery type of backend, the "iscsiadm -m discovery -t sendtargets -p ..." command will recreate all target information of specified node.[1] node.startup value wil be reverted to default value of existing targets by recreating. As a result, "automatic" targets and default value targets will be mixed on the host. So this patch recovers node.startup values after discovering. [1] This behavior was explained in following page: https://github.com/open-iscsi/open-iscsi/issues/58 Change-Id: I30b736ae3b916f77fc0778f5364c5f6ed6fecc60 closes-bug: #1670237 --- os_brick/initiator/connectors/iscsi.py | 54 +++++++++++- .../tests/initiator/connectors/test_iscsi.py | 82 ++++++++++++++++++- 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/os_brick/initiator/connectors/iscsi.py b/os_brick/initiator/connectors/iscsi.py index 4202cbfac..7756fac57 100644 --- a/os_brick/initiator/connectors/iscsi.py +++ b/os_brick/initiator/connectors/iscsi.py @@ -14,6 +14,7 @@ import collections +import copy import glob import os import re @@ -412,6 +413,8 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): {'target_portal': connection_properties[ 'target_portal']}) raise + old_node_startups = self._get_node_startup_values( + connection_properties) out = self._run_iscsiadm_bare( ['-m', 'discoverydb', '-t', 'sendtargets', @@ -419,13 +422,19 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): '-p', connection_properties['target_portal'], '--discover'], check_exit_code=[0, 255])[0] or "" + self._recover_node_startup_values(connection_properties, + old_node_startups) else: + old_node_startups = self._get_node_startup_values( + connection_properties) out = self._run_iscsiadm_bare( ['-m', 'discovery', '-t', 'sendtargets', '-I', iscsi_transport, '-p', connection_properties['target_portal']], check_exit_code=[0, 255])[0] or "" + self._recover_node_startup_values(connection_properties, + old_node_startups) ips, iqns = self._get_target_portals_from_iscsiadm_output(out) luns = self._get_luns(connection_properties, iqns) @@ -1037,7 +1046,6 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): {'iqn': target_iqn, 'portal': portal, 'err': err.exit_code}) return None, None - self._iscsiadm_update(connection_properties, "node.startup", "automatic") @@ -1092,3 +1100,47 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector): {'multipath_command': multipath_command, 'out': out, 'err': err}) return (out, err) + + def _get_node_startup_values(self, connection_properties): + out, __ = self._run_iscsiadm_bare( + ['-m', 'node', '--op', 'show', '-p', + connection_properties['target_portal']]) or "" + node_values = out.strip() + node_values = node_values.split("\n") + iqn = None + startup = None + startup_values = {} + + for node_value in node_values: + node_keys = node_value.split() + try: + if node_keys[0] == "node.name": + iqn = node_keys[2] + elif node_keys[0] == "node.startup": + startup = node_keys[2] + + if iqn and startup: + startup_values[iqn] = startup + iqn = None + startup = None + except IndexError: + pass + + return startup_values + + def _recover_node_startup_values(self, connection_properties, + old_node_startups): + node_startups = self._get_node_startup_values(connection_properties) + for iqn, node_startup in node_startups.items(): + old_node_startup = old_node_startups.get(iqn, None) + if old_node_startup and node_startup != old_node_startup: + # _iscsiadm_update() only uses "target_portal" and "target_iqn" + # of connection_properties. + # And the recovering target belongs to the same target_portal + # as discovering target. + # So target_iqn is updated, and other values aren't updated. + recover_connection = copy.deepcopy(connection_properties) + recover_connection['target_iqn'] = [iqn] + self._iscsiadm_update(recover_connection, + "node.startup", + old_node_startup) diff --git a/os_brick/tests/initiator/connectors/test_iscsi.py b/os_brick/tests/initiator/connectors/test_iscsi.py index 7df37e338..17e66505f 100644 --- a/os_brick/tests/initiator/connectors/test_iscsi.py +++ b/os_brick/tests/initiator/connectors/test_iscsi.py @@ -459,9 +459,11 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): 'auth_method': discovery_auth_method, 'username': discovery_auth_username, 'password': discovery_auth_password}, + 'iscsiadm -m node --op show -p %s' % location, 'iscsiadm -m discoverydb -t sendtargets -I %(iface)s' ' -p %(location)s --discover' % {'iface': interface, - 'location': location}] + 'location': location}, + 'iscsiadm -m node --op show -p %s' % location] self.assertEqual(expected_cmds, self.cmds) # Reset to run with a different transport type self.cmds = list() @@ -504,8 +506,10 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase): expected_cmds = [ 'iscsiadm -m discoverydb -t sendtargets -p %s -I default' ' --op new' % location, + 'iscsiadm -m node --op show -p %s' % location, 'iscsiadm -m discoverydb -t sendtargets -I default -p %s' - ' --discover' % location] + ' --discover' % location, + 'iscsiadm -m node --op show -p %s' % location] self.assertEqual(expected_cmds, self.cmds) self.assertRaises(exception.TargetPortalNotFound, @@ -1447,3 +1451,77 @@ Setting up iSCSI targets: unused mock.call('/dev/disk/by-id/scsi-wwn'), mock.call('/dev/disk/by-id/dm-...'), mock.call('/dev/disk/by-id/scsi-...')]) + + @mock.patch.object(iscsi.ISCSIConnector, '_run_iscsiadm_bare') + def test_get_node_startup_values(self, run_iscsiadm_bare_mock): + name1 = 'volume-00000001-1' + name2 = 'volume-00000001-2' + name3 = 'volume-00000001-3' + vol = {'id': 1, 'name': name1} + location = '10.0.2.15:3260' + iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 + iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 + iqn3 = 'iqn.2010-10.org.openstack:%s' % name3 + connection_properties = self.iscsi_connection(vol, [location], [iqn1]) + + node_startup1 = "manual" + node_startup2 = "automatic" + node_startup3 = "manual" + node_values = ( + '# BEGIN RECORD 2.0-873\n' + 'node.name = %s\n' + 'node.tpgt = 1\n' + 'node.startup = %s\n' + 'iface.hwaddress = \n' + '# END RECORD\n' + '# BEGIN RECORD 2.0-873\n' + 'node.name = %s\n' + 'node.tpgt = 1\n' + 'node.startup = %s\n' + 'iface.hwaddress = \n' + '# END RECORD\n' + '# BEGIN RECORD 2.0-873\n' + 'node.name = %s\n' + 'node.tpgt = 1\n' + 'node.startup = %s\n' + 'iface.hwaddress = \n' + '# END RECORD\n') % (iqn1, node_startup1, iqn2, node_startup2, + iqn3, node_startup3) + run_iscsiadm_bare_mock.return_value = (node_values, None) + + node_startups =\ + self.connector._get_node_startup_values( + connection_properties['data']) + expected_node_startups = {iqn1: node_startup1, iqn2: node_startup2, + iqn3: node_startup3} + self.assertEqual(node_startups, expected_node_startups) + + @mock.patch.object(iscsi.ISCSIConnector, '_get_node_startup_values') + @mock.patch.object(iscsi.ISCSIConnector, '_iscsiadm_update') + def test_recover_node_startup_values(self, iscsiadm_update_mock, + get_node_startup_values_mock): + name1 = 'volume-00000001-1' + name2 = 'volume-00000001-2' + name3 = 'volume-00000001-3' + vol = {'id': 1, 'name': name1} + location = '10.0.2.15:3260' + iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 + iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 + iqn3 = 'iqn.2010-10.org.openstack:%s' % name3 + connection_properties = self.iscsi_connection(vol, [location], [iqn1]) + recover_connection = self.iscsi_connection(vol, [location], [iqn2]) + + node_startup1 = "manual" + node_startup2 = "automatic" + node_startup3 = "manual" + get_node_startup_values_mock.return_value = {iqn1: node_startup1, + iqn2: node_startup2, + iqn3: node_startup3} + + old_node_startup_values = {iqn1: node_startup1, + iqn2: "manual", + iqn3: node_startup3} + self.connector._recover_node_startup_values( + connection_properties['data'], old_node_startup_values) + iscsiadm_update_mock.assert_called_once_with( + recover_connection['data'], "node.startup", "manual")