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

Proposed by Newell Jensen on 2017-07-27
Status: Merged
Approved by: Newell Jensen on 2017-07-27
Approved revision: 9b02c13f5825bd6b8d4c139be2f14135ffab30bf
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~newell-jensen/maas:lp1706196
Merge into: maas:master
Diff against target: 72 lines (+29/-5)
2 files modified
src/maasserver/api/machines.py (+9/-5)
src/maasserver/api/tests/test_machines.py (+20/-0)
Reviewer Review Type Date Requested Status
Blake Rouse 2017-07-27 Approve on 2017-07-27
Review via email: mp+328146@code.launchpad.net

Commit message

LP: #1706196 - Don't allocate composed machine with tags.

Exclude pod machine composition during allocation if tags are supplied.

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Looks good overall, but a question first.

review: Needs Information
Newell Jensen (newell-jensen) wrote :

Replied inline. Since input_constraints is a list of tuples, was searching to see if the key 'tags' was in one of the tuples.

~newell-jensen/maas:lp1706196 updated on 2017-07-27
fed8060... by Newell Jensen on 2017-07-27

Clean up code.

1737b4b... by Newell Jensen on 2017-07-27

Change logical statement for checking if 'tags' is part of the input_constraints.

Blake Rouse (blake-rouse) wrote :

Thanks for the fixes

review: Approve
~newell-jensen/maas:lp1706196 updated on 2017-07-27
9b02c13... by Newell Jensen on 2017-07-27

Delete print statement used for debugging.

Update scan failed

At least one of the branches involved have failed to scan. You can manually schedule a rescan if required.

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 dfca107..b709e10 100644
3--- a/src/maasserver/api/machines.py
4+++ b/src/maasserver/api/machines.py
5@@ -1412,11 +1412,11 @@ class MachinesHandler(NodesHandler, PowersMixin):
6 # XXX AndresRodriguez 2016-10-27: If new params are added and are not
7 # constraints, these need to be added to IGNORED_FIELDS in
8 # src/maasserver/node_constraint_filter_forms.py.
9- input_constraints = str([
10- param for param in request.data.lists() if param[0] != 'op'])
11+ input_constraints = [
12+ param for param in request.data.lists() if param[0] != 'op']
13 maaslog.info(
14 "Request from user %s to acquire a machine with constraints: %s",
15- request.user.username, input_constraints)
16+ request.user.username, str(input_constraints))
17 agent_name, bridge_all, bridge_fd, bridge_stp, comment = (
18 get_allocation_parameters(request))
19 verbose = get_optional_param(
20@@ -1457,7 +1457,11 @@ class MachinesHandler(NodesHandler, PowersMixin):
21 "storage": storage,
22 }
23 pods = Pod.objects.all()
24- if pods:
25+ # We don't want to compose a machine from a pod if the
26+ # constraints contain tags.
27+ if pods and not any(
28+ 'tags' in constraint
29+ for constraint in input_constraints):
30 if form.cleaned_data.get('pod'):
31 pods = pods.filter(name=form.cleaned_data.get('pod'))
32 elif form.cleaned_data.get('pod_type'):
33@@ -1484,7 +1488,7 @@ class MachinesHandler(NodesHandler, PowersMixin):
34 message = (
35 'No available machine matches constraints: %s '
36 '(resolved to "%s")' % (
37- input_constraints, constraints))
38+ str(input_constraints), constraints))
39 raise NodesNotAvailable(message)
40 if not dry_run:
41 machine.acquire(
42diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
43index c9d0c7e..f9d53e9 100644
44--- a/src/maasserver/api/tests/test_machines.py
45+++ b/src/maasserver/api/tests/test_machines.py
46@@ -1055,6 +1055,26 @@ class TestMachinesAPI(APITestCase.ForUser):
47 response.content.decode(settings.DEFAULT_CHARSET))
48 self.assertItemsEqual(machine_tag_names, response_json['tag_names'])
49
50+ def test_POST_allocate_does_not_compose_machine_by_tags(self):
51+ pod = factory.make_Pod()
52+ pod.architectures = [random.choice([
53+ "amd64/generic", "i386/generic",
54+ "armhf/generic", "arm64/generic"
55+ ])]
56+ pod.save()
57+ mock_filter_nodes = self.patch(AcquireNodeForm, 'filter_nodes')
58+ mock_filter_nodes.return_value = [], {}, {}
59+ mock_compose = self.patch(ComposeMachineForPodsForm, 'compose')
60+ factory.make_Tag('fast')
61+ factory.make_Tag('stable')
62+ # Legacy call using comma-separated tags.
63+ response = self.client.post(reverse('machines_handler'), {
64+ 'op': 'allocate',
65+ 'tags': ['fast', 'stable'],
66+ })
67+ self.assertThat(response, HasStatusCode(http.client.CONFLICT))
68+ self.assertThat(mock_compose, MockNotCalled())
69+
70 def test_POST_allocate_allocates_machine_by_negated_tags(self):
71 tagged_machine = factory.make_Node(
72 status=NODE_STATUS.READY, with_boot_disk=True)

Subscribers

People subscribed via source and target branches

to all changes: