← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/preseed-formats-dance into lp:maas

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/preseed-formats-dance into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #974175 in MAAS: "Set MAAS_PRESEED on commissioning node"
  https://bugs.launchpad.net/maas/+bug/974175

For more details, see:
https://code.launchpad.net/~jtv/maas/preseed-formats-dance/+merge/101012

This was going to be my big branch to make preseeds work as desired, with commissioning nodes getting a different format of preseed than other nodes, and patching up all the uncompleted work from my last branch.

Luckily, thanks to the wonders of TDD, all it took was adding some tests and patching up test doubles to work more like the real code.  Yes, one small change in non-testing code but it's entirely cosmetic.

Oh yes, and most of the diff is just moving tests around.  A bunch of helper tests didn't really talk to the pserv API at all and so didn't need to be in tests that could be retargeted against a real pserv etc.
-- 
https://code.launchpad.net/~jtv/maas/preseed-formats-dance/+merge/101012
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/preseed-formats-dance into lp:maas.
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	2012-04-05 17:01:14 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-04-05 17:49:25 +0000
@@ -12,6 +12,7 @@
 __all__ = []
 
 from abc import ABCMeta
+from base64 import b64decode
 from xmlrpclib import Fault
 
 from django.conf import settings
@@ -26,6 +27,8 @@
     NODE_STATUS_CHOICES,
     )
 from maasserver.provisioning import (
+    compose_cloud_init_preseed,
+    compose_commissioning_preseed,
     compose_preseed,
     get_metadata_server_url,
     name_arch_in_cobbler_style,
@@ -49,6 +52,120 @@
 import yaml
 
 
+class TestHelpers(TestCase):
+    """Tests for helpers that don't actually need any kind of pserv."""
+
+    def test_metadata_server_url_refers_to_own_metadata_service(self):
+        self.assertEqual(
+            "%s/metadata/"
+            % Config.objects.get_config('maas_url').rstrip('/'),
+            get_metadata_server_url())
+
+    def test_metadata_server_url_includes_script_name(self):
+        self.patch(settings, "FORCE_SCRIPT_NAME", "/MAAS")
+        self.assertEqual(
+            "%s/MAAS/metadata/"
+            % Config.objects.get_config('maas_url').rstrip('/'),
+            get_metadata_server_url())
+
+    def test_compose_preseed_for_commissioning_node_produces_yaml(self):
+        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
+        preseed = yaml.load(compose_preseed(node))
+        self.assertIn('datasource', preseed)
+        self.assertIn('MAAS', preseed['datasource'])
+        self.assertThat(
+            preseed['datasource']['MAAS'],
+            KeysEqual(
+                'metadata_url', 'consumer_key', 'token_key', 'token_secret'))
+
+    def test_compose_preseed_includes_metadata_url(self):
+        node = factory.make_node(status=NODE_STATUS.READY)
+        self.assertIn(get_metadata_server_url(), compose_preseed(node))
+
+    def test_compose_preseed_for_commissioning_includes_metadata_url(self):
+        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
+        preseed = yaml.load(compose_preseed(node))
+        self.assertEqual(
+            get_metadata_server_url(),
+            preseed['datasource']['MAAS']['metadata_url'])
+
+    def test_compose_preseed_includes_node_oauth_token(self):
+        node = factory.make_node(status=NODE_STATUS.READY)
+        preseed = compose_preseed(node)
+        token = NodeKey.objects.get_token_for_node(node)
+        self.assertIn('oauth_consumer_key=%s' % token.consumer.key, preseed)
+        self.assertIn('oauth_token_key=%s' % token.key, preseed)
+        self.assertIn('oauth_token_secret=%s' % token.secret, preseed)
+
+    def test_compose_preseed_for_commissioning_includes_auth_token(self):
+        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
+        preseed = yaml.load(compose_preseed(node))
+        maas_dict = preseed['datasource']['MAAS']
+        token = NodeKey.objects.get_token_for_node(node)
+        self.assertEqual(token.consumer.key, maas_dict['consumer_key'])
+        self.assertEqual(token.key, maas_dict['token_key'])
+        self.assertEqual(token.secret, maas_dict['token_secret'])
+
+    def test_present_user_friendly_fault_describes_pserv_fault(self):
+        self.assertIn(
+            "provisioning server",
+            present_user_friendly_fault(Fault(8002, 'error')).message)
+
+    def test_present_user_friendly_fault_covers_all_pserv_faults(self):
+        all_pserv_faults = set(map_enum(PSERV_FAULT).values())
+        presentable_pserv_faults = set(PRESENTATIONS.keys())
+        self.assertItemsEqual([], all_pserv_faults - presentable_pserv_faults)
+
+    def test_present_user_friendly_fault_rerepresents_all_pserv_faults(self):
+        fault_string = factory.getRandomString()
+        for fault_code in map_enum(PSERV_FAULT).values():
+            original_fault = Fault(fault_code, fault_string)
+            new_fault = present_user_friendly_fault(original_fault)
+            self.assertNotEqual(fault_string, new_fault.message)
+
+    def test_present_user_friendly_fault_describes_cobbler_fault(self):
+        friendly_fault = present_user_friendly_fault(
+            Fault(PSERV_FAULT.NO_COBBLER, factory.getRandomString()))
+        friendly_text = friendly_fault.message
+        self.assertIn("unable to reach", friendly_text)
+        self.assertIn("Cobbler", friendly_text)
+
+    def test_present_user_friendly_fault_describes_cobbler_auth_fail(self):
+        friendly_fault = present_user_friendly_fault(
+            Fault(PSERV_FAULT.COBBLER_AUTH_FAILED, factory.getRandomString()))
+        friendly_text = friendly_fault.message
+        self.assertIn("failed to authenticate", friendly_text)
+        self.assertIn("Cobbler", friendly_text)
+
+    def test_present_user_friendly_fault_describes_cobbler_auth_error(self):
+        friendly_fault = present_user_friendly_fault(
+            Fault(PSERV_FAULT.COBBLER_AUTH_ERROR, factory.getRandomString()))
+        friendly_text = friendly_fault.message
+        self.assertIn("authentication token", friendly_text)
+        self.assertIn("Cobbler", friendly_text)
+
+    def test_present_user_friendly_fault_describes_missing_profile(self):
+        profile = factory.getRandomString()
+        friendly_fault = present_user_friendly_fault(
+            Fault(
+                PSERV_FAULT.NO_SUCH_PROFILE,
+                "invalid profile name: %s" % profile))
+        friendly_text = friendly_fault.message
+        self.assertIn(profile, friendly_text)
+        self.assertIn("maas-import-isos", friendly_text)
+
+    def test_present_user_friendly_fault_describes_generic_cobbler_fail(self):
+        error_text = factory.getRandomString()
+        friendly_fault = present_user_friendly_fault(
+            Fault(PSERV_FAULT.GENERIC_COBBLER_ERROR, error_text))
+        friendly_text = friendly_fault.message
+        self.assertIn("Cobbler", friendly_text)
+        self.assertIn(error_text, friendly_text)
+
+    def test_present_user_friendly_fault_returns_None_for_other_fault(self):
+        self.assertIsNone(present_user_friendly_fault(Fault(9999, "!!!")))
+
+
 class ProvisioningTests:
     """Tests for the Provisioning API as maasserver sees it."""
 
@@ -218,57 +335,6 @@
         node = self.papi.get_nodes_by_name(["frank"])["frank"]
         self.assertEqual([], node["mac_addresses"])
 
-    def test_metadata_server_url_refers_to_own_metadata_service(self):
-        self.assertEqual(
-            "%s/metadata/"
-            % Config.objects.get_config('maas_url').rstrip('/'),
-            get_metadata_server_url())
-
-    def test_metadata_server_url_includes_script_name(self):
-        self.patch(settings, "FORCE_SCRIPT_NAME", "/MAAS")
-        self.assertEqual(
-            "%s/MAAS/metadata/"
-            % Config.objects.get_config('maas_url').rstrip('/'),
-            get_metadata_server_url())
-
-    def test_compose_preseed_for_commissioning_node_produces_yaml(self):
-        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
-        preseed = yaml.load(compose_preseed(node))
-        self.assertIn('datasource', preseed)
-        self.assertIn('MAAS', preseed['datasource'])
-        self.assertThat(
-            preseed['datasource']['MAAS'],
-            KeysEqual(
-                'metadata_url', 'consumer_key', 'token_key', 'token_secret'))
-
-    def test_compose_preseed_includes_metadata_url(self):
-        node = factory.make_node(status=NODE_STATUS.READY)
-        self.assertIn(get_metadata_server_url(), compose_preseed(node))
-
-    def test_compose_preseed_for_commissioning_includes_metadata_url(self):
-        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
-        preseed = yaml.load(compose_preseed(node))
-        self.assertEqual(
-            get_metadata_server_url(),
-            preseed['datasource']['MAAS']['metadata_url'])
-
-    def test_compose_preseed_includes_node_oauth_token(self):
-        node = factory.make_node(status=NODE_STATUS.READY)
-        preseed = compose_preseed(node)
-        token = NodeKey.objects.get_token_for_node(node)
-        self.assertIn('oauth_consumer_key=%s' % token.consumer.key, preseed)
-        self.assertIn('oauth_token_key=%s' % token.key, preseed)
-        self.assertIn('oauth_token_secret=%s' % token.secret, preseed)
-
-    def test_compose_preseed_for_commissioning_includes_auth_token(self):
-        node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
-        preseed = yaml.load(compose_preseed(node))
-        maas_dict = preseed['datasource']['MAAS']
-        token = NodeKey.objects.get_token_for_node(node)
-        self.assertEqual(token.consumer.key, maas_dict['consumer_key'])
-        self.assertEqual(token.key, maas_dict['token_key'])
-        self.assertEqual(token.secret, maas_dict['token_secret'])
-
     def test_papi_xmlrpc_faults_are_reported_helpfully(self):
 
         def raise_fault(*args, **kwargs):
@@ -289,65 +355,6 @@
         with ExpectedException(MAASAPIException, ".*Cobbler.*"):
             self.papi.add_node('node', 'profile', 'power', '')
 
-    def test_present_user_friendly_fault_describes_pserv_fault(self):
-        self.assertIn(
-            "provisioning server",
-            present_user_friendly_fault(Fault(8002, 'error')).message)
-
-    def test_present_user_friendly_fault_covers_all_pserv_faults(self):
-        all_pserv_faults = set(map_enum(PSERV_FAULT).values())
-        presentable_pserv_faults = set(PRESENTATIONS.keys())
-        self.assertItemsEqual([], all_pserv_faults - presentable_pserv_faults)
-
-    def test_present_user_friendly_fault_rerepresents_all_pserv_faults(self):
-        fault_string = factory.getRandomString()
-        for fault_code in map_enum(PSERV_FAULT).values():
-            original_fault = Fault(fault_code, fault_string)
-            new_fault = present_user_friendly_fault(original_fault)
-            self.assertNotEqual(fault_string, new_fault.message)
-
-    def test_present_user_friendly_fault_describes_cobbler_fault(self):
-        friendly_fault = present_user_friendly_fault(
-            Fault(PSERV_FAULT.NO_COBBLER, factory.getRandomString()))
-        friendly_text = friendly_fault.message
-        self.assertIn("unable to reach", friendly_text)
-        self.assertIn("Cobbler", friendly_text)
-
-    def test_present_user_friendly_fault_describes_cobbler_auth_fail(self):
-        friendly_fault = present_user_friendly_fault(
-            Fault(PSERV_FAULT.COBBLER_AUTH_FAILED, factory.getRandomString()))
-        friendly_text = friendly_fault.message
-        self.assertIn("failed to authenticate", friendly_text)
-        self.assertIn("Cobbler", friendly_text)
-
-    def test_present_user_friendly_fault_describes_cobbler_auth_error(self):
-        friendly_fault = present_user_friendly_fault(
-            Fault(PSERV_FAULT.COBBLER_AUTH_ERROR, factory.getRandomString()))
-        friendly_text = friendly_fault.message
-        self.assertIn("authentication token", friendly_text)
-        self.assertIn("Cobbler", friendly_text)
-
-    def test_present_user_friendly_fault_describes_missing_profile(self):
-        profile = factory.getRandomString()
-        friendly_fault = present_user_friendly_fault(
-            Fault(
-                PSERV_FAULT.NO_SUCH_PROFILE,
-                "invalid profile name: %s" % profile))
-        friendly_text = friendly_fault.message
-        self.assertIn(profile, friendly_text)
-        self.assertIn("maas-import-isos", friendly_text)
-
-    def test_present_user_friendly_fault_describes_generic_cobbler_fail(self):
-        error_text = factory.getRandomString()
-        friendly_fault = present_user_friendly_fault(
-            Fault(PSERV_FAULT.GENERIC_COBBLER_ERROR, error_text))
-        friendly_text = friendly_fault.message
-        self.assertIn("Cobbler", friendly_text)
-        self.assertIn(error_text, friendly_text)
-
-    def test_present_user_friendly_fault_returns_None_for_other_fault(self):
-        self.assertIsNone(present_user_friendly_fault(Fault(9999, "!!!")))
-
 
 class TestProvisioningWithFake(ProvisioningTests, ProvisioningFakeFactory,
                                TestCase):
@@ -389,3 +396,28 @@
             for status, pserv_node in pserv_nodes.items()
             }
         self.assertEqual(expected, observed)
+
+    def test_commissioning_node_gets_commissioning_preseed(self):
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        token = NodeKey.objects.get_token_for_node(node)
+        node.start_commissioning(factory.make_admin())
+        preseed = self.papi.nodes[node.system_id]['ks_meta']['MAAS_PRESEED']
+        self.assertEqual(
+            compose_commissioning_preseed(token), b64decode(preseed))
+
+    def test_non_commissioning_node_gets_cloud_init_preseed(self):
+        node = factory.make_node(status=NODE_STATUS.READY)
+        token = NodeKey.objects.get_token_for_node(node)
+        preseed = self.papi.nodes[node.system_id]['ks_meta']['MAAS_PRESEED']
+        self.assertEqual(
+            compose_cloud_init_preseed(token), b64decode(preseed))
+
+    def test_node_gets_cloud_init_preseed_after_commissioning(self):
+        node = factory.make_node(status=NODE_STATUS.DECLARED)
+        token = NodeKey.objects.get_token_for_node(node)
+        node.start_commissioning(factory.make_admin())
+        node.status = NODE_STATUS.READY
+        node.save()
+        preseed = self.papi.nodes[node.system_id]['ks_meta']['MAAS_PRESEED']
+        self.assertEqual(
+            compose_cloud_init_preseed(token), b64decode(preseed))

=== modified file 'src/provisioningserver/api.py'
--- src/provisioningserver/api.py	2012-04-05 16:10:17 +0000
+++ src/provisioningserver/api.py	2012-04-05 17:49:25 +0000
@@ -197,11 +197,10 @@
         assert isinstance(profile, basestring)
         assert power_type in (POWER_TYPE.VIRSH, POWER_TYPE.WAKE_ON_LAN)
         assert isinstance(preseed_data, basestring)
-        preseed = b64encode(preseed_data)
         attributes = {
             "hostname": hostname,
             "profile": profile,
-            "ks_meta": {"MAAS_PRESEED": preseed},
+            "ks_meta": {"MAAS_PRESEED": b64encode(preseed_data)},
             "power_type": power_type,
             }
         system = yield CobblerSystem.new(self.session, name, attributes)

=== modified file 'src/provisioningserver/testing/fakeapi.py'
--- src/provisioningserver/testing/fakeapi.py	2012-04-05 16:10:17 +0000
+++ src/provisioningserver/testing/fakeapi.py	2012-04-05 17:49:25 +0000
@@ -22,6 +22,7 @@
     "FakeSynchronousProvisioningAPI",
     ]
 
+from base64 import b64encode
 from functools import wraps
 
 from provisioningserver.interfaces import IProvisioningAPI
@@ -92,7 +93,9 @@
         self.nodes[name]["hostname"] = hostname
         self.nodes[name]["profile"] = profile
         self.nodes[name]["mac_addresses"] = []
-        self.nodes[name]["preseed_data"] = preseed_data
+        self.nodes[name]["ks_meta"] = {
+            "MAAS_PRESEED": b64encode(preseed_data),
+            }
         self.nodes[name]["power_type"] = power_type
         return name
 


Follow ups