Merge lp:~allenap/maas/rpc-tags-cluster into lp:~maas-committers/maas/trunk

[Gavin Panella]

New EvaluateTag RPC command on the cluster.

This hobbles tag evaluation in the region, but I won't land this as-is. It's a merge proposal for review purposes only.

[Graham Binns (gmb)]

Looking good so far. One minor nitpick, but otherwise it's cool beans.

review: Approve

[Gavin Panella (allenap)]

Thanks for the review :)

[MAAS Lander (maas-lander)]

The attempt to merge lp:~allenap/maas/rpc-tags-cluster into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Get:2 http://security.ubuntu.com trusty-security Release [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [59.7 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [44.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [10.8 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [144 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [48.4 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [120 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [84.7 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [322 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [204 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Fetched 1,101 kB in 0s (1,663 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl ...

=== modified file 'src/maasserver/api/tests/test_tag.py'
--- src/maasserver/api/tests/test_tag.py	2014-09-10 16:20:31 +0000
+++ src/maasserver/api/tests/test_tag.py	2014-09-23 09:14:01 +0000
@@ -104,6 +104,8 @@
         self.assertTrue(Tag.objects.filter(name='new-tag-name').exists())
 
     def test_PUT_updates_node_associations(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         node1 = factory.make_Node()
         inject_lshw_result(node1, b'<node><foo/></node>')
         node2 = factory.make_Node()
@@ -141,6 +143,8 @@
                          [r['system_id'] for r in parsed_result])
 
     def test_GET_nodes_hides_invisible_nodes(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         user2 = factory.make_User()
         node1 = factory.make_Node()
         inject_lshw_result(node1, b'<node><foo/></node>')
@@ -162,6 +166,8 @@
                               [r['system_id'] for r in parsed_result])
 
     def test_PUT_invalid_definition(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         self.become_admin()
         node = factory.make_Node()
         inject_lshw_result(node, b'<node ><child/></node>')
@@ -332,6 +338,8 @@
         self.assertItemsEqual([], node.tags.all())
 
     def test_POST_rebuild_rebuilds_node_mapping(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         tag = factory.make_Tag(definition='//foo/bar')
         # Only one node matches the tag definition, rebuilding should notice
         node_matching = factory.make_Node()
@@ -483,6 +491,8 @@
             extra_kernel_opts, Tag.objects.filter(name=name)[0].kernel_opts)
 
     def test_POST_new_populates_nodes(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         self.become_admin()
         node1 = factory.make_Node()
         inject_lshw_result(node1, b'<node><child/></node>')
 
=== modified file 'src/maasserver/models/tag.py'
--- src/maasserver/models/tag.py	2013-12-02 14:29:41 +0000
+++ src/maasserver/models/tag.py	2014-09-23 09:14:01 +0000
@@ -101,7 +101,6 @@
 
     def populate_nodes(self):
         """Find all nodes that match this tag, and update them."""
-        from maasserver.populate_tags import populate_tags
         if not self.is_defined:
             return
         # before we pass off any work, ensure the definition is valid XPATH
@@ -112,7 +111,9 @@
             raise ValidationError({'definition': [msg]})
         # Now delete the existing tags
         self.node_set.clear()
-        populate_tags(self)
+        # Being ported to RPC.
+        # from maasserver.populate_tags import populate_tags
+        # populate_tags(self)
 
     def save(self, *args, **kwargs):
         super(Tag, self).save(*args, **kwargs)
 
=== modified file 'src/maasserver/models/tests/test_tag.py'
--- src/maasserver/models/tests/test_tag.py	2014-09-19 13:27:37 +0000
+++ src/maasserver/models/tests/test_tag.py	2014-09-23 09:14:01 +0000
@@ -67,6 +67,8 @@
             self.assertRaises(ValidationError, factory.make_Tag, name=invalid)
 
     def test_applies_tags_to_nodes(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         node1 = factory.make_Node()
         inject_lshw_result(node1, b'<node><child /></node>')
         node2 = factory.make_Node()
@@ -76,6 +78,8 @@
         self.assertItemsEqual([], node2.tag_names())
 
     def test_removes_old_values(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         node1 = factory.make_Node()
         inject_lshw_result(node1, b'<node><foo /></node>')
         node2 = factory.make_Node()
@@ -94,6 +98,8 @@
         self.assertItemsEqual([], node2.tag_names())
 
     def test_doesnt_touch_other_tags(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         node1 = factory.make_Node()
         inject_lshw_result(node1, b'<node><foo /></node>')
         node2 = factory.make_Node()
@@ -106,6 +112,8 @@
         self.assertItemsEqual([tag2.name], node2.tag_names())
 
     def test_rollsback_invalid_xpath(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         node = factory.make_Node()
         inject_lshw_result(node, b'<node><foo /></node>')
         tag = factory.make_Tag(definition='//node/foo')
@@ -159,6 +167,8 @@
         self.assertItemsEqual([], tag.node_set.all())
 
     def test__calls_populate_tags(self):
+        self.skip("Tag evaluation is being ported to RPC.")
+
         populate_tags = self.patch_autospec(
             populate_tags_module, "populate_tags")
 
 
=== modified file 'src/maasserver/populate_tags.py'
--- src/maasserver/populate_tags.py	2014-08-13 21:49:35 +0000
+++ src/maasserver/populate_tags.py	2014-09-23 09:14:01 +0000
@@ -28,7 +28,6 @@
     )
 from maasserver.refresh_worker import refresh_worker
 from provisioningserver.tags import merge_details
-from provisioningserver.tasks import update_node_tags
 from provisioningserver.utils import classify
 from provisioningserver.utils.xpath import try_match_xpath
 
@@ -56,7 +55,8 @@
     logger.debug('Refreshing tag definition for %s' % (items,))
     for nodegroup in NodeGroup.objects.all():
         refresh_worker(nodegroup)
-        update_node_tags.apply_async(queue=nodegroup.work_queue, kwargs=items)
+        raise NotImplementedError(
+            "Tag evaluation is being ported to RPC.")
 
 
 def populate_tags_for_single_node(tags, node):
 
=== modified file 'src/maasserver/tests/test_populate_tags.py'
--- src/maasserver/tests/test_populate_tags.py	2014-09-15 14:28:28 +0000
+++ src/maasserver/tests/test_populate_tags.py	2014-09-23 09:14:01 +0000
@@ -14,49 +14,11 @@
 __metaclass__ = type
 __all__ = []
 
-from maasserver import populate_tags as populate_tags_module
 from maasserver.models import Tag
-from maasserver.populate_tags import (
-    populate_tags,
-    populate_tags_for_single_node,
-    tag_nsmap,
-    )
+from maasserver.populate_tags import populate_tags_for_single_node
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from metadataserver.models import commissioningscript
-import mock
-
-
-class TestPopulateTags(MAASServerTestCase):
-
-    def test_populate_tags_task_routed_to_nodegroup_worker(self):
-        nodegroup = factory.make_NodeGroup()
-        tag = factory.make_Tag()
-        task = self.patch(populate_tags_module, 'update_node_tags')
-        populate_tags(tag)
-        args, kwargs = task.apply_async.call_args
-        self.assertEqual(nodegroup.work_queue, kwargs['queue'])
-
-    def test_populate_tags_task_routed_to_all_nodegroup_workers(self):
-        nodegroups = [factory.make_NodeGroup() for _ in range(5)]
-        tag = factory.make_Tag()
-        refresh = self.patch(populate_tags_module, 'refresh_worker')
-        task = self.patch(populate_tags_module, 'update_node_tags')
-        populate_tags(tag)
-        refresh_calls = [mock.call(nodegroup) for nodegroup in nodegroups]
-        refresh.assert_has_calls(refresh_calls, any_order=True)
-        task_calls = [
-            mock.call(
-                queue=nodegroup.work_queue,
-                kwargs={
-                    'tag_name': tag.name,
-                    'tag_definition': tag.definition,
-                    'tag_nsmap': tag_nsmap,
-                },
-            )
-            for nodegroup in nodegroups
-        ]
-        task.apply_async.assert_has_calls(task_calls, any_order=True)
 
 
 class TestPopulateTagsForSingleNode(MAASServerTestCase):
 
=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-09-18 21:21:18 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-09-23 09:14:01 +0000
@@ -345,6 +345,26 @@
     error = []
 
 
+class EvaluateTag(amp.Command):
+    """Evaluate a tag against all of the cluster's nodes.
+
+    :since: 1.7
+    """
+
+    arguments = [
+        (b"tag_name", amp.Unicode()),
+        (b"tag_definition", amp.Unicode()),
+        (b"tag_nsmap", amp.AmpList([
+            (b"prefix", amp.Unicode()),
+            (b"uri", amp.Unicode()),
+        ])),
+        # A 3-part credential string for the web API.
+        (b"credentials", amp.Unicode()),
+    ]
+    response = []
+    errors = []
+
+
 class AddVirsh(amp.Command):
     """Probe for and enlist virsh VMs attached to the cluster.
 
 
=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-09-22 11:31:14 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-09-23 09:14:01 +0000
@@ -21,6 +21,7 @@
 import random
 from urlparse import urlparse
 
+from apiclient.creds import convert_string_to_tuple
 from apiclient.utils import ascii_url
 from provisioningserver.cluster_config import (
     get_cluster_uuid,
@@ -68,6 +69,7 @@
     change_power_state,
     get_power_state,
     )
+from provisioningserver.rpc.tags import evaluate_tag
 from provisioningserver.utils.network import find_ip_via_arp
 from twisted.application.internet import TimerService
 from twisted.internet.defer import inlineCallbacks
@@ -76,6 +78,7 @@
     TCP4ClientEndpoint,
     )
 from twisted.internet.error import ConnectError
+from twisted.internet.threads import deferToThread
 from twisted.protocols import amp
 from twisted.python import log
 from twisted.web.client import getPage
@@ -245,6 +248,22 @@
         else:
             return tls.get_tls_parameters_for_cluster()
 
+    @cluster.EvaluateTag.responder
+    def evaluate_tag(self, tag_name, tag_definition, tag_nsmap, credentials):
+        """evaluate_tag()
+
+        Implementation of
+        :py:class:`~provisioningserver.rpc.cluster.EvaluateTag`.
+        """
+        # It's got to run in a thread because it does blocking IO.
+        d = deferToThread(
+            evaluate_tag, tag_name, tag_definition,
+            # Transform tag_nsmap into a format that LXML likes.
+            {entry["prefix"]: entry["uri"] for entry in tag_nsmap},
+            # Parse the credential string into a 3-tuple.
+            convert_string_to_tuple(credentials))
+        return d.addCallback(lambda _: {})
+
     @cluster.AddVirsh.responder
     def add_virsh(self, poweraddr, password):
         """add_virsh()
 
=== added file 'src/provisioningserver/rpc/tags.py'
--- src/provisioningserver/rpc/tags.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/rpc/tags.py	2014-09-23 09:14:01 +0000
@@ -0,0 +1,46 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""RPC helpers for dealing with tags."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    "evaluate_tag",
+]
+
+from apiclient.maas_client import (
+    MAASClient,
+    MAASDispatcher,
+    MAASOAuth,
+    )
+from provisioningserver.cluster_config import (
+    get_cluster_uuid,
+    get_maas_url,
+    )
+from provisioningserver.tags import process_node_tags
+from provisioningserver.utils.twisted import synchronous
+
+
+@synchronous
+def evaluate_tag(tag_name, tag_definition, tag_nsmap, credentials):
+    """Evaluate `tag_definition` against this cluster's nodes' details.
+
+    :param tag_name: The name of the tag, used for logging.
+    :param tag_definition: The XPath expression of the tag.
+    :param tag_nsmap: The namespace map as used by LXML's ETree library.
+    :param credentials: A 3-tuple of OAuth credentials.
+    """
+    client = MAASClient(
+        auth=MAASOAuth(*credentials), dispatcher=MAASDispatcher(),
+        base_url=get_maas_url())
+    process_node_tags(
+        tag_name=tag_name, tag_definition=tag_definition, tag_nsmap=tag_nsmap,
+        client=client, nodegroup_uuid=get_cluster_uuid())
 
=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-22 21:34:33 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-23 09:14:01 +0000
@@ -25,6 +25,7 @@
 from random import randint
 from urlparse import urlparse
 
+from apiclient.creds import convert_tuple_to_string
 from fixtures import EnvironmentVariable
 from maastesting.factory import factory
 from maastesting.matchers import (
@@ -64,6 +65,7 @@
     osystems as osystems_rpc_module,
     power as power_module,
     region,
+    tags,
     )
 from provisioningserver.rpc.clusterservice import (
     Cluster,
@@ -1185,6 +1187,70 @@
         self.assertThat(running_monitors, Not(Contains(monitors[0]["id"])))
 
 
+class TestClusterProtocol_EvaluateTag(MAASTestCase):
+
+    run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
+
+    def test__is_registered(self):
+        protocol = Cluster()
+        responder = protocol.locateResponder(
+            cluster.EvaluateTag.commandName)
+        self.assertIsNot(responder, None)
+
+    @inlineCallbacks
+    def test_happy_path(self):
+        get_maas_url = self.patch_autospec(tags, "get_maas_url")
+        get_maas_url.return_value = sentinel.maas_url
+        get_cluster_uuid = self.patch_autospec(tags, "get_cluster_uuid")
+        get_cluster_uuid.return_value = sentinel.cluster_uuid
+
+        # Prevent real work being done, which would involve HTTP calls.
+        self.patch_autospec(tags, "process_node_tags")
+
+        response = yield call_responder(
+            Cluster(), cluster.EvaluateTag, {
+                "tag_name": "all-nodes",
+                "tag_definition": "//*",
+                "tag_nsmap": [
+                    {"prefix": "foo",
+                     "uri": "http://foo.example.com/"},
+                ],
+                "credentials": "abc:def:ghi",
+            })
+
+        self.assertEqual({}, response)
+
+    @inlineCallbacks
+    def test__calls_through_to_evaluate_tag_helper(self):
+        evaluate_tag = self.patch_autospec(clusterservice, "evaluate_tag")
+
+        tag_name = factory.make_name("tag-name")
+        tag_definition = factory.make_name("tag-definition")
+        tag_ns_prefix = factory.make_name("tag-ns-prefix")
+        tag_ns_uri = factory.make_name("tag-ns-uri")
+
+        consumer_key = factory.make_name("ckey")
+        resource_token = factory.make_name("rtok")
+        resource_secret = factory.make_name("rsec")
+        credentials = convert_tuple_to_string(
+            (consumer_key, resource_token, resource_secret))
+
+        yield call_responder(
+            Cluster(), cluster.EvaluateTag, {
+                "tag_name": tag_name,
+                "tag_definition": tag_definition,
+                "tag_nsmap": [
+                    {"prefix": tag_ns_prefix, "uri": tag_ns_uri},
+                ],
+                "credentials": credentials,
+            })
+
+        self.assertThat(evaluate_tag, MockCalledOnceWith(
+            tag_name, tag_definition, {tag_ns_prefix: tag_ns_uri},
+            (consumer_key, resource_token, resource_secret),
+        ))
+
+
 class TestClusterProtocol_AddVirsh(MAASTestCase):
 
     def test__is_registered(self):
 
=== added file 'src/provisioningserver/rpc/tests/test_tags.py'
--- src/provisioningserver/rpc/tests/test_tags.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/rpc/tests/test_tags.py	2014-09-23 09:14:01 +0000
@@ -0,0 +1,73 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for :py:module:`~provisioningserver.rpc.dhcp`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+from apiclient.maas_client import (
+    MAASClient,
+    MAASDispatcher,
+    MAASOAuth,
+    )
+from maastesting.factory import factory
+from maastesting.matchers import MockCalledOnceWith
+from maastesting.testcase import MAASTestCase
+from mock import (
+    ANY,
+    sentinel,
+    )
+from provisioningserver.rpc import tags
+
+
+class TestEvaluateTag(MAASTestCase):
+
+    def setUp(self):
+        super(TestEvaluateTag, self).setUp()
+        get_maas_url = self.patch_autospec(tags, "get_maas_url")
+        get_maas_url.return_value = sentinel.maas_url
+        get_cluster_uuid = self.patch_autospec(tags, "get_cluster_uuid")
+        get_cluster_uuid.return_value = sentinel.cluster_uuid
+
+    def test__calls_process_node_tags(self):
+        credentials = "aaa", "bbb", "ccc"
+        process_node_tags = self.patch_autospec(tags, "process_node_tags")
+        tags.evaluate_tag(
+            sentinel.tag_name, sentinel.tag_definition, sentinel.tag_nsmap,
+            credentials)
+        self.assertThat(
+            process_node_tags, MockCalledOnceWith(
+                tag_name=sentinel.tag_name,
+                tag_definition=sentinel.tag_definition,
+                tag_nsmap=sentinel.tag_nsmap, client=ANY,
+                nodegroup_uuid=sentinel.cluster_uuid))
+
+    def test__constructs_client_with_credentials(self):
+        consumer_key = factory.make_name("ckey")
+        resource_token = factory.make_name("rtok")
+        resource_secret = factory.make_name("rsec")
+        credentials = consumer_key, resource_token, resource_secret
+
+        self.patch_autospec(tags, "process_node_tags")
+        self.patch_autospec(tags, "MAASOAuth").side_effect = MAASOAuth
+
+        tags.evaluate_tag(
+            sentinel.tag_name, sentinel.tag_definition, sentinel.tag_nsmap,
+            credentials)
+
+        client = tags.process_node_tags.call_args[1]["client"]
+        self.assertIsInstance(client, MAASClient)
+        self.assertEqual(sentinel.maas_url, client.url)
+        self.assertIsInstance(client.dispatcher, MAASDispatcher)
+        self.assertIsInstance(client.auth, MAASOAuth)
+        self.assertThat(tags.MAASOAuth, MockCalledOnceWith(
+            consumer_key, resource_token, resource_secret))
 
=== modified file 'src/provisioningserver/tags.py'
--- src/provisioningserver/tags.py	2014-08-13 21:49:35 +0000
+++ src/provisioningserver/tags.py	2014-09-23 09:14:01 +0000
@@ -17,7 +17,6 @@
 __all__ = [
     'merge_details',
     'merge_details_cleanly',
-    'MissingCredentials',
     'process_node_tags',
     ]
 
@@ -27,18 +26,8 @@
 import httplib
 import urllib2
 
-from apiclient.maas_client import (
-    MAASClient,
-    MAASDispatcher,
-    MAASOAuth,
-    )
 import bson
 from lxml import etree
-from provisioningserver.auth import (
-    get_recorded_api_credentials,
-    get_recorded_nodegroup_uuid,
-    )
-from provisioningserver.cluster_config import get_maas_url
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.utils import classify
 from provisioningserver.utils.xpath import try_match_xpath
@@ -48,10 +37,6 @@
 maaslog = get_maas_logger("tag_processing")
 
 
-class MissingCredentials(Exception):
-    """The MAAS URL or credentials are not yet set."""
-
-
 # An example laptop's lshw XML dump was 135kB. An example lab's LLDP
 # XML dump was 1.6kB. A batch size of 100 would mean downloading ~14MB
 # from the region controller, which seems workable. The previous batch
@@ -60,25 +45,6 @@
 DEFAULT_BATCH_SIZE = 100
 
 
-def get_cached_knowledge():
-    """Get all the information that we need to know, or raise an error.
-
-    :return: (client, nodegroup_uuid)
-    """
-    api_credentials = get_recorded_api_credentials()
-    if api_credentials is None:
-        maaslog.error("Not updating tags: don't have API key yet.")
-        return None, None
-    nodegroup_uuid = get_recorded_nodegroup_uuid()
-    if nodegroup_uuid is None:
-        maaslog.error("Not updating tags: don't have UUID yet.")
-        return None, None
-    client = MAASClient(
-        MAASOAuth(*api_credentials), MAASDispatcher(),
-        get_maas_url())
-    return client, nodegroup_uuid
-
-
 # A content-type: function mapping that can decode data of that type.
 decoders = {
     "application/json": lambda data: json.loads(data),
@@ -346,20 +312,18 @@
         nodes_matched, nodes_unmatched)
 
 
-def process_node_tags(tag_name, tag_definition, tag_nsmap, batch_size=None):
+def process_node_tags(
+        tag_name, tag_definition, tag_nsmap,
+        client, nodegroup_uuid, batch_size=None):
     """Update the nodes for a new/changed tag definition.
 
+    :param client: A `MAASClient` used to fetch the node's details via
+        calls to the web API.
+    :param nodegroup_uuid: The UUID for this cluster.
     :param tag_name: Name of the tag to update nodes for
     :param tag_definition: Tag definition
     :param batch_size: Size of batch
     """
-    client, nodegroup_uuid = get_cached_knowledge()
-    if not all([client, nodegroup_uuid]):
-        maaslog.error(
-            "Unable to update tag: %s for definition %r. "
-            "Please refresh secrets, then rebuild this tag."
-            % (tag_name, tag_definition))
-        raise MissingCredentials()
     # We evaluate this early, so we can fail before sending a bunch of data to
     # the server
     xpath = etree.XPath(tag_definition, namespaces=tag_nsmap)
 
=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2014-09-17 08:51:31 +0000
+++ src/provisioningserver/tasks.py	2014-09-23 09:14:01 +0000
@@ -28,10 +28,7 @@
 
 from celery.app import app_or_default
 from celery.task import task
-from provisioningserver import (
-    boot_images,
-    tags,
-    )
+from provisioningserver import boot_images
 from provisioningserver.auth import (
     record_api_credentials,
     record_nodegroup_uuid,
@@ -242,35 +239,3 @@
 def report_boot_images():
     """For master worker only: report available netboot images."""
     boot_images.report_to_server()
-
-
-# How many times should a update node tags task be retried?
-UPDATE_NODE_TAGS_MAX_RETRY = 10
-
-# How long to wait between update node tags task retries (in seconds)?
-UPDATE_NODE_TAGS_RETRY_DELAY = 2
-
-
-# =====================================================================
-# Tags-related tasks
-# =====================================================================
-
-
-@task(max_retries=UPDATE_NODE_TAGS_MAX_RETRY)
-@log_call()
-@log_exception_text
-def update_node_tags(tag_name, tag_definition, tag_nsmap, retry=True):
-    """Update the nodes for a new/changed tag definition.
-
-    :param tag_name: Name of the tag to update nodes for
-    :param tag_definition: Tag definition
-    :param retry: Whether to retry on failure
-    """
-    try:
-        tags.process_node_tags(tag_name, tag_definition, tag_nsmap)
-    except tags.MissingCredentials, exc:
-        if retry:
-            return update_node_tags.retry(
-                exc=exc, countdown=UPDATE_NODE_TAGS_RETRY_DELAY)
-        else:
-            raise
 
=== modified file 'src/provisioningserver/tests/test_tags.py'
--- src/provisioningserver/tests/test_tags.py	2014-09-11 09:19:44 +0000
+++ src/provisioningserver/tests/test_tags.py	2014-09-23 09:14:01 +0000
@@ -37,7 +37,6 @@
     sentinel,
     )
 from provisioningserver import tags
-from provisioningserver.auth import get_recorded_nodegroup_uuid
 from provisioningserver.testing.testcase import PservTestCase
 from testtools.matchers import (
     DocTestMatches,
@@ -484,29 +483,6 @@
         super(TestTagUpdating, self).setUp()
         self.useFixture(FakeLogger())
 
-    def test_get_cached_knowledge_knows_nothing(self):
-        # If we haven't given it any secrets, we should get back nothing
-        self.assertEqual((None, None), tags.get_cached_knowledge())
-
-    def test_get_cached_knowledge_with_only_url(self):
-        self.set_maas_url()
-        self.assertEqual((None, None), tags.get_cached_knowledge())
-
-    def test_get_cached_knowledge_with_only_url_creds(self):
-        self.set_maas_url()
-        self.set_api_credentials()
-        self.assertEqual((None, None), tags.get_cached_knowledge())
-
-    def test_get_cached_knowledge_with_all_info(self):
-        self.set_maas_url()
-        self.set_api_credentials()
-        self.set_node_group_uuid()
-        client, uuid = tags.get_cached_knowledge()
-        self.assertIsNot(None, client)
-        self.assertIsInstance(client, MAASClient)
-        self.assertIsNot(None, uuid)
-        self.assertEqual(get_recorded_nodegroup_uuid(), uuid)
-
     def fake_client(self):
         return MAASClient(None, None, self.make_maas_url())
 
@@ -615,16 +591,6 @@
             (['a', 'c'], ['b']),
             tags.classify(xpath, node_details))
 
-    def test_process_node_tags_no_secrets(self):
-        self.patch(MAASClient, 'get')
-        self.patch(MAASClient, 'post')
-        tag_name = factory.make_name('tag')
-        self.assertRaises(
-            tags.MissingCredentials,
-            tags.process_node_tags, tag_name, '//node', None)
-        self.assertFalse(MAASClient.get.called)
-        self.assertFalse(MAASClient.post.called)
-
     def test_process_node_tags_integration(self):
         self.set_secrets()
         get_nodes = FakeMethod(
@@ -653,10 +619,12 @@
         self.patch(MAASClient, 'get', get_fake)
         self.patch(MAASClient, 'post', post_fake)
         tag_name = factory.make_name('tag')
-        nodegroup_uuid = get_recorded_nodegroup_uuid()
+        nodegroup_uuid = factory.make_name("nodegroup-uuid")
         tag_definition = '//lshw:node'
         tag_nsmap = {"lshw": "lshw"}
-        tags.process_node_tags(tag_name, tag_definition, tag_nsmap=tag_nsmap)
+        tags.process_node_tags(
+            tag_name, tag_definition, tag_nsmap,
+            self.fake_client(), nodegroup_uuid)
         nodegroup_url = '/api/1.0/nodegroups/%s/' % (nodegroup_uuid,)
         tag_url = '/api/1.0/tags/%s/' % (tag_name,)
         self.assertEqual(
@@ -687,9 +655,6 @@
         client = object()
         uuid = factory.make_name('nodegroupuuid')
         self.patch(
-            tags, 'get_cached_knowledge',
-            MagicMock(return_value=(client, uuid)))
-        self.patch(
             tags, 'get_nodes_for_node_group',
             MagicMock(return_value=['a', 'b', 'c']))
         fake_first = FakeMethod(result={
@@ -706,8 +671,8 @@
         tag_name = factory.make_name('tag')
         tag_definition = '//node'
         tags.process_node_tags(
-            tag_name, tag_definition, tag_nsmap=None, batch_size=2)
-        tags.get_cached_knowledge.assert_called_once_with()
+            tag_name, tag_definition, tag_nsmap=None, client=client,
+            nodegroup_uuid=uuid, batch_size=2)
         tags.get_nodes_for_node_group.assert_called_once_with(client, uuid)
         self.assertEqual([((client, uuid, ['a', 'c']), {})], fake_first.calls)
         self.assertEqual([((client, uuid, ['b']), {})], fake_second.calls)
 
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2014-09-15 14:28:28 +0000
+++ src/provisioningserver/tests/test_tasks.py	2014-09-23 09:14:01 +0000
@@ -37,7 +37,6 @@
     auth,
     boot_images,
     cache,
-    tags,
     tasks,
     )
 from provisioningserver.boot import tftppath
@@ -52,15 +51,12 @@
     DNSForwardZoneConfig,
     DNSReverseZoneConfig,
     )
-from provisioningserver.tags import MissingCredentials
 from provisioningserver.tasks import (
     refresh_secrets,
     report_boot_images,
     rndc_command,
     RNDC_COMMAND_MAX_RETRY,
     setup_rndc_configuration,
-    update_node_tags,
-    UPDATE_NODE_TAGS_MAX_RETRY,
     write_dns_config,
     write_dns_zone_config,
     write_full_dns_config,
@@ -348,44 +344,3 @@
 
         args, kwargs = MAASClient.post.call_args
         self.assertItemsEqual([image], json.loads(kwargs['images']))
-
-
-class TestTagTasks(PservTestCase):
-
-    def setUp(self):
-        super(TestTagTasks, self).setUp()
-        self.celery = self.useFixture(CeleryFixture())
-
-    def test_update_node_tags_can_be_retried(self):
-        self.set_secrets()
-        # The update_node_tags task can be retried.
-        # Simulate a temporary failure.
-        number_of_failures = UPDATE_NODE_TAGS_MAX_RETRY
-        raised_exception = MissingCredentials(
-            factory.make_name('exception'), random.randint(100, 200))
-        simulate_failures = MultiFakeMethod(
-            [FakeMethod(failure=raised_exception)] * number_of_failures +
-            [FakeMethod()])
-        self.patch(tags, 'process_node_tags', simulate_failures)
-        tag = factory.make_string()
-        result = update_node_tags.delay(
-            tag, '//node', tag_nsmap=None, retry=True)
-        assertTaskRetried(
-            self, result, UPDATE_NODE_TAGS_MAX_RETRY + 1,
-            'provisioningserver.tasks.update_node_tags')
-
-    def test_update_node_tags_is_retried_a_limited_number_of_times(self):
-        self.set_secrets()
-        # If we simulate UPDATE_NODE_TAGS_MAX_RETRY + 1 failures, the
-        # task fails.
-        number_of_failures = UPDATE_NODE_TAGS_MAX_RETRY + 1
-        raised_exception = MissingCredentials(
-            factory.make_name('exception'), random.randint(100, 200))
-        simulate_failures = MultiFakeMethod(
-            [FakeMethod(failure=raised_exception)] * number_of_failures +
-            [FakeMethod()])
-        self.patch(tags, 'process_node_tags', simulate_failures)
-        tag = factory.make_string()
-        self.assertRaises(
-            MissingCredentials, update_node_tags.delay, tag,
-            '//node', tag_nsmap=None, retry=True)