From 386743148866a2b6dd52738d36733d9377564dcf Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Fri, 30 Aug 2019 16:42:13 +0200
Subject: [PATCH] Prepare for deprecation of iscsi_verify_attempts in
 ironic-lib

ironic-lib itself is not concerned with iscsi deploy, and this option
is actually used for partition detection retry count. This is
confusing, so this patch moves the option to ironic.

Change-Id: Idacf9b576173f878475a4b7e1503109095dafcd0
---
 ironic/conf/iscsi.py                          | 10 +++++++++-
 ironic/drivers/modules/deploy_utils.py        | 19 ++++++++++++++-----
 .../unit/drivers/modules/test_deploy_utils.py |  9 +++++++++
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/ironic/conf/iscsi.py b/ironic/conf/iscsi.py
index c01053f8fd..0ac340012b 100644
--- a/ironic/conf/iscsi.py
+++ b/ironic/conf/iscsi.py
@@ -27,7 +27,15 @@ opts = [
                help=_('Flags that need to be sent to the dd command, '
                       'to control the conversion of the original file '
                       'when copying to the host. It can contain several '
-                      'options separated by commas.'))
+                      'options separated by commas.')),
+    # TODO(dtantsur): update in Ussuri when the deprecated option is removed
+    # from ironic-lib.
+    cfg.IntOpt('verify_attempts',
+               min=1,
+               help=_('Maximum attempts to verify an iSCSI connection is '
+                      'active, sleeping 1 second between attempts. Defaults '
+                      'to the deprecated [disk_utils]iscsi_verify_attempts '
+                      'option, after its removal will default to 3.')),
 ]
 
 
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index 6ba3555052..6e0d0c8d53 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -88,6 +88,15 @@ def _get_ironic_session():
     return _IRONIC_SESSION
 
 
+# TODO(dtantsur): just use CONF.iscsi.verify_attempts when
+# iscsi_verify_attempts is removed from ironic-lib.
+def _iscsi_verify_attempts():
+    # Be prepared for eventual removal, hardcode the default of 3
+    return (getattr(CONF.disk_utils, 'iscsi_verify_attempts', 3)
+            if CONF.iscsi.verify_attempts is None
+            else CONF.iscsi.verify_attempts)
+
+
 def _wrap_ipv6(ip):
     if netutils.is_valid_ipv6(ip):
         return "[%s]" % ip
@@ -185,7 +194,7 @@ def check_file_system_for_iscsi_device(portal_address,
     check_dir = "/dev/disk/by-path/ip-%s:%s-iscsi-%s-lun-1" % (portal_address,
                                                                portal_port,
                                                                target_iqn)
-    total_checks = CONF.disk_utils.iscsi_verify_attempts
+    total_checks = _iscsi_verify_attempts()
     for attempt in range(total_checks):
         if os.path.exists(check_dir):
             break
@@ -209,7 +218,8 @@ def verify_iscsi_connection(target_iqn):
     """Verify iscsi connection."""
     LOG.debug("Checking for iSCSI target to become active.")
 
-    for attempt in range(CONF.disk_utils.iscsi_verify_attempts):
+    total_checks = _iscsi_verify_attempts()
+    for attempt in range(total_checks):
         out, _err = utils.execute('iscsiadm',
                                   '-m', 'node',
                                   '-S',
@@ -220,11 +230,10 @@ def verify_iscsi_connection(target_iqn):
         time.sleep(1)
         LOG.debug("iSCSI connection not active. Rechecking. Attempt "
                   "%(attempt)d out of %(total)d",
-                  {"attempt": attempt + 1,
-                   "total": CONF.disk_utils.iscsi_verify_attempts})
+                  {"attempt": attempt + 1, "total": total_checks})
     else:
         msg = _("iSCSI connection did not become active after attempting to "
-                "verify %d times.") % CONF.disk_utils.iscsi_verify_attempts
+                "verify %d times.") % total_checks
         LOG.error(msg)
         raise exception.InstanceDeployFailure(msg)
 
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index 6b464aa607..b3029e317e 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -645,6 +645,15 @@ class PhysicalWorkTestCase(tests_base.TestCase):
                           utils.verify_iscsi_connection, iqn)
         self.assertEqual(3, mock_exec.call_count)
 
+    @mock.patch.object(common_utils, 'execute', autospec=True)
+    def test_verify_iscsi_connection_override_attempts(self, mock_exec):
+        utils.CONF.set_override('verify_attempts', 2, group='iscsi')
+        iqn = 'iqn.xyz'
+        mock_exec.return_value = ['iqn.abc', '']
+        self.assertRaises(exception.InstanceDeployFailure,
+                          utils.verify_iscsi_connection, iqn)
+        self.assertEqual(2, mock_exec.call_count)
+
     @mock.patch.object(os.path, 'exists', autospec=True)
     def test_check_file_system_for_iscsi_device_raises(self, mock_os):
         iqn = 'iqn.xyz'