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

[Newell Jensen]

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

Exclude pod machine composition during allocation if tags are supplied.

[Blake Rouse (blake-rouse)]

Looks good overall, but a question first.

review: Needs Information

[Newell Jensen (newell-jensen)]

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

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)]

Thanks for the fixes

review: Approve

9b02c13... by Newell Jensen on 2017-07-27

Delete print statement used for debugging.

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index dfca107..b709e10 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -1412,11 +1412,11 @@ class MachinesHandler(NodesHandler, PowersMixin):
         # XXX AndresRodriguez 2016-10-27: If new params are added and are not
         # constraints, these need to be added to IGNORED_FIELDS in
         # src/maasserver/node_constraint_filter_forms.py.
-        input_constraints = str([
-            param for param in request.data.lists() if param[0] != 'op'])
+        input_constraints = [
+            param for param in request.data.lists() if param[0] != 'op']
         maaslog.info(
             "Request from user %s to acquire a machine with constraints: %s",
-            request.user.username, input_constraints)
+            request.user.username, str(input_constraints))
         agent_name, bridge_all, bridge_fd, bridge_stp, comment = (
             get_allocation_parameters(request))
         verbose = get_optional_param(
@@ -1457,7 +1457,11 @@ class MachinesHandler(NodesHandler, PowersMixin):
                     "storage": storage,
                 }
                 pods = Pod.objects.all()
-                if pods:
+                # We don't want to compose a machine from a pod if the
+                # constraints contain tags.
+                if pods and not any(
+                        'tags' in constraint
+                        for constraint in input_constraints):
                     if form.cleaned_data.get('pod'):
                         pods = pods.filter(name=form.cleaned_data.get('pod'))
                     elif form.cleaned_data.get('pod_type'):
@@ -1484,7 +1488,7 @@ class MachinesHandler(NodesHandler, PowersMixin):
                     message = (
                         'No available machine matches constraints: %s '
                         '(resolved to "%s")' % (
-                            input_constraints, constraints))
+                            str(input_constraints), constraints))
                 raise NodesNotAvailable(message)
             if not dry_run:
                 machine.acquire(
diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
index c9d0c7e..f9d53e9 100644
--- a/src/maasserver/api/tests/test_machines.py
+++ b/src/maasserver/api/tests/test_machines.py
@@ -1055,6 +1055,26 @@ class TestMachinesAPI(APITestCase.ForUser):
             response.content.decode(settings.DEFAULT_CHARSET))
         self.assertItemsEqual(machine_tag_names, response_json['tag_names'])
 
+    def test_POST_allocate_does_not_compose_machine_by_tags(self):
+        pod = factory.make_Pod()
+        pod.architectures = [random.choice([
+            "amd64/generic", "i386/generic",
+            "armhf/generic", "arm64/generic"
+        ])]
+        pod.save()
+        mock_filter_nodes = self.patch(AcquireNodeForm, 'filter_nodes')
+        mock_filter_nodes.return_value = [], {}, {}
+        mock_compose = self.patch(ComposeMachineForPodsForm, 'compose')
+        factory.make_Tag('fast')
+        factory.make_Tag('stable')
+        # Legacy call using comma-separated tags.
+        response = self.client.post(reverse('machines_handler'), {
+            'op': 'allocate',
+            'tags': ['fast', 'stable'],
+        })
+        self.assertThat(response, HasStatusCode(http.client.CONFLICT))
+        self.assertThat(mock_compose, MockNotCalled())
+
     def test_POST_allocate_allocates_machine_by_negated_tags(self):
         tagged_machine = factory.make_Node(
             status=NODE_STATUS.READY, with_boot_disk=True)