From 2b55444f37c4b1cb93064589e38bc498a942720e Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Wed, 15 Jun 2022 10:50:36 +1200 Subject: [PATCH] Allocation candidates prefer matching name This change finds a node with the same name as the allocation and moves it to the beginning of the shuffled candidate list so that node is the first allocation attempt. It is common for node naming scheme to match the node's role (such as compute-1, compute-2). Also this often matches the hostname (allocation name) scheme. Without this change, this scenario will generally result in swapped names (node compute-1 having hostname compute-2, etc). By preferring matching names this situation can be avoided in the majority of cases, while not otherwise affecting the candidiate allocation approach. Change-Id: Ie990bfc209959d58852b9080778602eab5aa30af --- .../source/baremetal-api-v1-allocation.inc | 6 +++- ironic/api/controllers/v1/versions.py | 4 ++- ironic/common/release_mappings.py | 2 +- ironic/conductor/allocations.py | 9 +++++ .../tests/unit/conductor/test_allocations.py | 35 +++++++++++++++++++ ...allocation-node-name-46b473ec82662f7f.yaml | 7 ++++ 6 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/allocation-node-name-46b473ec82662f7f.yaml diff --git a/api-ref/source/baremetal-api-v1-allocation.inc b/api-ref/source/baremetal-api-v1-allocation.inc index 808005dba2..fe699efaeb 100644 --- a/api-ref/source/baremetal-api-v1-allocation.inc +++ b/api-ref/source/baremetal-api-v1-allocation.inc @@ -48,7 +48,11 @@ parameters must be missing or match the provided node. Added support for backfilling allocations. .. versionadded:: 1.60 - Introduced the ``owner`` field. + Introduced the ``owner`` field. + +.. versionadded:: 1.79 + A node with the same name as the allocation ``name`` is moved to the + start of the derived candidiate list. Normal response codes: 201 diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 14330a7340..7fc80bc970 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -116,6 +116,7 @@ BASE_VERSION = 1 # v1.76: Add support for changing boot_mode and secure_boot state # v1.77: Add fields selector to drivers list and driver detail. # v1.78: Add node history endpoint +# v1.79: Change allocation behaviour to prefer node name match MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -196,6 +197,7 @@ MINOR_75_NODE_BOOT_MODE = 75 MINOR_76_NODE_CHANGE_BOOT_MODE = 76 MINOR_77_DRIVER_FIELDS_SELECTOR = 77 MINOR_78_NODE_HISTORY = 78 +MINOR_79_ALLOCATION_NODE_NAME = 79 # When adding another version, update: # - MINOR_MAX_VERSION @@ -203,7 +205,7 @@ MINOR_78_NODE_HISTORY = 78 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_78_NODE_HISTORY +MINOR_MAX_VERSION = MINOR_79_ALLOCATION_NODE_NAME # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index a054501eb4..aaf423bfe1 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -471,7 +471,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.78', + 'api': '1.79', 'rpc': '1.55', 'objects': { 'Allocation': ['1.1'], diff --git a/ironic/conductor/allocations.py b/ironic/conductor/allocations.py index b6ce3ed1c8..298174fbbd 100644 --- a/ironic/conductor/allocations.py +++ b/ironic/conductor/allocations.py @@ -141,6 +141,15 @@ def _candidate_nodes(context, allocation): # in the same order. random.shuffle(nodes) + # NOTE(sbaker): if the allocation name matches a node name, attempt that + # node first. This will reduce confusion when nodes have the same naming + # scheme as allocations. + if allocation.name: + for i, node in enumerate(nodes): + if node.name == allocation.name: + nodes.insert(0, nodes.pop(i)) + break + LOG.debug('%(count)d nodes are candidates for allocation %(uuid)s', {'count': len(nodes), 'uuid': allocation.uuid}) return nodes diff --git a/ironic/tests/unit/conductor/test_allocations.py b/ironic/tests/unit/conductor/test_allocations.py index 91046b72f5..d063cd13af 100644 --- a/ironic/tests/unit/conductor/test_allocations.py +++ b/ironic/tests/unit/conductor/test_allocations.py @@ -476,6 +476,41 @@ class DoAllocateTestCase(db_base.DbTestCase): # All nodes are filtered out on the database level. self.assertFalse(mock_acquire.called) + def test_name_match_first(self): + obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + name='node-1', + power_state='power on', + resource_class='x-large', + provision_state='available') + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + name='node-2', + power_state='power on', + resource_class='x-large', + provision_state='available') + obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + name='node-3', + power_state='power on', + resource_class='x-large', + provision_state='available') + allocation = obj_utils.create_test_allocation(self.context, + name='node-2', + resource_class='x-large') + + allocations.do_allocate(self.context, allocation) + + allocation = objects.Allocation.get_by_uuid(self.context, + allocation['uuid']) + self.assertIsNone(allocation['last_error']) + self.assertEqual('active', allocation['state']) + + node = objects.Node.get_by_uuid(self.context, node['uuid']) + self.assertEqual(allocation['uuid'], node['instance_uuid']) + self.assertEqual(allocation['id'], node['allocation_id']) + self.assertEqual('node-2', node['name']) + class BackfillAllocationTestCase(db_base.DbTestCase): def test_with_associated_node(self): diff --git a/releasenotes/notes/allocation-node-name-46b473ec82662f7f.yaml b/releasenotes/notes/allocation-node-name-46b473ec82662f7f.yaml new file mode 100644 index 0000000000..0cfab27d11 --- /dev/null +++ b/releasenotes/notes/allocation-node-name-46b473ec82662f7f.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + When an allocation is being processed, the randomized candidate list is now + modified so that a node with a matching name to the allocation is moved + to the beginning of the list. This greatly increases the chance of node name + and allocation name matching in environments where the naming schemes align. \ No newline at end of file