From d52077f4fe8c668b258702e8298a4beaa19476d8 Mon Sep 17 00:00:00 2001
From: Zhenguo Niu <Niu.ZGlinux@gmail.com>
Date: Tue, 26 Apr 2016 10:19:04 +0800
Subject: [PATCH] Clear target_power_state on conductor startup

During clearing locks, also clear target_power_state. As nodes
may locked in powering process, sync_power_state task will sync
the power_state field, but nobody handles target_power_state.

Change-Id: I2293e03c05e13c716f78533680d128ba45ccda02
Closes-Bug: #1567255
---
 ironic/conductor/base_manager.py               |  2 ++
 ironic/db/sqlalchemy/api.py                    | 18 ++++++++++++++++++
 ironic/tests/unit/db/test_conductor.py         | 18 ++++++++++++++++++
 ...de-target-power-state-de1f25be46d3e6d7.yaml |  4 ++++
 4 files changed, 42 insertions(+)
 create mode 100644 releasenotes/notes/clear-node-target-power-state-de1f25be46d3e6d7.yaml

diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py
index 4d1a132e5e..3d0a48cfa0 100644
--- a/ironic/conductor/base_manager.py
+++ b/ironic/conductor/base_manager.py
@@ -139,6 +139,8 @@ class BaseConductorManager(object):
             self._periodic_task_callables,
             executor_factory=periodics.ExistingExecutor(self._executor))
 
+        # clear all target_power_state with locks by this conductor
+        self.dbapi.clear_node_target_power_state(self.host)
         # clear all locks held by this conductor before registering
         self.dbapi.clear_node_reservations_for_conductor(self.host)
         try:
diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py
index 02bf4782c1..c89d17d7a2 100644
--- a/ironic/db/sqlalchemy/api.py
+++ b/ironic/db/sqlalchemy/api.py
@@ -755,6 +755,24 @@ class Connection(api.Connection):
                 _LW('Cleared reservations held by %(hostname)s: '
                     '%(nodes)s'), {'hostname': hostname, 'nodes': nodes})
 
+    def clear_node_target_power_state(self, hostname):
+        nodes = []
+        with _session_for_write():
+            query = (model_query(models.Node)
+                     .filter_by(reservation=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"})
+
+        if nodes:
+            nodes = ', '.join(nodes)
+            LOG.warning(
+                _LW('Cleared target_power_state of the locked nodes in '
+                    'powering process, their power state can be incorrect: '
+                    '%(nodes)s'), {'nodes': nodes})
+
     def get_active_driver_dict(self, interval=None):
         if interval is None:
             interval = CONF.conductor.heartbeat_timeout
diff --git a/ironic/tests/unit/db/test_conductor.py b/ironic/tests/unit/db/test_conductor.py
index a6a4e16384..09341bf9c3 100644
--- a/ironic/tests/unit/db/test_conductor.py
+++ b/ironic/tests/unit/db/test_conductor.py
@@ -110,6 +110,24 @@ class DbConductorTestCase(base.DbTestCase):
         self.assertEqual('hostname2', node2.reservation)
         self.assertIsNone(node3.reservation)
 
+    def test_clear_node_target_power_state(self):
+        node1 = self.dbapi.create_node({'reservation': 'hostname1',
+                                        'target_power_state': 'power on'})
+        node2 = self.dbapi.create_node({'reservation': 'hostname2',
+                                        'target_power_state': 'power on'})
+        node3 = self.dbapi.create_node({'reservation': None,
+                                        '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)
+        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)
+
     @mock.patch.object(timeutils, 'utcnow', autospec=True)
     def test_get_active_driver_dict_one_host_no_driver(self, mock_utcnow):
         h = 'fake-host'
diff --git a/releasenotes/notes/clear-node-target-power-state-de1f25be46d3e6d7.yaml b/releasenotes/notes/clear-node-target-power-state-de1f25be46d3e6d7.yaml
new file mode 100644
index 0000000000..d9319ad198
--- /dev/null
+++ b/releasenotes/notes/clear-node-target-power-state-de1f25be46d3e6d7.yaml
@@ -0,0 +1,4 @@
+---
+fixes:
+  - Clear target_power_state of the nodes locked by the conductor on its
+    startup.