Merge ~newell-jensen/maas:redfish-commissioning into maas:master

[Newell Jensen]

Only update the power parameters in enlistment if the power type is 'ipmi' or 'manual'. This makes it so other power types do not have their parameters overwritten.

* Allows a user to update an enlisted node to a power type other than 'ipmi' or 'manual' and commission that node without the power type parameters being overwritten.

* Tested in the CI with IPMI and Redfish power type machines.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b redfish-commissioning lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ec8b1d4fa68ab949097d002aea61d0bd1163c02e

review: Approve

[Andres Rodriguez (andreserl)]

If the machine is set to redfish and you commission, the bmc password will be changed but that won’t be reflected In MAAS.

review: Needs Fixing

[Newell Jensen (newell-jensen)]

> If the machine is set to redfish and you commission, the bmc password will be
> changed but that won’t be reflected In MAAS.

Steps performed:

1. Enlisted machine had IPMI power type and parameters.
2. Power type changed to Redfish. To have this work, the username and password for Redfish had to be provided along with the node id (power parameters for Redfish power type).
3. Commissioned the machine. The power type and power parameters were not changed.
4. Deployed the machine.

Are you referring to the BMC password for the IPMI interface? Why would we care about that if the power type is Redfish and we are not saving the IPMI credentials anyways?

[Lee Trager (ltrager)]

Since we don't modify Redfish settings in the ephemeral environment we can leverage the existing skip_bmc_config option. Just modify the CommissionForm to set skip_bmc_config to True whenever the power type is redfish and the BMC autodetect code won't be run on the client.

[Andres Rodriguez (andreserl)]

Needs to be done the other way around where that is totally
Possible to do and use.

On Wed, Jan 23, 2019 at 2:47 PM Lee Trager <email address hidden> wrote:

> Since we don't modify Redfish settings in the ephemeral environment we can
> leverage the existing skip_bmc_config option. Just modify the
> CommissionForm to set skip_bmc_config to True whenever the power type is
> redfish and the BMC autodetect code won't be run on the client.
> --
> https://code.launchpad.net/~newell-jensen/maas/+git/maas/+merge/361754
> You are reviewing the proposed merge of
> ~newell-jensen/maas:redfish-commissioning into maas:master.
>
--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

6dccce4... by Newell Jensen on 2019-01-25

Add redfish power type to parameters that will have their credentials saved.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b redfish-commissioning lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 6dccce4e1a01f8194ae58112f945338dd156ce55

review: Approve

[Andres Rodriguez (andreserl)]

see inline.

review: Needs Information

b6c5321... by Newell Jensen on 2019-01-31

Handle redfish power type in the store_node_power_parameters so it can work properly when the booting needs to resend multiple requests.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b redfish-commissioning lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: b6c5321734fffc45f718933e453f051b3b5bfbb3

review: Approve

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

diff --git a/src/maasserver/api/nodes.py b/src/maasserver/api/nodes.py
index 737f73c..cd29572 100644
--- a/src/maasserver/api/nodes.py
+++ b/src/maasserver/api/nodes.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
@@ -168,7 +168,11 @@ def store_node_power_parameters(node, request):
 
     The parameters should be JSON, passed with key `power_parameters`.
     """
-    power_type = request.POST.get("power_type", None)
+    # Don't overwrite redfish power type with ipmi.
+    if node.power_type == 'redfish':
+        power_type = node.power_type
+    else:
+        power_type = request.POST.get("power_type", None)
     if power_type is None:
         return
 
@@ -185,7 +189,15 @@ def store_node_power_parameters(node, request):
     power_parameters = request.POST.get("power_parameters", None)
     if power_parameters and not power_parameters.isspace():
         try:
-            node.power_parameters = json.loads(power_parameters)
+            power_parameters = json.loads(power_parameters)
+            if power_type == 'redfish':
+                node.power_parameters = {
+                    **node.instance_power_parameters,
+                    **power_parameters
+                }
+            else:
+                node.power_parameters = power_parameters
+
         except ValueError:
             raise MAASAPIBadRequest("Failed to parse JSON power_parameters")
 
diff --git a/src/maasserver/api/tests/test_api.py b/src/maasserver/api/tests/test_api.py
index b36cfc9..c918ff8 100644
--- a/src/maasserver/api/tests/test_api.py
+++ b/src/maasserver/api/tests/test_api.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test maasserver API."""
@@ -180,6 +180,23 @@ class TestStoreNodeParameters(APITestCase.ForUser):
         self.assertEqual({}, self.node.power_parameters)
         self.save.assert_has_calls([])
 
+    def test_power_type_redish(self):
+        power_parameters = {"foo": [1, 2, 3]}
+        self.request.POST = {
+            "power_type": 'ipmi',
+            "power_parameters": json.dumps(power_parameters),
+            }
+        self.node.power_type = 'redfish'
+        self.node.instance_power_parameters = {
+            'node_id': '1',
+            }
+        store_node_power_parameters(self.node, self.request)
+        self.assertEqual('redfish', self.node.power_type)
+        self.assertEqual({
+            **power_parameters,
+            **self.node.instance_power_parameters}, self.node.power_parameters)
+        self.save.assert_has_calls([])
+
     def test_power_type_set_but_no_parameters(self):
         # When power_type is valid, it is set. However, if power_parameters is
         # not specified, the node's power_parameters is left alone, and the