Revisit required provision states

* Recommend using ENROLL for setting IPMI credentials
* Deprecate using maintenance mode, will be dropped once we stop
  supporting Ironic Kilo
* Drop bits related to Ironic Juno, we no longer support it
* Clarify error message about wrong provision state, stop mentioning
  maintenance mode there

Change-Id: I3a9d3ba24a32c7844cd6fd5e4f8ec4b15b2d9d20
Closes-Bug: #1479331

README.rst

@@ -262,29 +262,20 @@ drivers, please refer to `Ironic inspection documentation`_ for details.
 Node States
 ~~~~~~~~~~~
 
-* As of Ironic Kilo release the nodes should be moved to ``MANAGEABLE``
-  provision state before introspection (requires *python-ironicclient*
-  of version 0.5.0 or newer)::
+* The nodes should be moved to ``MANAGEABLE`` provision state before
+  introspection (requires *python-ironicclient* of version 0.5.0 or newer)::
 
     ironic node-set-provision-state <UUID> manage
 
-  With Juno release and/or older *python-ironicclient* it's recommended
-  to set maintenance mode, so that nodes are not taken by Nova for deploying::
-
-    ironic node-update <UUID> replace maintenance=true
-
 * After successful introspection and before deploying nodes should be made
-  available to Nova, either by moving them to ``AVAILABLE`` state (Kilo)::
+  available to Nova, by moving them to ``AVAILABLE`` state::
 
     ironic node-set-provision-state <UUID> provide
 
-  or by removing maintenance mode (Juno and/or older client)::
-
-    ironic node-update <UUID> replace maintenance=false
-
   .. note::
     Due to how Nova interacts with Ironic driver, you should wait 1 minute
-    before Nova becomes aware of available nodes after issuing these commands.
+    before Nova becomes aware of available nodes after issuing this command.
+    Use ``nova hypervisor-stats`` command output to check it.
 
 Setting IPMI Credentials
 ~~~~~~~~~~~~~~~~~~~~~~~~
@@ -296,13 +287,22 @@ is as follows:
 * Ensure nodes will PXE boot on the right network by default.
 
 * Set ``enable_setting_ipmi_credentials = true`` in the **ironic-inspector**
-  configuration file.
+  configuration file, restart **ironic-inspector**.
 
-* Enroll nodes in Ironic with setting their ``ipmi_address`` only. This step
-  allows **ironic-inspector** to distinguish nodes.
+* Enroll nodes in Ironic with setting their ``ipmi_address`` only (or
+  equivalent driver-specific property, as per ``ipmi_address_fields``
+  configuration option).
 
-* Set maintenance mode on nodes. That's an important step, otherwise Ironic
-  might interfere with introspection process.
+  With Ironic Liberty use ironic API version ``1.11``, so that new node gets
+  into ``enroll`` provision state::
+
+    ironic --ironic-api-version 1.11 node-create -d <DRIVER> -i ipmi_address=<ADDRESS>
+
+  Providing ``ipmi_address`` allows **ironic-inspector** to distinguish nodes.
+
+* With Ironic Kilo or older, set maintenance mode on nodes.
+  That's an important step, otherwise Ironic might interfere with introspection
+  process. This is replaced by ``enroll`` state in Ironic Liberty.
 
 * Start introspection with providing additional parameters:
 
@@ -313,7 +313,9 @@ is as follows:
 * Manually power on the nodes and wait.
 
 * After introspection is finished (watch nodes power state or use
-  **ironic-inspector** status API) you can turn maintenance mode off.
+  **ironic-inspector** status API) you can move node to ``manageable`` and
+  then ``available`` states - see `Node States`_. With Ironic Kilo you have to
+  move a node out of maintenance mode.
 
 Note that due to various limitations on password value in different BMC,
 **ironic-inspector** will only accept passwords with length between 1 and 20
@@ -358,8 +360,7 @@ Troubleshooting
 Errors when starting introspection
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-* *Refusing to introspect node <UUID> with provision state "available"
-  and maintenance mode off*
+* *Invalid provision state "available"*
 
   In Kilo release with *python-ironicclient* 0.5.0 or newer Ironic
   defaults to reporting provision state ``AVAILABLE`` for newly enrolled

ironic_inspector/introspect.py

@@ -44,11 +44,6 @@ def _validate_ipmi_credentials(node, new_ipmi_credentials):
         raise utils.Error(
             _('IPMI credentials setup is disabled in configuration'))
 
-    if not node.maintenance:
-        # Otherwise Ironic is going to interfere
-        raise utils.Error(_('Node should be in maintenance mode to set '
-                            'IPMI credentials on it'))
-
     new_username, new_password = new_ipmi_credentials
     if not new_username:
         new_username = node.driver_info.get('ipmi_username')
@@ -89,7 +84,7 @@ def introspect(uuid, new_ipmi_credentials=None, token=None):
         raise utils.Error(_("Cannot get node %(node)s: %(exc)s") %
                           {'node': uuid, 'exc': exc})
 
-    utils.check_provision_state(node)
+    utils.check_provision_state(node, with_credentials=new_ipmi_credentials)
 
     if new_ipmi_credentials:
         new_ipmi_credentials = (

ironic_inspector/test/functional.py

@@ -140,7 +140,7 @@ class Test(Base):
             {'op': 'add', 'path': '/driver_info/ipmi_password',
              'value': 'pwd'},
         ]
-        self.node.maintenance = True
+        self.node.provision_state = 'enroll'
         self.call_introspect(self.uuid, new_ipmi_username='admin',
                              new_ipmi_password='pwd')
         eventlet.greenthread.sleep(DEFAULT_SLEEP)

ironic_inspector/test/test_introspect.py

@@ -230,8 +230,7 @@ class TestIntrospect(BaseTest):
         cli.node.get.return_value = self.node
 
         self.assertRaisesRegexp(
-            utils.Error,
-            'node %s with provision state "active"' % self.uuid,
+            utils.Error, 'Invalid provision state "active"',
             introspect.introspect, self.uuid)
 
         self.assertEqual(0, self.node_info.ports.call_count)
@@ -322,7 +321,7 @@ class TestSetIpmiCredentials(BaseTest):
                           'processing')
         self.new_creds = ('user', 'password')
         self.node_info.options['new_ipmi_credentials'] = self.new_creds
-        self.node.maintenance = True
+        self.node.provision_state = 'enroll'
 
     def test_ok(self, client_mock, add_mock, filters_mock):
         cli = self._prepare(client_mock)
@@ -339,6 +338,24 @@ class TestSetIpmiCredentials(BaseTest):
         add_mock.return_value.set_option.assert_called_once_with(
             'new_ipmi_credentials', self.new_creds)
 
+    def test_any_state_with_maintenance(self, client_mock, add_mock,
+                                        filters_mock):
+        self.node.provision_state = 'manageable'
+        self.node.maintenance = True
+        cli = self._prepare(client_mock)
+        add_mock.return_value = self.node_info
+
+        introspect.introspect(self.uuid, new_ipmi_credentials=self.new_creds)
+
+        add_mock.assert_called_once_with(self.uuid,
+                                         bmc_address=self.bmc_address)
+        filters_mock.assert_called_with(cli)
+        self.assertFalse(cli.node.validate.called)
+        self.assertFalse(cli.node.set_boot_device.called)
+        self.assertFalse(cli.node.set_power_state.called)
+        add_mock.return_value.set_option.assert_called_once_with(
+            'new_ipmi_credentials', self.new_creds)
+
     def test_disabled(self, client_mock, add_mock, filters_mock):
         CONF.set_override('enable_setting_ipmi_credentials', False,
                           'processing')
@@ -385,8 +402,8 @@ class TestSetIpmiCredentials(BaseTest):
         self.assertRaises(utils.Error, introspect.introspect, self.uuid,
                           new_ipmi_credentials=self.new_creds)
 
-    def test_require_maintenance(self, client_mock, add_mock, filters_mock):
-        self.node.maintenance = False
+    def test_wrong_state(self, client_mock, add_mock, filters_mock):
+        self.node.provision_state = 'manageable'
         self._prepare(client_mock)
 
         self.assertRaises(utils.Error, introspect.introspect, self.uuid,

ironic_inspector/utils.py

@@ -22,12 +22,13 @@ from oslo_config import cfg
 from oslo_log import log
 import six
 
-from ironic_inspector.common.i18n import _, _LE, _LI
+from ironic_inspector.common.i18n import _, _LE, _LW
 
 CONF = cfg.CONF
 
 # See http://specs.openstack.org/openstack/ironic-specs/specs/kilo/new-ironic-state-machine.html  # noqa
 VALID_STATES = {'enroll', 'manageable', 'inspecting', 'inspectfail'}
+SET_CREDENTIALS_VALID_STATES = {'enroll'}
 
 
 LOG = log.getLogger('ironic_inspector.utils')
@@ -167,16 +168,23 @@ def get_ipmi_address(node):
                 raise Error(msg % (value, node.uuid))
 
 
-def check_provision_state(node):
-    if not node.maintenance:
-        provision_state = node.provision_state
-        if provision_state and provision_state.lower() not in VALID_STATES:
-            msg = _('Refusing to introspect node %(node)s with provision state'
-                    ' "%(state)s" and maintenance mode off')
-            raise Error(msg % {'node': node.uuid, 'state': provision_state})
-    else:
-        LOG.info(_LI('Node %s is in maintenance mode, skipping'
-                     ' provision states check'), node.uuid)
+def check_provision_state(node, with_credentials=False):
+    if node.maintenance:
+        LOG.warn(_LW('Introspecting nodes in maintenance mode is deprecated, '
+                     'accepted states: %s'), VALID_STATES)
+        return
+
+    state = node.provision_state.lower()
+    if with_credentials and state not in SET_CREDENTIALS_VALID_STATES:
+        msg = _('Invalid provision state "%(state)s" for setting IPMI '
+                'credentials on node %(node)s, valid states are %(valid)s')
+        raise Error(msg % {'node': node.uuid, 'state': state,
+                           'valid': list(SET_CREDENTIALS_VALID_STATES)})
+    elif not with_credentials and state not in VALID_STATES:
+        msg = _('Invalid provision state "%(state)s" for introspection of '
+                'node %(node)s, valid states are "%(valid)s"')
+        raise Error(msg % {'node': node.uuid, 'state': state,
+                           'valid': list(VALID_STATES)})
 
 
 def capabilities_to_dict(caps):