From f147d3ece9d22f087a2bb01de6103685f617e59c Mon Sep 17 00:00:00 2001
From: Raphael Glon <raphael.glon@corp.ovh.com>
Date: Fri, 6 Sep 2019 15:28:21 +0200
Subject: [PATCH] Minor: change a misleading InvalidState error message

Only in a very specific case: when trying to update resource class
on nodes, no matter if the node is in maintenance or not, the
update is not possible.

Change-Id: I38be81e198a69083207f28e0d8d325cd4845c46d
---
 ironic/conductor/manager.py                 |  6 ++++--
 ironic/tests/unit/conductor/test_manager.py | 16 ++++++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 2085ed5be8..78e7ed36b4 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -242,10 +242,12 @@ class ConductorManager(base_manager.BaseConductorManager):
             if ('resource_class' in delta and task.node.resource_class
                     and task.node.provision_state
                     not in allowed_update_states):
+                action = _("Node %(node)s can not have resource_class "
+                           "updated unless it is in one of allowed "
+                           "(%(allowed)s) states.")
                 raise exception.InvalidState(
                     action % {'node': node_obj.uuid,
-                              'allowed': ', '.join(allowed_update_states),
-                              'field': 'resource_class'})
+                              'allowed': ', '.join(allowed_update_states)})
 
             if ('instance_uuid' in delta and task.node.allocation_id
                     and not node_obj.instance_uuid):
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 1622aac180..ed07927959 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -20,6 +20,7 @@
 
 from collections import namedtuple
 import datetime
+import re
 
 import eventlet
 from futurist import waiters
@@ -754,11 +755,13 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def _test_update_node_change_resource_class(self, state,
                                                 resource_class=None,
                                                 new_resource_class='new',
-                                                expect_error=False):
+                                                expect_error=False,
+                                                maintenance=False):
         node = obj_utils.create_test_node(self.context, driver='fake-hardware',
                                           uuid=uuidutils.generate_uuid(),
                                           provision_state=state,
-                                          resource_class=resource_class)
+                                          resource_class=resource_class,
+                                          maintenance=maintenance)
         self.addCleanup(node.destroy)
 
         node.resource_class = new_resource_class
@@ -770,6 +773,12 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             # Compare true exception hidden by @messaging.expected_exceptions
             self.assertEqual(exception.InvalidState, exc.exc_info[0])
 
+            expected_msg_regex = \
+                (r'^Node {} can not have resource_class updated unless it is '
+                 r'in one of allowed \(.*\) states.$').format(
+                    re.escape(node.uuid))
+            self.assertRegex(str(exc.exc_info[1]), expected_msg_regex)
+
             # verify change did not happen
             res = objects.Node.get_by_uuid(self.context, node['uuid'])
             self.assertEqual(resource_class, res['resource_class'])
@@ -798,6 +807,9 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self._test_update_node_change_resource_class(
             states.ACTIVE, resource_class='old', new_resource_class=None,
             expect_error=True)
+        self._test_update_node_change_resource_class(
+            states.ACTIVE, resource_class='old', new_resource_class=None,
+            expect_error=True, maintenance=True)
 
     def test_update_node_hardware_type(self):
         existing_hardware = 'fake-hardware'