launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/maas-pserv-add-node into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/maas-pserv-add-node/+merge/92446
-- 
https://code.launchpad.net/~allenap/maas/maas-pserv-add-node/+merge/92446
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/maas-pserv-add-node into lp:maas.

=== modified file 'Makefile'
--- Makefile	2012-02-01 12:39:54 +0000
+++ Makefile	2012-02-10 11:36:18 +0000
@@ -46,7 +46,7 @@
 lint: sources = setup.py src templates utilities
 lint: bin/flake8
 	@bin/flake8 $(sources) | \
-	    (! fgrep -v "from maas.settings import *")
+	    (! egrep -v "from maas[.](settings|development) import [*]")
 
 check: clean test
 
@@ -72,8 +72,16 @@
 	$(RM) docs/api.rst
 	$(RM) -r docs/_build/
 
-run: bin/maas dev-db
-	bin/maas runserver 8000
+pserv.pid: bin/twistd.pserv
+	bin/twistd.pserv --pidfile=$@ maas-pserv --port=8001
+
+pserv-start: pserv.pid
+
+pserv-stop:
+	{ test -e pserv.pid && cat pserv.pid; } | xargs --no-run-if-empty kill
+
+run: bin/maas dev-db pserv.pid
+	bin/maas runserver 8000 --settings=maas.demo
 
 harness: bin/maas dev-db
 	bin/maas shell
@@ -83,4 +91,5 @@
 
 .PHONY: \
     build check clean dev-db distclean doc \
-    harness lint run syncdb test sampledata
+    harness lint pserv-start pserv-stop run \
+    syncdb test sampledata

=== added file 'src/maas/demo.py'
--- src/maas/demo.py	1970-01-01 00:00:00 +0000
+++ src/maas/demo.py	2012-02-10 11:36:18 +0000
@@ -0,0 +1,18 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Django DEMO settings for maas project."""
+
+from __future__ import (
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+
+from maas.development import *
+from maas.settings import *
+
+
+# This should match the setting in Makefile:pserv.pid.
+PSERV_URL = "http://localhost:8001/api";

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-02-09 07:43:25 +0000
+++ src/maas/settings.py	2012-02-10 11:36:18 +0000
@@ -201,3 +201,7 @@
         },
     }
 }
+
+# The location of the Provisioning API XML-RPC endpoint. If PSERV_URL is None,
+# use the fake Provisioning API.
+PSERV_URL = None

=== modified file 'src/maasserver/__init__.py'
--- src/maasserver/__init__.py	2012-02-08 14:21:17 +0000
+++ src/maasserver/__init__.py	2012-02-10 11:36:18 +0000
@@ -0,0 +1,18 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""MaaS Server application."""
+
+from __future__ import (
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver import provisioning
+
+# This has been imported so that it can register its signal handlers early on,
+# before it misses anything. (Mentioned below to silence lint warnings.)
+provisioning

=== added file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/provisioning.py	2012-02-10 11:36:18 +0000
@@ -0,0 +1,78 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interact with the Provisioning API."""
+
+from __future__ import (
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from uuid import uuid1
+import xmlrpclib
+
+from django.conf import settings
+from django.db.models.signals import (
+    post_delete,
+    post_save,
+    )
+from django.dispatch import receiver
+from maasserver.models import (
+    MACAddress,
+    Node,
+    )
+
+
+def get_provisioning_api_proxy():
+    """Return a proxy to the Provisioning API."""
+    # FIXME: This is a little ugly.
+    url = settings.PSERV_URL
+    if url is None:
+        from provisioningserver.testing.fakeapi import (
+            FakeSynchronousProvisioningAPI,
+            )
+        return FakeSynchronousProvisioningAPI()
+    else:
+        return xmlrpclib.ServerProxy(
+            url, allow_none=True, use_datetime=True)
+
+
+@receiver(post_save, sender=Node)
+def provision_post_save_Node(sender, instance, created, **kwargs):
+    """Create or update nodes in the provisioning server."""
+    # Create or update the node in the provisioning prov.
+    papi = get_provisioning_api_proxy()
+    nodes = papi.get_nodes_by_name([instance.system_id])
+    if instance.system_id in nodes:
+        profile = nodes[instance.system_id]["profile"]
+    else:
+        # TODO: Get these from somewhere.
+        distro = papi.add_distro(
+            "distro-%s" % uuid1().get_hex(),
+            "initrd", "kernel")
+        profile = papi.add_profile(
+            "profile-%s" % uuid1().get_hex(),
+            distro)
+    papi.add_node(instance.system_id, profile)
+
+
+@receiver(post_save, sender=MACAddress)
+def provision_post_save_MACAddress(sender, instance, created, **kwargs):
+    """Create or update MACs in the provisioning server."""
+    # TODO
+
+
+@receiver(post_delete, sender=Node)
+def provision_post_delete_Node(sender, instance, **kwargs):
+    """Delete nodes in the provisioning server."""
+    papi = get_provisioning_api_proxy()
+    papi.delete_nodes_by_name([instance.system_id])
+
+
+@receiver(post_delete, sender=MACAddress)
+def provision_post_delete_MACAddress(sender, instance, **kwargs):
+    """Delete MACs in the provisioning server."""
+    # TODO

=== added file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-02-10 11:36:18 +0000
@@ -0,0 +1,86 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `maasserver.provisioning`."""
+
+from __future__ import (
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from fixtures import MonkeyPatch
+from maasserver import provisioning
+from maasserver.models import Node
+from maastesting import TestCase
+from provisioningserver.testing.fakeapi import FakeSynchronousProvisioningAPI
+
+
+class TestProvisioning(TestCase):
+
+    def patch_in_fake_papi(self):
+        papi_fake = FakeSynchronousProvisioningAPI()
+        patch = MonkeyPatch(
+            "maasserver.provisioning.get_provisioning_api_proxy",
+            lambda: papi_fake)
+        self.useFixture(patch)
+        return papi_fake
+
+    def test_patch_in_fake_papi(self):
+        # patch_in_fake_papi() patches in a fake provisioning API so that we
+        # can observe what the signal handlers are doing.
+        papi = provisioning.get_provisioning_api_proxy()
+        papi_fake = self.patch_in_fake_papi()
+        self.assertIsNot(provisioning.get_provisioning_api_proxy(), papi)
+        self.assertIs(provisioning.get_provisioning_api_proxy(), papi_fake)
+        # The fake has small database, and it's empty to begin with.
+        self.assertEqual({}, papi_fake.distros)
+        self.assertEqual({}, papi_fake.profiles)
+        self.assertEqual({}, papi_fake.nodes)
+
+    def test_provision_post_save_Node_create(self):
+        # Creating and saving a node automatically creates a dummy distro and
+        # profile too, and associates it with the new node.
+        papi_fake = self.patch_in_fake_papi()
+        node_model = Node(system_id="frank")
+        provisioning.provision_post_save_Node(
+            sender=Node, instance=node_model, created=True)
+        self.assertEqual(["frank"], sorted(papi_fake.nodes))
+        node = papi_fake.nodes["frank"]
+        profile_name = node["profile"]
+        self.assertIn(profile_name, papi_fake.profiles)
+        profile = papi_fake.profiles[profile_name]
+        distro_name = profile["distro"]
+        self.assertIn(distro_name, papi_fake.distros)
+
+    def test_provision_post_save_Node_update(self):
+        # Saving an existing node does not change the profile or distro
+        # associated with it.
+        papi_fake = self.patch_in_fake_papi()
+        node_model = Node(system_id="frank")
+        provisioning.provision_post_save_Node(
+            sender=Node, instance=node_model, created=True)
+        # Record the current profile name.
+        node = papi_fake.nodes["frank"]
+        profile_name1 = node["profile"]
+        # Update the model node.
+        provisioning.provision_post_save_Node(
+            sender=Node, instance=node_model, created=False)
+        # The profile name is unchanged.
+        node = papi_fake.nodes["frank"]
+        profile_name2 = node["profile"]
+        self.assertEqual(profile_name1, profile_name2)
+
+    def test_provision_post_delete_Node(self):
+        papi_fake = self.patch_in_fake_papi()
+        node_model = Node(system_id="frank")
+        provisioning.provision_post_save_Node(
+            sender=Node, instance=node_model, created=True)
+        provisioning.provision_post_delete_Node(
+            sender=Node, instance=node_model)
+        # The node is deleted, but the profile and distro remain.
+        self.assertNotEqual({}, papi_fake.distros)
+        self.assertNotEqual({}, papi_fake.profiles)
+        self.assertEqual({}, papi_fake.nodes)

=== modified file 'src/provisioningserver/testing/fakeapi.py'
--- src/provisioningserver/testing/fakeapi.py	2012-02-09 12:35:21 +0000
+++ src/provisioningserver/testing/fakeapi.py	2012-02-10 11:36:18 +0000
@@ -27,6 +27,7 @@
 from provisioningserver.interfaces import IProvisioningAPI
 from twisted.internet import defer
 from zope.interface import implementer
+from zope.interface.interface import Method
 
 
 class FakeProvisioningDatabase(dict):
@@ -65,6 +66,8 @@
 @implementer(IProvisioningAPI)
 class FakeSynchronousProvisioningAPI:
 
+    # TODO: Referential integrity might be a nice thing.
+
     def __init__(self):
         super(FakeSynchronousProvisioningAPI, self).__init__()
         self.distros = FakeProvisioningDatabase()
@@ -125,4 +128,5 @@
     b"FakeAsynchronousProvisioningAPI", (FakeSynchronousProvisioningAPI,), {
         name: async(getattr(FakeSynchronousProvisioningAPI, name))
         for name in IProvisioningAPI.names(all=True)
+        if isinstance(IProvisioningAPI[name], Method)
         })

=== modified file 'src/provisioningserver/testing/fakecobbler.py'
--- src/provisioningserver/testing/fakecobbler.py	2012-02-08 21:31:17 +0000
+++ src/provisioningserver/testing/fakecobbler.py	2012-02-10 11:36:18 +0000
@@ -208,7 +208,7 @@
 
     def _api_get_object(self, object_type, name):
         """Get object's attributes by name."""
-        location = self.store[None][object_type]
+        location = self.store[None].get(object_type, {})
         matches = [obj for obj in location.values() if obj['name'] == name]
         assert len(matches) <= 1, (
             "Multiple %s objects are called '%s'." % (object_type, name))

=== modified file 'src/provisioningserver/tests/test_fakecobbler.py'
--- src/provisioningserver/tests/test_fakecobbler.py	2012-02-03 10:58:38 +0000
+++ src/provisioningserver/tests/test_fakecobbler.py	2012-02-10 11:36:18 +0000
@@ -282,6 +282,13 @@
         self.assertEqual(comment, found_obj['comment'])
 
     @inlineCallbacks
+    def test_get_values_returns_None_for_non_existent_object(self):
+        session = yield fake_cobbler_session()
+        name = self.make_name()
+        values = yield self.cobbler_class(session, name).get_values()
+        self.assertIsNone(values)
+
+    @inlineCallbacks
     def test_get_handle_finds_handle(self):
         session = yield fake_cobbler_session()
         name = self.make_name()