From 235641fc4c0bb2864bc9b97553865dac3c4b6b1d Mon Sep 17 00:00:00 2001
From: Chris Krelle <nobodycam@gmail.com>
Date: Wed, 4 Feb 2015 09:33:38 -0800
Subject: [PATCH] improve iSCSI connection check

This patch improves the iSCSI connection verification by polling
/dev/disk/by-path for the iSCSI connection. This should ensure that
the iSCSI block device is seen by the file system after login.

It also removes the hard coded sleep in the login method.

Change-Id: Ibf6b6a15471a6130f33788b8c3f26d4577bf79a3
Closes-bug: #1417307
---
 ironic/drivers/modules/deploy_utils.py    | 29 +++++++++++++++++++++--
 ironic/tests/drivers/test_deploy_utils.py | 27 +++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 712aa5fbd3..3eeb9bcaa5 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -86,12 +86,37 @@ def login_iscsi(portal_address, portal_port, target_iqn):
                   check_exit_code=[0],
                   attempts=5,
                   delay_on_retry=True)
-    # NOTE(dprince): partial revert of 4606716 until we debug further
-    time.sleep(3)
     # Ensure the login complete
     verify_iscsi_connection(target_iqn)
     # force iSCSI initiator to re-read luns
     force_iscsi_lun_update(target_iqn)
+    # ensure file system sees the block device
+    check_file_system_for_iscsi_device(portal_address,
+                                       portal_port,
+                                       target_iqn)
+
+
+def check_file_system_for_iscsi_device(portal_address,
+                                       portal_port,
+                                       target_iqn):
+    """Ensure the file system sees the iSCSI block device."""
+    check_dir = "/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (portal_address,
+                                                               portal_port,
+                                                               target_iqn)
+    total_checks = CONF.deploy.iscsi_verify_attempts
+    for attempt in range(total_checks):
+        if os.path.exists(check_dir):
+            break
+        time.sleep(1)
+        LOG.debug("iSCSI connection not seen by file system. Rechecking. "
+                  "Attempt %(attempt)d out of %(total)d",
+                  {"attempt": attempt + 1,
+                   "total": total_checks})
+    else:
+        msg = _("iSCSI connection was not seen by the file system after "
+                "attempting to verify %d times.") % total_checks
+        LOG.error(msg)
+        raise exception.InstanceDeployFailure(msg)
 
 
 def verify_iscsi_connection(target_iqn):
diff --git a/ironic/tests/drivers/test_deploy_utils.py b/ironic/tests/drivers/test_deploy_utils.py
index d619cdcc06..1ec6b5db95 100644
--- a/ironic/tests/drivers/test_deploy_utils.py
+++ b/ironic/tests/drivers/test_deploy_utils.py
@@ -450,6 +450,29 @@ class PhysicalWorkTestCase(tests_base.TestCase):
                 utils.verify_iscsi_connection, iqn)
         self.assertEqual(3, mock_exec.call_count)
 
+    @mock.patch.object(os.path, 'exists')
+    def test_check_file_system_for_iscsi_device_raises(self, mock_os):
+        iqn = 'iqn.xyz'
+        ip = "127.0.0.1"
+        port = "22"
+        mock_os.return_value = False
+        self.assertRaises(exception.InstanceDeployFailure,
+                utils.check_file_system_for_iscsi_device, ip, port, iqn)
+        self.assertEqual(3, mock_os.call_count)
+
+    @mock.patch.object(os.path, 'exists')
+    def test_check_file_system_for_iscsi_device(self, mock_os):
+        iqn = 'iqn.xyz'
+        ip = "127.0.0.1"
+        port = "22"
+        check_dir = "/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (ip,
+                                                                   port,
+                                                                   iqn)
+
+        mock_os.return_value = True
+        utils.check_file_system_for_iscsi_device(ip, port, iqn)
+        mock_os.assert_called_once_with(check_dir)
+
     @mock.patch.object(common_utils, 'execute')
     def test_verify_iscsi_connection(self, mock_exec):
         iqn = 'iqn.xyz'
@@ -475,7 +498,9 @@ class PhysicalWorkTestCase(tests_base.TestCase):
     @mock.patch.object(common_utils, 'execute')
     @mock.patch.object(utils, 'verify_iscsi_connection')
     @mock.patch.object(utils, 'force_iscsi_lun_update')
+    @mock.patch.object(utils, 'check_file_system_for_iscsi_device')
     def test_login_iscsi_calls_verify_and_update(self,
+                                                 mock_check_dev,
                                                  mock_update,
                                                  mock_verify,
                                                  mock_exec):
@@ -498,6 +523,8 @@ class PhysicalWorkTestCase(tests_base.TestCase):
 
         mock_update.assert_called_once_with(iqn)
 
+        mock_check_dev.assert_called_once_with(address, port, iqn)
+
     def test_always_logout_and_delete_iscsi(self):
         """Check if logout_iscsi() and delete_iscsi() are called.