Merge ~newell-jensen/maas:lp1763825 into maas:master

Proposed by Newell Jensen
Status: Merged
Approved by: Newell Jensen
Approved revision: f78a6ede5cdc5fe8d16bcc5147822c417f4a52ca
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~newell-jensen/maas:lp1763825
Merge into: maas:master
Diff against target: 166 lines (+50/-7)
7 files modified
src/maasserver/models/node.py (+4/-0)
src/maasserver/models/tests/test_node.py (+17/-0)
src/maasserver/static/js/angular/controllers/node_details.js (+4/-1)
src/maasserver/static/js/angular/controllers/tests/test_node_details.js (+14/-1)
src/maasserver/static/js/angular/directives/power_parameters.js (+8/-3)
src/maasserver/static/js/angular/directives/tests/test_power_parameters.js (+2/-1)
src/provisioningserver/drivers/pod/virsh.py (+1/-1)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Alberto Donato (community) Approve
Review via email: mp+343481@code.launchpad.net

Commit message

LP: #1763825 -- Remove default_storage_pool from machine's power parameters and disable the ability for a user to change a pod's power parameters via the machine's power configuration.

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: c8cd974329a5a7cd25798ace25f70b7103ff3089

review: Approve
Revision history for this message
Alberto Donato (ack) wrote :

LGTM.

A few nits inline

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

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

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
2index ca283a3..6c7fda1 100644
3--- a/src/maasserver/models/node.py
4+++ b/src/maasserver/models/node.py
5@@ -1182,6 +1182,10 @@ class Node(CleanSave, TimestampedModel):
6 bmc_parameters = {}
7 if self.bmc and self.bmc.power_parameters:
8 bmc_parameters = self.bmc.power_parameters
9+ # Remove default_storage_pool if node is part of a Virsh pod.
10+ if (self.bmc.bmc_type == BMC_TYPE.POD and
11+ self.power_type == 'virsh'):
12+ bmc_parameters.pop('default_storage_pool', None)
13 return {**bmc_parameters, **instance_parameters}
14
15 @power_parameters.setter
16diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
17index 17be5fe..0365c8c 100644
18--- a/src/maasserver/models/tests/test_node.py
19+++ b/src/maasserver/models/tests/test_node.py
20@@ -4503,6 +4503,23 @@ class TestNodePowerParameters(MAASServerTestCase):
21 self.assertEqual(bmc_parameters, node.bmc.power_parameters)
22 self.assertEqual("10.0.2.1", node.bmc.ip_address.ip)
23
24+ def test_default_storage_pool_not_part_of_power_parameters(self):
25+ node = factory.make_Node(power_type='virsh')
26+ default_storage_pool = factory.make_name('default_storage_pool')
27+ bmc_parameters = dict(
28+ power_address="qemu+ssh://trapnine@10.0.2.1/system",
29+ power_pass=factory.make_string(),
30+ default_storage_pool=default_storage_pool,
31+ )
32+ node_parameters = dict(
33+ power_id="maas-x",
34+ )
35+ parameters = {**bmc_parameters, **node_parameters}
36+ node.power_parameters = parameters
37+ node.save()
38+ node = reload_object(node)
39+ self.assertNotIn(default_storage_pool, node.power_parameters)
40+
41 def test_unknown_power_parameter_stored_on_node(self):
42 node = factory.make_Node(power_type='hmc')
43 bmc_parameters = dict(power_address=factory.make_ipv4_address())
44diff --git a/src/maasserver/static/js/angular/controllers/node_details.js b/src/maasserver/static/js/angular/controllers/node_details.js
45index 2838071..268dc26 100644
46--- a/src/maasserver/static/js/angular/controllers/node_details.js
47+++ b/src/maasserver/static/js/angular/controllers/node_details.js
48@@ -100,7 +100,8 @@ angular.module('MAAS').controller('NodeDetailsController', [
49 editing: false,
50 type: null,
51 bmc_node_count: 0,
52- parameters: {}
53+ parameters: {},
54+ in_pod: false
55 };
56
57 // Get the display text for device ip assignment type.
58@@ -215,6 +216,8 @@ angular.module('MAAS').controller('NodeDetailsController', [
59 $scope.node.node_type === 0) {
60 $scope.power.editing = true;
61 }
62+
63+ $scope.power.in_pod = angular.isDefined($scope.node.pod);
64 }
65
66 // Updates the currently selected items in the summary section.
67diff --git a/src/maasserver/static/js/angular/controllers/tests/test_node_details.js b/src/maasserver/static/js/angular/controllers/tests/test_node_details.js
68index 4a2624b..e6cd1f1 100644
69--- a/src/maasserver/static/js/angular/controllers/tests/test_node_details.js
70+++ b/src/maasserver/static/js/angular/controllers/tests/test_node_details.js
71@@ -249,7 +249,8 @@ describe("NodeDetailsController", function() {
72 editing: false,
73 type: null,
74 bmc_node_count: 0,
75- parameters: {}
76+ parameters: {},
77+ in_pod: false
78 });
79 });
80
81@@ -1884,6 +1885,7 @@ describe("NodeDetailsController", function() {
82 $scope.editPower();
83 expect($scope.power.editing).toBe(true);
84 });
85+
86 });
87
88 describe("cancelEditPower", function() {
89@@ -1913,6 +1915,17 @@ describe("NodeDetailsController", function() {
90 $scope.cancelEditPower();
91 expect($scope.power.editing).toBe(false);
92 });
93+
94+ it("sets in_pod to true for node in pod", function() {
95+ var controller = makeController();
96+ node.power_type = makeName("power");
97+ node.pod = makeName("pod");
98+ $scope.node = node;
99+ $scope.power.editing = true;
100+ $scope.cancelEditPower();
101+ expect($scope.power.in_pod).toBe(true);
102+ });
103+
104 });
105
106 describe("saveEditPower", function() {
107diff --git a/src/maasserver/static/js/angular/directives/power_parameters.js b/src/maasserver/static/js/angular/directives/power_parameters.js
108index 01fb2ee..7dfcaaa 100644
109--- a/src/maasserver/static/js/angular/directives/power_parameters.js
110+++ b/src/maasserver/static/js/angular/directives/power_parameters.js
111@@ -12,7 +12,7 @@ angular.module('MAAS').run(['$templateCache', function ($templateCache) {
112 'class="form__group-label col-2">Power type</label>',
113 '<div class="form__group-input col-3">',
114 '<select name="power-type" id="power-type" ',
115- 'data-ng-disabled="ngDisabled" ',
116+ 'data-ng-disabled="ngDisabled || ngModel.in_pod" ',
117 'data-ng-class="{ invalid: !ngModel.type }" ',
118 'data-ng-model="ngModel.type" ',
119 'data-ng-options="',
120@@ -26,12 +26,17 @@ angular.module('MAAS').run(['$templateCache', function ($templateCache) {
121 '</div>',
122 '<div class="p-form__group" ',
123 'data-ng-repeat="field in ngModel.type.fields">',
124- '<label for="{$ field.name $}" class="form__group-label col-2">',
125+ '<label for="{$ field.name $}" class="form__group-label col-2" ',
126+ 'data-ng-if="field.name !== ' + "'default_storage_pool' && ",
127+ "(field.scope !== 'bmc' || !ngModel.in_pod)" + '">',
128 '{$ field.label $}',
129 '</label>',
130 '<div class="form__group-input col-3">',
131 '<maas-power-input field="field" ',
132- 'data-ng-disabled="ngDisabled" ',
133+ 'data-ng-disabled="ngDisabled || (field.scope === ',
134+ "'bmc' && ngModel.in_pod)" + '" ',
135+ 'data-ng-if="field.name !== ' + "'default_storage_pool' ",
136+ "&& (field.scope !== 'bmc' || !ngModel.in_pod)" + '" ',
137 'data-ng-model="ngModel.parameters[field.name]">',
138 '</div>',
139 '</div>'
140diff --git a/src/maasserver/static/js/angular/directives/tests/test_power_parameters.js b/src/maasserver/static/js/angular/directives/tests/test_power_parameters.js
141index 82d3cda..1836c90 100644
142--- a/src/maasserver/static/js/angular/directives/tests/test_power_parameters.js
143+++ b/src/maasserver/static/js/angular/directives/tests/test_power_parameters.js
144@@ -198,7 +198,8 @@ describe("maasPowerParameters", function() {
145 expect(select.attr("data-ng-options")).toBe(
146 "type as type.description for type " +
147 "in maasPowerParameters track by type.name");
148- expect(select.attr("data-ng-disabled")).toBe("ngDisabled");
149+ expect(select.attr("data-ng-disabled")).toBe(
150+ "ngDisabled || ngModel.in_pod");
151 });
152
153 it("creates option with description", function() {
154diff --git a/src/provisioningserver/drivers/pod/virsh.py b/src/provisioningserver/drivers/pod/virsh.py
155index 537de91..1efc669 100644
156--- a/src/provisioningserver/drivers/pod/virsh.py
157+++ b/src/provisioningserver/drivers/pod/virsh.py
158@@ -839,7 +839,7 @@ class VirshPodDriver(PodDriver):
159 required=True),
160 make_setting_field(
161 'default_storage_pool', "Default storage pool (optional)",
162- required=False),
163+ scope=SETTING_SCOPE.BMC, required=False),
164 ]
165 default_storage_pool = None
166 ip_extractor = make_ip_extractor(

Subscribers

People subscribed via source and target branches