Clear locks on conductor startup

This causes all node reservations being held by a given conductor
hostname to be cleared when that conductor initializes, just
before registration. This ensures that any locks that are stuck when a
conductor shuts down are cleared when the conductor starts again.

Partial-Bug: #1382698

Change-Id: I0c3c2a9c94b6a85a1cbbcc570f57c3c34c256092
(cherry picked from commit 2a4a9d7fdac3642724c2f1a358bb9eb01b63a888)

4 files changed, 37 insertions, 0 deletions

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 0a2edfccd..1328b3e4e 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -204,6 +204,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
             LOG.error(msg, self.host)
             raise exception.NoDriversLoaded(conductor=self.host)
 
+        # clear all locks held by this conductor before registering
+        self.dbapi.clear_node_reservations_for_conductor(self.host)
         try:
             # Register this conductor with the cluster
             cdr = self.dbapi.register_conductor({'hostname': self.host,
diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index d01b2359d..98ef4b5ec 100644
--- a/ironic/db/sqlalchemy/api.py
+++ b/ironic/db/sqlalchemy/api.py
@@ -28,6 +28,7 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from ironic.common import exception
 from ironic.common.i18n import _
+from ironic.common.i18n import _LW
 from ironic.common import states
 from ironic.common import utils
 from ironic.db import api
@@ -553,6 +554,20 @@ class Connection(api.Connection):
             if count == 0:
                 raise exception.ConductorNotFound(conductor=hostname)
 
+    def clear_node_reservations_for_conductor(self, hostname):
+        session = get_session()
+        nodes = []
+        with session.begin():
+            query = model_query(models.Node, session=session).filter_by(
+                    reservation=hostname)
+            nodes = [node['uuid'] for node in query]
+            query.update({'reservation': None})
+
+        if nodes:
+            nodes = ', '.join(nodes)
+            LOG.warn(_LW('Cleared reservations held by %(hostname)s: '
+                         '%(nodes)s'), {'hostname': hostname, 'nodes': nodes})
+
     def get_active_driver_dict(self, interval=None):
         if interval is None:
             interval = CONF.conductor.heartbeat_timeout
diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py
index e13465291..ca48e34ec 100644
--- a/ironic/tests/conductor/test_manager.py
+++ b/ironic/tests/conductor/test_manager.py
@@ -178,6 +178,14 @@ class StartStopTestCase(_ServiceSetUpMixin, tests_db_base.DbTestCase):
         res = objects.Conductor.get_by_hostname(self.context, self.hostname)
         self.assertEqual(self.hostname, res['hostname'])
 
+    def test_start_clears_conductor_locks(self):
+        node = obj_utils.create_test_node(self.context,
+                                          reservation=self.hostname)
+        node.save()
+        self._start_service()
+        node.refresh()
+        self.assertIsNone(node.reservation)
+
     def test_stop_unregisters_conductor(self):
         self._start_service()
         res = objects.Conductor.get_by_hostname(self.context, self.hostname)
diff --git a/ironic/tests/db/test_conductor.py b/ironic/tests/db/test_conductor.py
index 4501d3218..760adce69 100644
--- a/ironic/tests/db/test_conductor.py
+++ b/ironic/tests/db/test_conductor.py
@@ -103,6 +103,18 @@ class DbConductorTestCase(base.DbTestCase):
         self.dbapi.touch_conductor(c.hostname)
         self.dbapi.get_conductor(c.hostname)
 
+    def test_clear_node_reservations_for_conductor(self):
+        node1 = self.dbapi.create_node({'reservation': 'hostname1'})
+        node2 = self.dbapi.create_node({'reservation': 'hostname2'})
+        node3 = self.dbapi.create_node({'reservation': None})
+        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)
+        self.assertIsNone(node1.reservation)
+        self.assertEqual('hostname2', node2.reservation)
+        self.assertIsNone(node3.reservation)
+
     @mock.patch.object(timeutils, 'utcnow')
     def test_get_active_driver_dict_one_host_no_driver(self, mock_utcnow):
         h = 'fake-host'