Merge ~bjornt/maas:bug-2056740-3.5 into maas:3.5

[Björn Tillenius]

Bug #2056740: Can't commission/deploy AMT machines

The maas-power script was passed "--boot-mode pxe" for AMT machines
and failed, since such a parameter wasn't defined.

All other power drivers, including the newer AMT one, sets PXE boot every
time the machine is powered on. I changed things, so that "boot-mode" is
no longer passed, and the older AMT driver also sets the machine to PXE
boot whenever it's turned on.

(cherry picked from commit 116ccedb102a78136d9e1d62d3144850e861947e)

[Björn Tillenius (bjornt)]

Self-approve backport.

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bug-2056740-3.5 lp:~bjornt/maas/+git/maas into -b 3.5 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 3469f13b0842b909f1099b2a97d6394372aef645

review: Approve

diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index e34f5d4..e9a3ac2 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -3134,17 +3134,6 @@ class Node(CleanSave, TimestampedModel):
         if self.bmc is not None and self.bmc.power_type == "ipmi":
             power_params.setdefault("power_off_mode", "")
 
-        # boot_mode is something that tells the template whether this is
-        # a PXE boot or a local HD boot.
-        if self.bmc is not None and self.bmc.power_type == "amt":
-            if (
-                self.status == NODE_STATUS.DEPLOYED
-                or self.node_type != NODE_TYPE.MACHINE
-            ):
-                power_params["boot_mode"] = "local"
-            else:
-                power_params["boot_mode"] = "pxe"
-
         return power_params
 
     def get_effective_power_info(self):
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 6195c28..5802bf5 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -1579,17 +1579,6 @@ class TestNode(MAASServerTestCase):
         params = node.get_effective_power_parameters()
         self.assertEqual("qemu://localhost/system", params["power_address"])
 
-    def test_get_effective_power_parameters_sets_local_boot_mode(self):
-        node = factory.make_Node(status=NODE_STATUS.DEPLOYED, power_type="amt")
-        params = node.get_effective_power_parameters()
-        self.assertEqual("local", params["boot_mode"])
-
-    def test_get_effective_power_parameters_sets_pxe_boot_mode(self):
-        status = factory.pick_enum(NODE_STATUS, but_not=[NODE_STATUS.DEPLOYED])
-        node = factory.make_Node(status=status, power_type="amt")
-        params = node.get_effective_power_parameters()
-        self.assertEqual("pxe", params["boot_mode"])
-
     def test_get_effective_power_info_is_False_for_unset_power_type(self):
         node = factory.make_Node(power_type=None)
         self.assertEqual(
diff --git a/src/provisioningserver/drivers/power/amt.py b/src/provisioningserver/drivers/power/amt.py
index 63ecb29..8c7279b 100644
--- a/src/provisioningserver/drivers/power/amt.py
+++ b/src/provisioningserver/drivers/power/amt.py
@@ -178,13 +178,12 @@ class AMTPowerDriver(PowerDriver):
         cmd: str,
         ip_address: str,
         power_pass: str,
-        amttool_boot_mode=None,
         stdin=None,
     ) -> bytes:
         """Perform a command using amttool."""
         command = ("amttool", ip_address, cmd)
         if cmd in ("power-cycle", "powerup"):
-            command += (amttool_boot_mode,)
+            command += ("pxe",)
         return self._run(command, power_pass, stdin=stdin)
 
     def _issue_wsman_command(
@@ -304,17 +303,16 @@ class AMTPowerDriver(PowerDriver):
                     "Got unknown power state from node: %s" % state
                 )
 
-    def amttool_restart(self, ip_address, power_pass, amttool_boot_mode):
+    def amttool_restart(self, ip_address, power_pass):
         """Restart the node via amttool."""
         self._issue_amttool_command(
             "power_cycle",
             ip_address,
             power_pass,
-            amttool_boot_mode=amttool_boot_mode,
             stdin=b"yes",
         )
 
-    def amttool_power_on(self, ip_address, power_pass, amttool_boot_mode):
+    def amttool_power_on(self, ip_address, power_pass):
         """Power on the node via amttool."""
         # Try several times.  Power commands often fail the first time.
         for _ in range(10):
@@ -323,7 +321,6 @@ class AMTPowerDriver(PowerDriver):
                 "powerup",
                 ip_address,
                 power_pass,
-                amttool_boot_mode=amttool_boot_mode,
                 stdin=b"yes",
             )
             if self.amttool_query_state(ip_address, power_pass) == "on":
@@ -413,16 +410,6 @@ class AMTPowerDriver(PowerDriver):
                 else:
                     return "amttool"
 
-    def _get_amttool_boot_mode(self, boot_mode):
-        """Set amttool boot mode."""
-        # boot_mode tells us whether we're pxe booting or local booting.
-        # For local booting, the argument to amttool must be empty
-        # (NOT 'hd', it doesn't work!).
-        if boot_mode == "local":
-            return ""
-        else:
-            return boot_mode
-
     def _get_wsman_command(self, *args):
         base_path = snap.SnapPaths.from_environ().snap or "/"
         return (
@@ -455,15 +442,10 @@ class AMTPowerDriver(PowerDriver):
         power_pass = context.get("power_pass")
         amt_command = self._get_amt_command(ip_address, power_pass)
         if amt_command == "amttool":
-            amttool_boot_mode = self._get_amttool_boot_mode(
-                context.get("boot_mode")
-            )
             if self.amttool_query_state(ip_address, power_pass) == "on":
-                self.amttool_restart(ip_address, power_pass, amttool_boot_mode)
+                self.amttool_restart(ip_address, power_pass)
             else:
-                self.amttool_power_on(
-                    ip_address, power_pass, amttool_boot_mode
-                )
+                self.amttool_power_on(ip_address, power_pass)
         elif amt_command == "wsman":
             if self.wsman_query_state(ip_address, power_pass) == "on":
                 self.wsman_power_on(ip_address, power_pass, restart=True)
diff --git a/src/provisioningserver/drivers/power/tests/test_amt.py b/src/provisioningserver/drivers/power/tests/test_amt.py
index db207d8..4ab4bae 100644
--- a/src/provisioningserver/drivers/power/tests/test_amt.py
+++ b/src/provisioningserver/drivers/power/tests/test_amt.py
@@ -75,7 +75,6 @@ def make_context():
         "power_address": factory.make_name("power_address"),
         "ip_address": factory.make_ipv4_address(),
         "power_pass": factory.make_name("power_pass"),
-        "boot_mode": factory.make_name("boot_mode"),
     }
 
 
@@ -245,10 +244,9 @@ class TestAMTPowerDriver(MAASTestCase):
         amt_power_driver = AMTPowerDriver()
         ip_address = factory.make_ipv4_address()
         power_pass = factory.make_name("power_pass")
-        amttool_boot_mode = factory.make_name("amttool_boot_mode")
         stdin = factory.make_name("stdin").encode("utf-8")
         cmd = choice(["power-cycle", "powerup"])
-        command = "amttool", ip_address, cmd, amttool_boot_mode
+        command = "amttool", ip_address, cmd, "pxe"
         _run_mock = self.patch(amt_power_driver, "_run")
         _run_mock.return_value = b"output"
 
@@ -256,7 +254,6 @@ class TestAMTPowerDriver(MAASTestCase):
             cmd,
             ip_address,
             power_pass,
-            amttool_boot_mode=amttool_boot_mode,
             stdin=stdin,
         )
 
@@ -440,20 +437,16 @@ class TestAMTPowerDriver(MAASTestCase):
         amt_power_driver = AMTPowerDriver()
         ip_address = factory.make_ipv4_address()
         power_pass = factory.make_name("power_pass")
-        amttool_boot_mode = factory.make_name("amttool_boot_mode")
         _issue_amttool_command_mock = self.patch(
             amt_power_driver, "_issue_amttool_command"
         )
 
-        amt_power_driver.amttool_restart(
-            ip_address, power_pass, amttool_boot_mode
-        )
+        amt_power_driver.amttool_restart(ip_address, power_pass)
 
         _issue_amttool_command_mock.assert_called_once_with(
             "power_cycle",
             ip_address,
             power_pass,
-            amttool_boot_mode=amttool_boot_mode,
             stdin=b"yes",
         )
 
@@ -461,7 +454,6 @@ class TestAMTPowerDriver(MAASTestCase):
         amt_power_driver = AMTPowerDriver()
         ip_address = factory.make_ipv4_address()
         power_pass = factory.make_name("power_pass")
-        amttool_boot_mode = factory.make_name("amttool_boot_mode")
         _issue_amttool_command_mock = self.patch(
             amt_power_driver, "_issue_amttool_command"
         )
@@ -470,15 +462,12 @@ class TestAMTPowerDriver(MAASTestCase):
         )
         amttool_query_state_mock.return_value = "on"
 
-        amt_power_driver.amttool_power_on(
-            ip_address, power_pass, amttool_boot_mode
-        )
+        amt_power_driver.amttool_power_on(ip_address, power_pass)
 
         _issue_amttool_command_mock.assert_called_once_with(
             "powerup",
             ip_address,
             power_pass,
-            amttool_boot_mode=amttool_boot_mode,
             stdin=b"yes",
         )
         amttool_query_state_mock.assert_called_once_with(
@@ -489,7 +478,6 @@ class TestAMTPowerDriver(MAASTestCase):
         amt_power_driver = AMTPowerDriver()
         ip_address = factory.make_ipv4_address()
         power_pass = factory.make_name("power_pass")
-        amttool_boot_mode = factory.make_name("amttool_boot_mode")
         self.patch(amt_power_driver, "_issue_amttool_command")
         amttool_query_state_mock = self.patch(
             amt_power_driver, "amttool_query_state"
@@ -501,7 +489,6 @@ class TestAMTPowerDriver(MAASTestCase):
             amt_power_driver.amttool_power_on,
             ip_address,
             power_pass,
-            amttool_boot_mode,
         )
 
     def test_amttool_power_off_powers_off(self):
@@ -787,17 +774,6 @@ class TestAMTPowerDriver(MAASTestCase):
                 factory.make_name("power_pass"),
             )
 
-    def test_get_amttool_boot_mode_local_boot(self):
-        amt_power_driver = AMTPowerDriver()
-        result = amt_power_driver._get_amttool_boot_mode("local")
-        self.assertEqual(result, "")
-
-    def test_get_ammtool_boot_mode_pxe_booting(self):
-        amt_power_driver = AMTPowerDriver()
-        boot_mode = factory.make_name("boot_mode")
-        result = amt_power_driver._get_amttool_boot_mode(boot_mode)
-        self.assertEqual(result, boot_mode)
-
     def test_get_ip_address_returns_ip_address(self):
         amt_power_driver = AMTPowerDriver()
         power_address = factory.make_name("power_address")
@@ -841,7 +817,6 @@ class TestAMTPowerDriver(MAASTestCase):
         amttool_restart_mock.assert_called_once_with(
             context["ip_address"],
             context["power_pass"],
-            context["boot_mode"],
         )
 
     def test_power_on_powers_on_with_amttool_when_already_off(self):
@@ -870,7 +845,6 @@ class TestAMTPowerDriver(MAASTestCase):
         amttool_power_on_mock.assert_called_once_with(
             context["ip_address"],
             context["power_pass"],
-            context["boot_mode"],
         )
 
     def test_power_on_powers_on_with_wsman_when_already_on(self):