Merge ~ltrager/maas:hmcz_transitional_states into maas:master

[Lee Trager]

HMCZ - Wait for transitional states

IBM Z has transitional states starting and stopping. When these operations
are occurring the HMC will reject any call to change states or properties.
If detected wait for the transition to complete before making a call to
the HMC.

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 1fe22b9f21ecd0fa38ec5832a4bf0f31aab30f87

review: Approve

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 0fb0f0eaefb085fc8e19037daf24c390c66282e9

review: Approve

[Adam Collard (adam-collard)]

basically looks good - some nit picks

review: Approve

c27b242... by Lee Trager on 2021-03-29

Merge branch 'master' into hmcz_transitional_states

a3f4186... by Lee Trager on 2021-03-29

adam-collard fixes

[Lee Trager (ltrager)]

Thanks for the review. Updated as suggested!

diff --git a/src/provisioningserver/drivers/power/hmcz.py b/src/provisioningserver/drivers/power/hmcz.py
index 5ccef9a..a4d7a56 100644
--- a/src/provisioningserver/drivers/power/hmcz.py
+++ b/src/provisioningserver/drivers/power/hmcz.py
@@ -18,14 +18,18 @@ from provisioningserver.drivers import (
     make_setting_field,
     SETTING_SCOPE,
 )
-from provisioningserver.drivers.power import PowerActionError, PowerDriver
+from provisioningserver.drivers.power import (
+    PowerActionError,
+    PowerDriver,
+    PowerError,
+)
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.rpc.utils import commission_node, create_node
 from provisioningserver.utils import typed
 from provisioningserver.utils.twisted import asynchronous, threadDeferred
 
 try:
-    from zhmcclient import Client, NotFound, Session
+    from zhmcclient import Client, NotFound, Session, StatusTimeout
 except ImportError:
     no_zhmcclient = True
 else:
@@ -101,6 +105,21 @@ class HMCZPowerDriver(PowerDriver):
             # IBM is aware this isn't optimal for us so they are looking into
             # modifying IBM Z to go into a stopped state.
             partition.stop(wait_for_completion=True)
+        elif status == "stopping":
+            # The HMC does not allow a machine to be powered on if its
+            # currently stopping. Wait 120s for it which should be more
+            # than enough time.
+            try:
+                partition.wait_for_status("stopped", 120)
+            except StatusTimeout:
+                # If 120s isn't enough time raise a PowerError() which will
+                # trigger the builtin retry code in the base PowerDriver()
+                # class.
+                raise PowerError(
+                    "Partition is stuck in a "
+                    f"{partition.get_property('status')} state!"
+                )
+
         partition.start(wait_for_completion=False)
 
     @typed
@@ -109,6 +128,21 @@ class HMCZPowerDriver(PowerDriver):
     def power_off(self, system_id: str, context: dict):
         """Power off IBM Z DPM."""
         partition = self._get_partition(context)
+        status = partition.get_property("status")
+        if status == "starting":
+            # The HMC does not allow a machine to be powered off if its
+            # currently starting. Wait 120s for it which should be more
+            # than enough time.
+            try:
+                partition.wait_for_status("active", 120)
+            except StatusTimeout:
+                # If 120s isn't enough time raise a PowerError() which will
+                # trigger the builtin retry code in the base PowerDriver()
+                # class.
+                raise PowerError(
+                    "Partition is stuck in a "
+                    f"{partition.get_property('status')} state!"
+                )
         partition.stop(wait_for_completion=False)
 
     @typed
@@ -147,6 +181,14 @@ class HMCZPowerDriver(PowerDriver):
         :param order: An ordered list of network or storage devices.
         """
         partition = self._get_partition(context)
+        status = partition.get_property("status")
+
+        if status in {"starting", "stopping"}:
+            # The HMC does not allow a machine's boot order to be reconfigured
+            # while in a transitional state. Wait for it to complete. If this
+            # times out allow it to be raised so the region can log it.
+            partition.wait_for_status(["stopped", "active"], 120)
+
         # You can only specify one boot device on IBM Z
         boot_device = order[0]
         if boot_device.get("mac_address"):
diff --git a/src/provisioningserver/drivers/power/tests/test_hmcz.py b/src/provisioningserver/drivers/power/tests/test_hmcz.py
index 20d4f76..d76b347 100644
--- a/src/provisioningserver/drivers/power/tests/test_hmcz.py
+++ b/src/provisioningserver/drivers/power/tests/test_hmcz.py
@@ -9,7 +9,9 @@ import random
 import typing
 from unittest.mock import Mock
 
+import pytest
 from twisted.internet.defer import inlineCallbacks, returnValue, succeed
+from zhmcclient import StatusTimeout
 from zhmcclient_mock import FakedSession
 
 from maastesting.factory import factory
@@ -20,7 +22,7 @@ from maastesting.matchers import (
 )
 from maastesting.testcase import MAASTestCase, MAASTwistedRunTest
 from provisioningserver.drivers.power import hmcz as hmcz_module
-from provisioningserver.drivers.power import PowerActionError
+from provisioningserver.drivers.power import PowerActionError, PowerError
 from provisioningserver.rpc import clusterservice, region
 from provisioningserver.rpc.testing import MockLiveClusterToRegionRPCFixture
 
@@ -160,6 +162,28 @@ class TestHMCZPowerDriver(MAASTestCase):
         )
 
     @inlineCallbacks
+    def test_power_on_waits_for_stopping_state(self):
+        mock_get_partition = self.patch(self.hmcz, "_get_partition")
+        mock_get_partition.return_value.get_property.return_value = "stopping"
+        yield self.hmcz.power_on(None, self.make_context())
+        mock_get_partition.return_value.wait_for_status.assert_called_once_with(
+            "stopped", 120
+        )
+        mock_get_partition.return_value.start.assert_called_once_with(
+            wait_for_completion=False
+        )
+
+    @inlineCallbacks
+    def test_power_on_times_out_waiting_for_stopping_state(self):
+        mock_get_partition = self.patch(self.hmcz, "_get_partition")
+        mock_get_partition.return_value.get_property.return_value = "stopping"
+        mock_get_partition.return_value.wait_for_status.side_effect = (
+            StatusTimeout(None, None, None, None)
+        )
+        with pytest.raises(PowerError):
+            yield self.hmcz.power_on(None, self.make_context())
+
+    @inlineCallbacks
     def test_power_off(self):
         mock_get_partition = self.patch(self.hmcz, "_get_partition")
         yield self.hmcz.power_off(None, self.make_context())
@@ -169,6 +193,28 @@ class TestHMCZPowerDriver(MAASTestCase):
         )
 
     @inlineCallbacks
+    def test_power_off_waits_for_starting_state(self):
+        mock_get_partition = self.patch(self.hmcz, "_get_partition")
+        mock_get_partition.return_value.get_property.return_value = "starting"
+        yield self.hmcz.power_off(None, self.make_context())
+        mock_get_partition.return_value.wait_for_status.assert_called_once_with(
+            "active", 120
+        )
+        mock_get_partition.return_value.stop.assert_called_once_with(
+            wait_for_completion=False
+        )
+
+    @inlineCallbacks
+    def test_power_off_timesout_waiting_for_stopping_state(self):
+        mock_get_partition = self.patch(self.hmcz, "_get_partition")
+        mock_get_partition.return_value.get_property.return_value = "starting"
+        mock_get_partition.return_value.wait_for_status.side_effect = (
+            StatusTimeout(None, None, None, None)
+        )
+        with pytest.raises(PowerError):
+            yield self.hmcz.power_off(None, self.make_context())
+
+    @inlineCallbacks
     def test_power_query_starting(self):
         power_partition_name = factory.make_name("power_partition_name")
         cpc = self.fake_session.hmc.cpcs.add(
@@ -430,6 +476,40 @@ class TestHMCZPowerDriver(MAASTestCase):
             }
         )
 
+    @inlineCallbacks
+    def test_set_boot_order_waits_for_starting_state(self):
+        # Mock must be used as partition.wait_for_status() hangs when not
+        # connected to a real HMC
+        mock_get_partition = self.patch(self.hmcz, "_get_partition")
+        mock_get_partition.return_value.get_property.return_value = "starting"
+
+        yield self.hmcz.set_boot_order(
+            None,
+            self.make_context(),
+            [{"mac_address": factory.make_mac_address()}],
+        )
+
+        mock_get_partition.return_value.wait_for_status.assert_called_once_with(
+            ["stopped", "active"], 120
+        )
+
+    @inlineCallbacks
+    def test_set_boot_order_waits_for_stopping_state(self):
+        # Mock must be used as partition.wait_for_status() hangs when not
+        # connected to a real HMC
+        mock_get_partition = self.patch(self.hmcz, "_get_partition")
+        mock_get_partition.return_value.get_property.return_value = "stopping"
+
+        yield self.hmcz.set_boot_order(
+            None,
+            self.make_context(),
+            [{"mac_address": factory.make_mac_address()}],
+        )
+
+        mock_get_partition.return_value.wait_for_status.assert_called_once_with(
+            ["stopped", "active"], 120
+        )
+
 
 @dataclass
 class FakeNode: