Prevent changes of a resource class for an active node

Doing so would confuse nova-scheduler, and may result in attempts
to schedule a new instance on such node. The API version is not
updated, as this behavior is broken already, we're just moving
the breakage to the API level.

Change-Id: I758587d36c927c8eed852170728f6267ae18f001
This commit is contained in:
Dmitry Tantsur 2017-08-09 20:13:34 +02:00
parent 15c31b9023
commit 6525a0da29
3 changed files with 75 additions and 4 deletions

View File

@ -165,6 +165,9 @@ class ConductorManager(base_manager.BaseConductorManager):
# interfaces for active nodes in the future.
allowed_update_states = [states.ENROLL, states.INSPECTING,
states.MANAGEABLE, states.AVAILABLE]
action = _("Node %(node)s can not have %(field)s "
"updated unless it is in one of allowed "
"(%(allowed)s) states or in maintenance mode.")
for iface in drivers_base.ALL_INTERFACES:
interface_field = '%s_interface' % iface
if interface_field not in delta:
@ -172,13 +175,10 @@ class ConductorManager(base_manager.BaseConductorManager):
if not (node_obj.provision_state in allowed_update_states or
node_obj.maintenance):
action = _("Node %(node)s can not have %(iface)s "
"updated unless it is in one of allowed "
"(%(allowed)s) states or in maintenance mode.")
raise exception.InvalidState(
action % {'node': node_obj.uuid,
'allowed': ', '.join(allowed_update_states),
'iface': interface_field})
'field': interface_field})
driver_factory.check_and_update_node_interfaces(node_obj)
@ -192,6 +192,17 @@ class ConductorManager(base_manager.BaseConductorManager):
raise exception.NodeAssociated(
node=node_id, instance=task.node.instance_uuid)
# NOTE(dtantsur): if the resource class is changed for an active
# instance, nova will not update its internal record. That will
# result in the new resource class exposed on the node as available
# for consumption, and nova may try to schedule on this node again.
if ('resource_class' in delta and task.node.resource_class and
task.node.provision_state not in allowed_update_states):
raise exception.InvalidState(
action % {'node': node_obj.uuid,
'allowed': ', '.join(allowed_update_states),
'field': 'resource_class'})
node_obj.save()
return node_obj

View File

@ -655,6 +655,54 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
for iface_name in self.IFACE_UPDATE_DICT:
self._test_update_node_interface_invalid(node, iface_name)
def _test_update_node_change_resource_class(self, state,
resource_class=None,
new_resource_class='new',
expect_error=False):
node = obj_utils.create_test_node(self.context, driver='fake',
uuid=uuidutils.generate_uuid(),
provision_state=state,
resource_class=resource_class)
self.addCleanup(node.destroy)
node.resource_class = new_resource_class
if expect_error:
exc = self.assertRaises(messaging.rpc.ExpectedException,
self.service.update_node,
self.context,
node)
# Compare true exception hidden by @messaging.expected_exceptions
self.assertEqual(exception.InvalidState, exc.exc_info[0])
# verify change did not happen
res = objects.Node.get_by_uuid(self.context, node['uuid'])
self.assertEqual(resource_class, res['resource_class'])
else:
self.service.update_node(self.context, node)
res = objects.Node.get_by_uuid(self.context, node['uuid'])
self.assertEqual('new', res['resource_class'])
def test_update_resource_class_allowed_state(self):
for state in [states.ENROLL, states.MANAGEABLE, states.INSPECTING,
states.AVAILABLE]:
self._test_update_node_change_resource_class(
state, resource_class='old', expect_error=False)
def test_update_resource_class_no_previous_value(self):
for state in [states.ENROLL, states.MANAGEABLE, states.INSPECTING,
states.AVAILABLE, states.ACTIVE]:
self._test_update_node_change_resource_class(
state, resource_class=None, expect_error=False)
def test_update_resource_class_not_allowed(self):
self._test_update_node_change_resource_class(
states.ACTIVE, resource_class='old', new_resource_class='new',
expect_error=True)
self._test_update_node_change_resource_class(
states.ACTIVE, resource_class='old', new_resource_class=None,
expect_error=True)
@mgr_utils.mock_record_keepalive
class VendorPassthruTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):

View File

@ -0,0 +1,12 @@
---
upgrade:
- |
Changing the ``resource_class`` field of a node in the ``active`` state
or any of the transient states is no longer possible. Please update your
scripts to only set a resource class for nodes that are not deployed to.
Setting a resource class for nodes that do not have it is still possible.
fixes:
- |
No longer allows changing the ``resource_class`` field for ``active`` nodes
if it was already set to a non-empty value. Doing so would break the
Compute scheduler.