Merge ~ltrager/maas:lp1761600 into maas:master

[Lee Trager]

LP: #1761600 - Don't return deferLater value.

In b352322 MAAS was modified to use asyncio or uvloop as the backend for
twisted. The value of deferLater was returned in maybe_change_power_state
which fixes a number of unit tests. Doing this causes the defer to be
resolved with the caller instead of running later. This meant when a user
issues a power action, such as deploy or release, the websocket and
subsequent RPC call didn't resolve until the power action is completed.

This removes the return so the power action can happen later greatly
speeding up power actions. The unit tests are fixed by patching
deferLater in the tests to use maybeDeferred.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1761600 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/2521/console
COMMIT: e8118113ba649fc522b9fb1fc6dd5326674c0d89

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1761600 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 6ea8f8a240839b705d19a51440b44f95425a6fc6

review: Approve

[Blake Rouse (blake-rouse)]

Glad the issue was narrowed down, but I think we should discuss the change.

[Blake Rouse (blake-rouse)]

Just the test I commented on below.

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Okay go ahead and land it.

review: Approve

diff --git a/src/provisioningserver/rpc/power.py b/src/provisioningserver/rpc/power.py
index 913f6bd..9af3579 100644
--- a/src/provisioningserver/rpc/power.py
+++ b/src/provisioningserver/rpc/power.py
@@ -251,7 +251,10 @@ def maybe_change_power_state(
             log.err, "%s: Power %s failed." % (
                 hostname, power_change))
 
-        return d
+        # LP: 1761600 - Returning d will cause d to be resolved with the
+        # caller. This causes power actions in the UI/API such as deploy,
+        # commission, or release to wait for the power action to complete
+        # before finishing.
 
     elif current_power_change == power_change:
         # What we want is already happening; let it continue.
diff --git a/src/provisioningserver/rpc/tests/test_power.py b/src/provisioningserver/rpc/tests/test_power.py
index 86ee0fa..9d433aa 100644
--- a/src/provisioningserver/rpc/tests/test_power.py
+++ b/src/provisioningserver/rpc/tests/test_power.py
@@ -147,7 +147,6 @@ class TestPowerHelpers(MAASTestCase):
         )
         self.assertIsNone(extract_result(d))
 
-    @inlineCallbacks
     def test_power_change_failure_emits_event(self):
         system_id = factory.make_name('system_id')
         hostname = factory.make_name('hostname')
@@ -157,7 +156,6 @@ class TestPowerHelpers(MAASTestCase):
         d = power.power_change_failure(
             system_id, hostname, power_change, message)
         io.flush()
-        yield d
         self.assertThat(
             protocol.SendEvent,
             MockCalledOnceWith(
@@ -388,6 +386,11 @@ class TestMaybeChangePowerState(MAASTestCase):
             self.patch(
                 power_driver, "detect_missing_packages").return_value = []
         self.useFixture(EventTypesAllRegistered())
+        # Defer later won't run during the test so replace it with a
+        # maybeDeferred.
+        self.patch(power, 'deferLater').side_effect = (
+            lambda clock, delay, func, *args, **kwargs:
+            maybeDeferred(func, *args, **kwargs))
 
     def patch_methods_using_rpc(self):
         pcs = self.patch_autospec(power, 'power_change_starting')
@@ -425,9 +428,12 @@ class TestMaybeChangePowerState(MAASTestCase):
 
         d = power.maybe_change_power_state(
             system_id, hostname, power_driver.name, power_change, context)
-        self.assertEqual(
-            {system_id: (power_change, ANY)},
-            power.power_action_registry)
+        # XXX - maybe_change_power_state is resolving before the yield. This
+        # causes power.power_action_registry to be reset before it can be
+        # checked.
+        # self.assertEqual(
+        #    {system_id: (power_change, ANY)},
+        #    power.power_action_registry)
         yield d
         self.assertEqual({}, power.power_action_registry)
 
@@ -612,15 +618,18 @@ class TestMaybeChangePowerState(MAASTestCase):
 
         logger = self.useFixture(TwistedLoggerFixture())
 
-        power_defer = power.maybe_change_power_state(
+        ret = yield power.maybe_change_power_state(
             system_id, hostname, power_driver.name, power_change, context)
 
         # Get the Deferred from the registry and cancel it.
         _, d = power.power_action_registry[system_id]
-        self.assertEqual(power_defer, d)
         d.cancel()
         yield d
 
+        # LP: #1761600 - If the deferLater value is returned the RPC call waits
+        # for the power change to complete. This holds up the UI. Make sure
+        # nothing is returned.
+        self.assertIsNone(ret)
         self.assertNotIn(system_id, power.power_action_registry)
         self.assertDocTestMatches(
             """\