summaryrefslogtreecommitdiff
path: root/ironic
diff options
context:
space:
mode:
authorJulia Kreger <juliaashleykreger@gmail.com>2020-03-06 12:25:33 -0800
committerRiccardo Pittau <elfosardo@gmail.com>2020-03-10 12:54:17 +0000
commita18f5ef1876e46aa976db3e8e077451d431bcd6c (patch)
treefa04cbc1ad5476c0ea838a0d761767986e31dcc2 /ironic
parent73ec9b8e8e7bb06ef02507120b0d5bfe15e38034 (diff)
downloadironic-a18f5ef1876e46aa976db3e8e077451d431bcd6c.tar.gz
Make reservation checks caseless
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 (cherry picked from commit ae3a14e65c81f48472040a086075b63ca1ce76ac)
Diffstat (limited to 'ironic')
-rw-r--r--ironic/db/sqlalchemy/api.py9
-rw-r--r--ironic/tests/unit/db/test_conductor.py8
2 files changed, 13 insertions, 4 deletions
diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index 8ea75ba04..47353e716 100644
--- a/ironic/db/sqlalchemy/api.py
+++ b/ironic/db/sqlalchemy/api.py
@@ -983,9 +983,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)
@@ -998,13 +998,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)
diff --git a/ironic/tests/unit/db/test_conductor.py b/ironic/tests/unit/db/test_conductor.py
index ebde34817..1ae38de88 100644
--- a/ironic/tests/unit/db/test_conductor.py
+++ b/ironic/tests/unit/db/test_conductor.py
@@ -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):