Failover to alternative iSCSI portals on login failure

Some Cinder iSCSI backend drivers may return alternative
portals/tragets/luns information in the response from
initialize_connection API, for the case the main portal
is unreachable by network failure. This patch enables
brick to fail-over to alternative portals when it fails
to establish a session to the main portal.

Change-Id: Ic08d018d3b5f282ece823971576043f76bad1b2d
Implements: blueprint iscsi-alternative-portal

os_brick/initiator/connector.py

@@ -225,6 +225,11 @@ class ISCSIConnector(InitiatorConnector):
             props['target_lun'] = lun
             yield props
 
+    def _alternative_targets(self, connection_properties):
+        return zip(connection_properties.get('target_alternative_portals', []),
+                   connection_properties.get('target_alternative_iqns', []),
+                   connection_properties.get('target_alternative_luns', []))
+
     def _multipath_targets(self, connection_properties):
         return zip(connection_properties.get('target_portals', []),
                    connection_properties.get('target_iqns', []),
@@ -273,8 +278,19 @@ class ISCSIConnector(InitiatorConnector):
             self._rescan_iscsi()
             host_devices = self._get_device_path(connection_properties)
         else:
-            self._connect_to_iscsi_portal(connection_properties)
+            target_props = connection_properties
-            host_devices = self._get_device_path(connection_properties)
+            if not self._connect_to_iscsi_portal(target_props):
+                for props in self._iterate_multiple_targets(
+                        connection_properties,
+                        self._alternative_targets(connection_properties)):
+                    if self._connect_to_iscsi_portal(props):
+                        target_props = props
+                        break
+                else:
+                    LOG.warn(_LW(
+                        'Failed to login to any of the iSCSI targets.'))
+
+            host_devices = self._get_device_path(target_props)
 
         # The /dev/disk/by-path/... node is not always present immediately
         # TODO(justinsb): This retry-with-delay is a pattern, move to utils?
@@ -293,7 +309,7 @@ class ISCSIConnector(InitiatorConnector):
             if self.use_multipath:
                 self._rescan_iscsi()
             else:
-                self._run_iscsiadm(connection_properties, ("--rescan",))
+                self._run_iscsiadm(target_props, ("--rescan",))
 
             tries = tries + 1
             if all(map(lambda x: not os.path.exists(x), host_devices)):
@@ -360,9 +376,10 @@ class ISCSIConnector(InitiatorConnector):
         # unused devices created by logging into other LUNs' session.
         ips_iqns_luns = self._multipath_targets(connection_properties)
         if not ips_iqns_luns:
-            ips_iqns_luns = [[connection_properties['target_portal'],
+            ips_iqns_luns = ([[connection_properties['target_portal'],
                               connection_properties['target_iqn'],
-                              connection_properties.get('target_lun', 0)]]
+                              connection_properties.get('target_lun', 0)]] +
+                             self._alternative_targets(connection_properties))
         for props in self._iterate_multiple_targets(connection_properties,
                                                     ips_iqns_luns):
             self._disconnect_volume_iscsi(props)
@@ -536,17 +553,20 @@ class ISCSIConnector(InitiatorConnector):
                                    ("--login",),
                                    check_exit_code=[0, 255])
             except putils.ProcessExecutionError as err:
-                #as this might be one of many paths,
+                # exit_code=15 means the session already exists, so it should
-                #only set successful logins to startup automatically
+                # be regarded as successful login.
-                if err.exit_code in [15]:
+                if err.exit_code not in [15]:
-                    self._iscsiadm_update(connection_properties,
+                    LOG.warn(_LW('Failed to login iSCSI target %(iqn)s '
-                                          "node.startup",
+                                 'on portal %(portal)s (exit code %(err)s).'),
-                                          "automatic")
+                             {'iqn': connection_properties['target_iqn'],
-                    return
+                              'portal': connection_properties['target_portal'],
+                              'err': err.exit_code})
+                    return False
 
             self._iscsiadm_update(connection_properties,
                                   "node.startup",
                                   "automatic")
+        return True
 
     def _disconnect_from_iscsi_portal(self, connection_properties):
         self._iscsiadm_update(connection_properties, "node.startup", "manual",

os_brick/tests/initiator/test_connector.py

@@ -230,9 +230,7 @@ class ISCSIConnectorTestCase(ConnectorTestCase):
         initiator = self.connector.get_initiator()
         self.assertEqual(initiator, 'iqn.1234-56.foo.bar:01:23456789abc')
 
-    @testtools.skipUnless(os.path.exists('/dev/disk/by-path'),
+    def _test_connect_volume(self, extra_props, additional_commands):
-                          'Test requires /dev/disk/by-path')
-    def test_connect_volume(self):
         exists_mock = mock.Mock()
         exists_mock.return_value = True
         os.path.exists = exists_mock
@@ -241,6 +239,8 @@ class ISCSIConnectorTestCase(ConnectorTestCase):
         iqn = 'iqn.2010-10.org.openstack:%s' % name
         vol = {'id': 1, 'name': name}
         connection_info = self.iscsi_connection(vol, location, iqn)
+        for key, value in extra_props.iteritems():
+            connection_info['data'][key] = value
         device = self.connector.connect_volume(connection_info['data'])
         dev_str = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location, iqn)
         self.assertEqual(device['type'], 'block')
@@ -264,12 +264,83 @@ class ISCSIConnectorTestCase(ConnectorTestCase):
                              ('iscsiadm -m node -T %s -p %s --logout' %
                               (iqn, location)),
                              ('iscsiadm -m node -T %s -p %s --op delete' %
-                              (iqn, location)), ]
+                              (iqn, location)), ] + additional_commands
         LOG.debug("self.cmds = %s" % self.cmds)
         LOG.debug("expected = %s" % expected_commands)
 
         self.assertEqual(expected_commands, self.cmds)
 
+    @testtools.skipUnless(os.path.exists('/dev/disk/by-path'),
+                          'Test requires /dev/disk/by-path')
+    def test_connect_volume(self):
+        self._test_connect_volume({}, [])
+
+    @testtools.skipUnless(os.path.exists('/dev/disk/by-path'),
+                          'Test requires /dev/disk/by-path')
+    def test_connect_volume_with_alternative_targets(self):
+        location2 = '10.0.3.15:3260'
+        iqn2 = 'iqn.2010-10.org.openstack:volume-00000001-2'
+        extra_props = {'target_alternative_portals': [location2],
+                       'target_alternative_iqns': [iqn2],
+                       'target_alternative_luns': [2]}
+        additional_commands = [('blockdev --flushbufs /dev/sdb'),
+                               ('tee -a /sys/block/sdb/device/delete'),
+                               ('iscsiadm -m node -T %s -p %s --op update'
+                                ' -n node.startup -v manual' %
+                                (iqn2, location2)),
+                               ('iscsiadm -m node -T %s -p %s --logout' %
+                                (iqn2, location2)),
+                               ('iscsiadm -m node -T %s -p %s --op delete' %
+                                (iqn2, location2))]
+        self._test_connect_volume(extra_props, additional_commands)
+
+    @testtools.skipUnless(os.path.exists('/dev/disk/by-path'),
+                          'Test requires /dev/disk/by-path')
+    @mock.patch.object(os.path, 'exists')
+    @mock.patch.object(connector.ISCSIConnector, '_run_iscsiadm')
+    def test_connect_volume_with_alternative_targets_primary_error(
+            self, mock_iscsiadm, mock_exists):
+        location = '10.0.2.15:3260'
+        location2 = '10.0.3.15:3260'
+        name = 'volume-00000001'
+        iqn = 'iqn.2010-10.org.openstack:%s' % name
+        iqn2 = 'iqn.2010-10.org.openstack:%s-2' % name
+        vol = {'id': 1, 'name': name}
+        connection_info = self.iscsi_connection(vol, location, iqn)
+        connection_info['data']['target_alternative_portals'] = [location2]
+        connection_info['data']['target_alternative_iqns'] = [iqn2]
+        connection_info['data']['target_alternative_luns'] = [2]
+        dev_str2 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (location2, iqn2)
+
+        def fake_run_iscsiadm(iscsi_properties, iscsi_command, **kwargs):
+            if iscsi_properties['target_portal'] == location:
+                if iscsi_command == ('--login',):
+                    raise putils.ProcessExecutionError(None, None, 21)
+            return mock.DEFAULT
+
+        mock_iscsiadm.side_effect = fake_run_iscsiadm
+        mock_exists.side_effect = lambda x: x == dev_str2
+        device = self.connector.connect_volume(connection_info['data'])
+        self.assertEqual('block', device['type'])
+        self.assertEqual(dev_str2, device['path'])
+        props = connection_info['data'].copy()
+        props['target_portal'] = location2
+        props['target_iqn'] = iqn2
+        props['target_lun'] = 2
+        mock_iscsiadm.assert_any_call(props, ('--login',),
+                                      check_exit_code=[0, 255])
+
+        mock_iscsiadm.reset_mock()
+        self.connector.disconnect_volume(connection_info['data'], device)
+        props = connection_info['data'].copy()
+        mock_iscsiadm.assert_any_call(props, ('--logout',),
+                                      check_exit_code=[0, 21, 255])
+        props['target_portal'] = location2
+        props['target_iqn'] = iqn2
+        props['target_lun'] = 2
+        mock_iscsiadm.assert_any_call(props, ('--logout',),
+                                      check_exit_code=[0, 21, 255])
+
     def test_connect_volume_with_multipath(self):
         location = '10.0.2.15:3260'
         name = 'volume-00000001'