MAAS

Merge ~bjornt/maas:api-acquire-from-pool into maas:master

  1. Git
  2. lp:~bjornt/maas
  3. api-acquire-from-pool
  4. Merge into master
Proposed by Björn Tillenius
Status: Merged
Approved by: Björn Tillenius
Approved revision: af1878456285a8479b161bb91a9652b25272d4c5
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~bjornt/maas:api-acquire-from-pool
Merge into: maas:master
Diff against target: 415 lines (+214/-26)
5 files modified
src/maasserver/api/machines.py (+11/-1)
src/maasserver/api/tests/test_machines.py (+51/-0)
src/maasserver/node_constraint_filter_forms.py (+57/-10)
src/maasserver/testing/factory.py (+9/-1)
src/maasserver/tests/test_node_constraint_filter_forms.py (+86/-14)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Alberto Donato (community) Approve
Review via email: mp+335994@code.launchpad.net

Commit message

Add a pool constraint to the allocate API command.

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

UNIT TESTS
-b api-acquire-from-pool lp:~bjornt/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/1225/console
COMMIT: 463f5d9b7afcdd8c269855a9346328232fbb99b4

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

+1, nice.

Just a few nits inline

review: Approve
Revision history for this message
Björn Tillenius (bjornt) :
Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b api-acquire-from-pool lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/1236/consoleText

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

UNIT TESTS
-b api-acquire-from-pool lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

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

review: Needs Fixing
~bjornt/maas:api-acquire-from-pool updated
ce9b85e... by Björn Tillenius

Merge remote-tracking branch 'upstream/master' into api-acquire-from-pool

af18784... by Björn Tillenius

Fix test failures.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
2index 302c6c1..eb25a51 100644
3--- a/src/maasserver/api/machines.py
4+++ b/src/maasserver/api/machines.py
5@@ -1406,7 +1406,7 @@ class MachinesHandler(NodesHandler, PowersMixin):
6 :type tags: unicode (accepts multiple)
7 :param zone: Physical zone name the machine must be located in.
8 :type zone: unicode
9- :type not_in_zone: List of physical zones from which the machine must
10+ :param not_in_zone: List of physical zones from which the machine must
11 not be acquired.
12
13 If multiple zones are specified, the machine must NOT be
14@@ -1414,6 +1414,16 @@ class MachinesHandler(NodesHandler, PowersMixin):
15 exclude, this parameter must be repeated in the request with each
16 value.
17 :type not_in_zone: unicode (accepts multiple)
18+ :param pool: Resource pool name the machine must belong to.
19+ :type pool: unicode
20+ :param not_in_pool: List of resource pool from which the machine must
21+ not be acquired.
22+
23+ If multiple pools are specified, the machine must NOT be
24+ associated with ANY of them. To request multiple pools to
25+ exclude, this parameter must be repeated in the request with each
26+ value.
27+ :type not_in_pool: unicode (accepts multiple)
28 :param pod: Pod the machine must be located in.
29 :type pod: unicode
30 :param pod_type: Pod type the machine must be located in.
31diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
32index 4704aca..2241cd0 100644
33--- a/src/maasserver/api/tests/test_machines.py
34+++ b/src/maasserver/api/tests/test_machines.py
35@@ -1133,6 +1133,37 @@ class TestMachinesAPI(APITestCase.ForUser):
36 })
37 self.assertEqual(http.client.BAD_REQUEST, response.status_code)
38
39+ def test_POST_allocate_allocates_machine_by_pool(self):
40+ node1 = factory.make_Node(
41+ status=NODE_STATUS.READY, with_boot_disk=True)
42+ factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
43+ pool = factory.make_ResourcePool(nodes=[node1], users=[self.user])
44+ response = self.client.post(reverse('machines_handler'), {
45+ 'op': 'allocate',
46+ 'pool': pool.name,
47+ })
48+ self.assertThat(response, HasStatusCode(http.client.OK))
49+ response_json = json.loads(
50+ response.content.decode(settings.DEFAULT_CHARSET))
51+ self.assertEqual(node1.system_id, response_json['system_id'])
52+
53+ def test_POST_allocate_allocates_machine_by_pool_fails_if_no_machine(self):
54+ factory.make_Node(
55+ status=NODE_STATUS.READY, with_boot_disk=True)
56+ pool = factory.make_ResourcePool(users=[self.user])
57+ response = self.client.post(reverse('machines_handler'), {
58+ 'op': 'allocate',
59+ 'pool': pool.name,
60+ })
61+ self.assertThat(response, HasStatusCode(http.client.CONFLICT))
62+
63+ def test_POST_allocate_rejects_unknown_pool(self):
64+ response = self.client.post(reverse('machines_handler'), {
65+ 'op': 'allocate',
66+ 'pool': factory.make_name('pool'),
67+ })
68+ self.assertEqual(http.client.BAD_REQUEST, response.status_code)
69+
70 def test_POST_allocate_allocates_machine_by_tags_comma_separated(self):
71 machine = factory.make_Node(
72 status=NODE_STATUS.READY, with_boot_disk=True)
73@@ -1450,6 +1481,26 @@ class TestMachinesAPI(APITestCase.ForUser):
74 response.content.decode(settings.DEFAULT_CHARSET))['system_id']
75 self.assertEqual(eligible_machine.system_id, system_id)
76
77+ def test_POST_allocate_obeys_not_in_pool(self):
78+ # Pool we don't want to acquire from.
79+ node1 = factory.make_Node(
80+ status=NODE_STATUS.READY, with_boot_disk=True)
81+ node2 = factory.make_Node(
82+ status=NODE_STATUS.READY, with_boot_disk=True)
83+ pool1 = factory.make_ResourcePool(nodes=[node1], users=[self.user])
84+ factory.make_ResourcePool(nodes=[node2], users=[self.user])
85+
86+ response = self.client.post(
87+ reverse('machines_handler'),
88+ {
89+ 'op': 'allocate',
90+ 'not_in_pool': [pool1.name],
91+ })
92+ self.assertEqual(http.client.OK, response.status_code)
93+ system_id = json.loads(
94+ response.content.decode(settings.DEFAULT_CHARSET))['system_id']
95+ self.assertEqual(node2.system_id, system_id)
96+
97 def test_POST_allocate_sets_a_token(self):
98 # "acquire" should set the Token being used in the request on
99 # the Machine that is allocated.
100diff --git a/src/maasserver/node_constraint_filter_forms.py b/src/maasserver/node_constraint_filter_forms.py
101index 3c705b9..3a3f229 100644
102--- a/src/maasserver/node_constraint_filter_forms.py
103+++ b/src/maasserver/node_constraint_filter_forms.py
104@@ -33,6 +33,7 @@ from maasserver.models import (
105 Filesystem,
106 Interface,
107 Pod,
108+ ResourcePool,
109 Subnet,
110 Tag,
111 VLAN,
112@@ -229,18 +230,20 @@ class RenamableFieldsForm(forms.Form):
113 setattr(self, "clean_%s" % new_name, method)
114
115
116-def detect_nonexistent_zone_names(names):
117- """Check for, and return, names of nonexistent physical zones.
118+def detect_nonexistent_names(model_class, names):
119+ """Check for, and return, names of nonexistent objects.
120
121- Used for checking zone names as passed to the `AcquireNodeForm`.
122+ Used for checking object names as passed to the `AcquireNodeForm`.
123
124+ :param model_class: A model class that has a name attribute.
125 :param names: List, tuple, or set of purpoprted zone names.
126 :return: A sorted list of those names that did not name existing zones.
127 """
128 assert isinstance(names, (list, tuple, set))
129 if len(names) == 0:
130 return []
131- existing_names = set(Zone.objects.all().values_list('name', flat=True))
132+ existing_names = set(
133+ model_class.objects.all().values_list('name', flat=True))
134 return sorted(set(names) - existing_names)
135
136
137@@ -579,6 +582,15 @@ class AcquireNodeForm(RenamableFieldsForm):
138 'invalid_list': "Invalid parameter: must list physical zones.",
139 })
140
141+ pool = forms.CharField(label="Resource pool", required=False)
142+
143+ not_in_pool = ValidatorMultipleChoiceField(
144+ validator=MODEL_NAME_VALIDATOR,
145+ label="Not in resource pool", required=False,
146+ error_messages={
147+ 'invalid_list': "Invalid parameter: must list resource pools.",
148+ })
149+
150 storage = forms.CharField(
151 validators=[storage_validator], label="Storage", required=False)
152
153@@ -646,8 +658,8 @@ class AcquireNodeForm(RenamableFieldsForm):
154 def clean_zone(self):
155 value = self.cleaned_data[self.get_field_name('zone')]
156 if value:
157- nonexistent_names = detect_nonexistent_zone_names([value])
158- if len(nonexistent_names) > 0:
159+ nonexistent_names = detect_nonexistent_names(Zone, [value])
160+ if nonexistent_names:
161 error_msg = "No such zone: '%s'." % value
162 set_form_error(self, self.get_field_name('zone'), error_msg)
163 return None
164@@ -656,15 +668,37 @@ class AcquireNodeForm(RenamableFieldsForm):
165
166 def clean_not_in_zone(self):
167 value = self.cleaned_data[self.get_field_name('not_in_zone')]
168- if value is None or len(value) == 0:
169+ if not value:
170 return None
171- nonexistent_names = detect_nonexistent_zone_names(value)
172- if len(nonexistent_names) > 0:
173+ nonexistent_names = detect_nonexistent_names(Zone, value)
174+ if nonexistent_names:
175 error_msg = "No such zone(s): %s." % ', '.join(nonexistent_names)
176 set_form_error(self, self.get_field_name('not_in_zone'), error_msg)
177 return None
178 return value
179
180+ def clean_pool(self):
181+ value = self.cleaned_data[self.get_field_name('pool')]
182+ if value:
183+ nonexistent_names = detect_nonexistent_names(ResourcePool, [value])
184+ if nonexistent_names:
185+ error_msg = "No such pool: '%s'." % value
186+ set_form_error(self, self.get_field_name('pool'), error_msg)
187+ return None
188+ return value
189+ return None
190+
191+ def clean_not_in_pool(self):
192+ value = self.cleaned_data[self.get_field_name('not_in_pool')]
193+ if not value:
194+ return None
195+ nonexistent_names = detect_nonexistent_names(ResourcePool, value)
196+ if nonexistent_names:
197+ error_msg = "No such pool(s): %s." % ', '.join(nonexistent_names)
198+ set_form_error(self, self.get_field_name('not_in_pool'), error_msg)
199+ return None
200+ return value
201+
202 def _clean_specifiers(self, model, specifiers):
203 if not specifiers:
204 return None
205@@ -762,6 +796,7 @@ class AcquireNodeForm(RenamableFieldsForm):
206 filtered_nodes = self.filter_by_mem(filtered_nodes)
207 filtered_nodes = self.filter_by_tags(filtered_nodes)
208 filtered_nodes = self.filter_by_zone(filtered_nodes)
209+ filtered_nodes = self.filter_by_pool(filtered_nodes)
210 filtered_nodes = self.filter_by_subnets(filtered_nodes)
211 filtered_nodes = self.filter_by_vlans(filtered_nodes)
212 filtered_nodes = self.filter_by_fabrics(filtered_nodes)
213@@ -868,11 +903,23 @@ class AcquireNodeForm(RenamableFieldsForm):
214 zone_obj = Zone.objects.get(name=zone)
215 filtered_nodes = filtered_nodes.filter(zone=zone_obj)
216 not_in_zone = self.cleaned_data.get(self.get_field_name('not_in_zone'))
217- if not_in_zone is not None and len(not_in_zone) > 0:
218+ if not_in_zone:
219 not_in_zones = Zone.objects.filter(name__in=not_in_zone)
220 filtered_nodes = filtered_nodes.exclude(zone__in=not_in_zones)
221 return filtered_nodes
222
223+ def filter_by_pool(self, filtered_nodes):
224+ pool_name = self.cleaned_data.get(self.get_field_name('pool'))
225+ if pool_name:
226+ pool = ResourcePool.objects.get(name=pool_name)
227+ filtered_nodes = filtered_nodes.filter(pool=pool)
228+ not_in_pool = self.cleaned_data.get(self.get_field_name('not_in_pool'))
229+ if not_in_pool:
230+ pools_to_exclude = ResourcePool.objects.filter(
231+ name__in=not_in_pool)
232+ filtered_nodes = filtered_nodes.exclude(pool__in=pools_to_exclude)
233+ return filtered_nodes
234+
235 def filter_by_tags(self, filtered_nodes):
236 tags = self.cleaned_data.get(self.get_field_name('tags'))
237 if tags:
238diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py
239index 953f585..e29cbe5 100644
240--- a/src/maasserver/testing/factory.py
241+++ b/src/maasserver/testing/factory.py
242@@ -1015,13 +1015,21 @@ class Factory(maastesting.factory.Factory):
243 user.userprofile.save()
244 return user
245
246- def make_ResourcePool(self, name=None, description=None):
247+ def make_ResourcePool(self, name=None, description=None,
248+ nodes=None, users=None):
249 if name is None:
250 name = self.make_name('resourcepool')
251 if description is None:
252 description = self.make_string()
253 pool = ResourcePool(name=name, description=description)
254 pool.save()
255+ if nodes is not None:
256+ for node in nodes:
257+ node.pool = pool
258+ node.save()
259+ if users is not None:
260+ for user in users:
261+ pool.grant_user(user)
262 return pool
263
264 def make_Role(self, name=None, description=None):
265diff --git a/src/maasserver/tests/test_node_constraint_filter_forms.py b/src/maasserver/tests/test_node_constraint_filter_forms.py
266index dbc4093..8fc328f 100644
267--- a/src/maasserver/tests/test_node_constraint_filter_forms.py
268+++ b/src/maasserver/tests/test_node_constraint_filter_forms.py
269@@ -19,10 +19,11 @@ from maasserver.enum import (
270 from maasserver.models import (
271 Domain,
272 Machine,
273+ Zone,
274 )
275 from maasserver.node_constraint_filter_forms import (
276 AcquireNodeForm,
277- detect_nonexistent_zone_names,
278+ detect_nonexistent_names,
279 generate_architecture_wildcards,
280 get_architecture_wildcards,
281 get_storage_constraints_from_string,
282@@ -109,35 +110,36 @@ class TestUtils(MAASServerTestCase):
283 list(AcquireNodeForm().fields),
284 ContainsAll(JUJU_ACQUIRE_FORM_FIELDS_MAPPING))
285
286- def test_detect_nonexistent_zone_names_returns_empty_if_no_names(self):
287- self.assertEqual([], detect_nonexistent_zone_names([]))
288+ def test_detect_nonexistent_names_returns_empty_if_no_names(self):
289+ self.assertEqual([], detect_nonexistent_names(Zone, []))
290
291- def test_detect_nonexistent_zone_names_returns_empty_if_all_OK(self):
292+ def test_detect_nonexistent_names_returns_empty_if_all_OK(self):
293 zones = [factory.make_Zone() for _ in range(3)]
294 self.assertEqual(
295 [],
296- detect_nonexistent_zone_names([zone.name for zone in zones]))
297+ detect_nonexistent_names(Zone, [zone.name for zone in zones]))
298
299- def test_detect_nonexistent_zone_names_reports_unknown_zone_names(self):
300+ def test_detect_nonexistent_names_reports_unknown_names(self):
301 non_zone = factory.make_name('nonzone')
302- self.assertEqual([non_zone], detect_nonexistent_zone_names([non_zone]))
303+ self.assertEqual(
304+ [non_zone], detect_nonexistent_names(Zone, [non_zone]))
305
306- def test_detect_nonexistent_zone_names_is_consistent(self):
307+ def test_detect_nonexistent_names_is_consistent(self):
308 names = [factory.make_name('nonzone') for _ in range(3)]
309 self.assertEqual(
310- detect_nonexistent_zone_names(names),
311- detect_nonexistent_zone_names(names))
312+ detect_nonexistent_names(Zone, names),
313+ detect_nonexistent_names(Zone, names))
314
315- def test_detect_nonexistent_zone_names_combines_good_and_bad_names(self):
316+ def test_detect_nonexistent_names_combines_good_and_bad_names(self):
317 zone = factory.make_Zone().name
318 non_zone = factory.make_name('nonzone')
319 self.assertEqual(
320 [non_zone],
321- detect_nonexistent_zone_names([zone, non_zone]))
322+ detect_nonexistent_names(Zone, [zone, non_zone]))
323
324- def test_detect_nonexistent_zone_names_asserts_parameter_type(self):
325+ def test_detect_nonexistent_names_asserts_parameter_type(self):
326 self.assertRaises(
327- AssertionError, detect_nonexistent_zone_names, "text")
328+ AssertionError, detect_nonexistent_names, Zone, "text")
329
330 def test_get_storage_constraints_from_string_returns_None_for_empty(self):
331 self.assertEqual(None, get_storage_constraints_from_string(""))
332@@ -770,6 +772,74 @@ class TestAcquireNodeForm(MAASServerTestCase):
333 [nodes[2]],
334 {'not_in_zone': [nodes[0].zone.name, nodes[1].zone.name]})
335
336+ def test_pool(self):
337+ node1 = factory.make_Node()
338+ node2 = factory.make_Node()
339+ node3 = factory.make_Node()
340+
341+ pool1 = factory.make_ResourcePool(nodes=[node1, node2])
342+ pool2 = factory.make_ResourcePool()
343+
344+ self.assertConstrainedNodes(
345+ [node1, node2], {'pool': pool1.name})
346+ self.assertConstrainedNodes(
347+ [node1, node2, node3], {'pool': ''})
348+ self.assertConstrainedNodes(
349+ [node1, node2, node3], {})
350+ self.assertConstrainedNodes(
351+ [], {'pool': pool2.name})
352+
353+ def test_invalid_pool(self):
354+ form = AcquireNodeForm(data={'pool': 'unknown'})
355+ self.assertEqual(
356+ (False, {'pool': ["No such pool: 'unknown'."]}),
357+ (form.is_valid(), form.errors))
358+
359+ def test_not_in_pool_excludes_given_pools(self):
360+ node1 = factory.make_Node()
361+ node2 = factory.make_Node()
362+ node3 = factory.make_Node()
363+ node4 = factory.make_Node()
364+
365+ factory.make_ResourcePool(nodes=[node1, node2])
366+ pool2 = factory.make_ResourcePool(nodes=[node3, node4])
367+ self.assertConstrainedNodes(
368+ [node1, node2], {'not_in_pool': [pool2.name]})
369+
370+ def test_not_in_pool_with_required_pool_yields_no_nodes(self):
371+ node = factory.make_Node()
372+ pool = factory.make_ResourcePool(nodes=[node])
373+ self.assertConstrainedNodes(
374+ [], {'pool': pool.name, 'not_in_pool': [pool.name]})
375+
376+ def test_validates_not_in_pool(self):
377+ bad_pool_name = '#$&*!'
378+ form = AcquireNodeForm(data={'not_in_pool': [bad_pool_name]})
379+ self.assertFalse(form.is_valid())
380+ self.assertEqual(['not_in_pool'], list(form.errors.keys()))
381+
382+ def test_not_in_pool_must_be_pool_name(self):
383+ non_pool = factory.make_name('nonpool')
384+ form = AcquireNodeForm(data={'not_in_pool': [non_pool]})
385+ self.assertFalse(form.is_valid())
386+ self.assertEqual(
387+ {'not_in_pool': ["No such pool(s): %s." % non_pool]},
388+ form.errors)
389+
390+ def test_not_in_pool_can_exclude_multiple_pool(self):
391+ # Three nodes, all in different pools. If we say we don't
392+ # want the first node's pool or the second node's pool, we get the node
393+ # in the remaining pool.
394+ node1 = factory.make_Node()
395+ pool1 = factory.make_ResourcePool(nodes=[node1])
396+ node2 = factory.make_Node()
397+ pool2 = factory.make_ResourcePool(nodes=[node2])
398+ node3 = factory.make_Node()
399+ factory.make_ResourcePool(nodes=[node3])
400+
401+ self.assertConstrainedNodes(
402+ [node3], {'not_in_pool': [pool1.name, pool2.name]})
403+
404 def test_tags(self):
405 tag_big = factory.make_Tag(name='big')
406 tag_burly = factory.make_Tag(name='burly')
407@@ -1390,6 +1460,8 @@ class TestAcquireNodeForm(MAASServerTestCase):
408 'not_connected_to': [factory.make_mac_address()],
409 'zone': factory.make_Zone(),
410 'not_in_zone': [factory.make_Zone().name],
411+ 'pool': factory.make_ResourcePool(),
412+ 'not_in_pool': [factory.make_ResourcePool().name],
413 'pod': factory.make_name(),
414 'pod_type': factory.make_name(),
415 'storage': '0(ssd),10(ssd)',

Subscribers

People subscribed via source and target branches