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

3 files changed, 32 insertions, 6 deletions

diff --git a/ironic/conf/iscsi.py b/ironic/conf/iscsi.py
index c01053f8f..0ac340012 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 6ba355505..6e0d0c8d5 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 6b464aa60..b3029e317 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'