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

@@ -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.')),
 ]
 
 

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)
 

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'