From f484bf617260b643080e5ae84d6bdf9cc07e8970 Mon Sep 17 00:00:00 2001
From: Zhenguo Niu <Niu.ZGlinux@gmail.com>
Date: Fri, 6 Jan 2017 15:55:31 +0800
Subject: [PATCH] Add a looping call to wait for power action completed

1. Change to use rpc cast for set_power_state.
2. Wait until node target power state goes to NOSTATE.

Change-Id: I4fc2acb4d5551cbfb74b35183aaee2a9bdd3086e
---
 mogan/engine/manager.py                 | 30 +++++++++++++++++++------
 mogan/engine/rpcapi.py                  |  3 +--
 mogan/tests/unit/engine/test_manager.py |  9 ++++++--
 mogan/tests/unit/engine/test_rpcapi.py  |  2 +-
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/mogan/engine/manager.py b/mogan/engine/manager.py
index 319e623e..4179a1c9 100644
--- a/mogan/engine/manager.py
+++ b/mogan/engine/manager.py
@@ -239,16 +239,32 @@ class EngineManager(base_manager.BaseEngineManager):
 
         return self._instance_states(context, instance)
 
-    def _set_power_state(self, context, instance, state):
-        ironic.set_power_state(self.ironicclient, instance.node_uuid, state)
-        LOG.info(_LI('Successfully set ironic node power state: %s'),
-                 state)
+    def _wait_for_power_state(self, instance):
+        """Wait for the node to complete a power state change."""
+        try:
+            node = ironic.get_node_by_instance(self.ironicclient,
+                                               instance.uuid)
+        except ironic_exc.NotFound:
+            LOG.debug("While waiting for node to complete a power state "
+                      "change, it dissociate with the instance.",
+                      instance=instance)
+            raise exception.NodeNotFound()
+
+        if node.target_power_state == ironic_states.NOSTATE:
+            raise loopingcall.LoopingCallDone()
 
     def set_power_state(self, context, instance, state):
-        """Get an instance states."""
-        LOG.debug("set power state...")
+        """Set power state for the specified instance."""
+        LOG.debug('Power %(state)s called for instance %(instance)s',
+                  {'state': state,
+                   'instance': instance})
+        ironic.set_power_state(self.ironicclient, instance.node_uuid, state)
 
-        return self._set_power_state(context, instance, state)
+        timer = loopingcall.FixedIntervalLoopingCall(
+            self._wait_for_power_state, instance)
+        timer.start(interval=CONF.ironic.api_retry_interval).wait()
+        LOG.info(_LI('Successfully set node power state: %s'),
+                 state, instance=instance)
 
     @messaging.expected_exceptions(exception.NodeNotFound)
     def get_ironic_node(self, context, instance_uuid, fields):
diff --git a/mogan/engine/rpcapi.py b/mogan/engine/rpcapi.py
index d2098aa0..78ae9b22 100644
--- a/mogan/engine/rpcapi.py
+++ b/mogan/engine/rpcapi.py
@@ -71,8 +71,7 @@ class EngineAPI(object):
     def set_power_state(self, context, instance, state):
         """Signal to engine service to perform power action on instance."""
         cctxt = self.client.prepare(topic=self.topic, server=CONF.host)
-        # need return?
-        return cctxt.call(context, 'set_power_state',
+        return cctxt.cast(context, 'set_power_state',
                           instance=instance, state=state)
 
     def get_ironic_node(self, context, instance_uuid, fields):
diff --git a/mogan/tests/unit/engine/test_manager.py b/mogan/tests/unit/engine/test_manager.py
index bc61ac82..e0a1db7a 100644
--- a/mogan/tests/unit/engine/test_manager.py
+++ b/mogan/tests/unit/engine/test_manager.py
@@ -148,10 +148,14 @@ class ManageInstanceTestCase(mgr_utils.ServiceSetUpMixin,
 
         self.assertFalse(destroy_inst_mock.called)
 
+    @mock.patch.object(ironic, 'get_node_by_instance')
     @mock.patch.object(ironic, 'set_power_state')
-    def test_change_instance_power_state(self, set_power_mock,
-                                         refresh_cache_mock):
+    def test_change_instance_power_state(
+            self, set_power_mock, get_node_mock, refresh_cache_mock):
         instance = obj_utils.create_test_instance(self.context)
+        fake_node = mock.MagicMock()
+        fake_node.target_power_state = ironic_states.NOSTATE
+        get_node_mock.return_value = fake_node
         refresh_cache_mock.side_effect = None
         self._start_service()
 
@@ -161,6 +165,7 @@ class ManageInstanceTestCase(mgr_utils.ServiceSetUpMixin,
 
         set_power_mock.assert_called_once_with(mock.ANY, instance.node_uuid,
                                                ironic_states.POWER_ON)
+        get_node_mock.assert_called_once_with(mock.ANY, instance.uuid)
 
     @mock.patch.object(ironic, 'get_node_states')
     def test_get_instance_states(self, get_states_mock, refresh_cache_mock):
diff --git a/mogan/tests/unit/engine/test_rpcapi.py b/mogan/tests/unit/engine/test_rpcapi.py
index bfb42e2d..4fd29e21 100644
--- a/mogan/tests/unit/engine/test_rpcapi.py
+++ b/mogan/tests/unit/engine/test_rpcapi.py
@@ -124,7 +124,7 @@ class RPCAPITestCase(base.DbTestCase):
 
     def test_set_power_state(self):
         self._test_rpcapi('set_power_state',
-                          'call',
+                          'cast',
                           version='1.0',
                           instance=self.fake_instance_obj,
                           state='power on')