openstack/ironic
Code Issues Proposed changes

Make reservation checks caseless

Browse Source 
If a conductor hostname is changed while reservations are
issued to a conductor with one hostname, such as 'hostName'
and then the process is restarted with 'hostname', then the
queries would not match the node and effectively the nodes
would become inaccessible until the reservation is cleared.

This patch changes the queries to use a case insenitive
like search, better known as ilike.

Special thanks to the nova team for finding a bug in the hash
ring calculation in the ironic virt driver where a customer
changed their nova-compute hostname, which inspired this
patch.

Change-Id: Ib7da925ba5ca6a82ba542e0f4ae4cf7f0d070835
This commit is contained in:
Julia Kreger 2020-03-06 12:25:33 -08:00
parent 7a15df60c3
commit ae3a14e65c
3 changed files with 18 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ironic/db/sqlalchemy/api.py
View File

@ -999,9 +999,9 @@ class Connection(api.Connection):
nodes = []
with _session_for_write():
query = (model_query(models.Node)
.filter_by(reservation=hostname))
.filter(models.Node.reservation.ilike(hostname)))
nodes = [node['uuid'] for node in query]
query.update({'reservation': None})
query.update({'reservation': None}, synchronize_session=False)
if nodes:
nodes = ', '.join(nodes)
@ -1014,13 +1014,14 @@ class Connection(api.Connection):
nodes = []
with _session_for_write():
query = (model_query(models.Node)
.filter_by(reservation=hostname))
.filter(models.Node.reservation.ilike(hostname)))
query = query.filter(models.Node.target_power_state != sql.null())
nodes = [node['uuid'] for node in query]
query.update({'target_power_state': None,
'last_error': _("Pending power operation was "
"aborted due to conductor "
"restart")})
"restart")},
synchronize_session=False)
if nodes:
nodes = ', '.join(nodes)

8
ironic/tests/unit/db/test_conductor.py
View File

@ -179,13 +179,16 @@ class DbConductorTestCase(base.DbTestCase):
node1 = self.dbapi.create_node({'reservation': 'hostname1'})
node2 = self.dbapi.create_node({'reservation': 'hostname2'})
node3 = self.dbapi.create_node({'reservation': None})
node4 = self.dbapi.create_node({'reservation': 'hostName1'})
self.dbapi.clear_node_reservations_for_conductor('hostname1')
node1 = self.dbapi.get_node_by_id(node1.id)
node2 = self.dbapi.get_node_by_id(node2.id)
node3 = self.dbapi.get_node_by_id(node3.id)
node4 = self.dbapi.get_node_by_id(node4.id)
self.assertIsNone(node1.reservation)
self.assertEqual('hostname2', node2.reservation)
self.assertIsNone(node3.reservation)
self.assertIsNone(node4.reservation)
def test_clear_node_target_power_state(self):
node1 = self.dbapi.create_node({'reservation': 'hostname1',
@ -194,16 +197,21 @@ class DbConductorTestCase(base.DbTestCase):
'target_power_state': 'power on'})
node3 = self.dbapi.create_node({'reservation': None,
'target_power_state': 'power on'})
node4 = self.dbapi.create_node({'reservation': 'hostName1',
'target_power_state': 'power on'})
self.dbapi.clear_node_target_power_state('hostname1')
node1 = self.dbapi.get_node_by_id(node1.id)
node2 = self.dbapi.get_node_by_id(node2.id)
node3 = self.dbapi.get_node_by_id(node3.id)
node4 = self.dbapi.get_node_by_id(node4.id)
self.assertIsNone(node1.target_power_state)
self.assertIn('power operation was aborted', node1.last_error)
self.assertEqual('power on', node2.target_power_state)
self.assertIsNone(node2.last_error)
self.assertEqual('power on', node3.target_power_state)
self.assertIsNone(node3.last_error)
self.assertIsNone(node4.target_power_state)
self.assertIn('power operation was aborted', node4.last_error)
@mock.patch.object(timeutils, 'utcnow', autospec=True)
def test_get_active_hardware_type_dict_one_host_no_ht(self, mock_utcnow):

5
releasenotes/notes/caseless-conductor-restart-check-f70005fbf65f6bb6.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
Fixes an issue where a node may be locked from changes if a conductor's
hostname case is changed before restarting the conductor service.