launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/raise-missing-profile into lp:maas with lp:~jtv/maas/cobbler-exception as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/raise-missing-profile/+merge/98151

When maasserver wants a node created, but pserv reports that Cobbler doesn't know the required profile, maasserver should explain to the user that this is likely to indicate a problem with the maas-import-isos script.

Arguably perhaps this should have been done in pserv.  One the one hand, that isolates Cobbler concerns more nicely.  On the other hand, this is a user-facing message about integration between the application per se and one of its support scripts that run separately.  As such I do feel it belongs in maasserver.
-- 
https://code.launchpad.net/~jtv/maas/raise-missing-profile/+merge/98151
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/raise-missing-profile into lp:maas.

=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2012-03-15 13:58:32 +0000
+++ src/maasserver/exceptions.py	2012-03-19 06:10:23 +0000
@@ -30,6 +30,10 @@
     """User can't be deleted."""
 
 
+class MissingProfileException(MAASException):
+    """System profile does not exist."""
+
+
 class MAASAPIException(Exception):
     """Base class for MAAS' API exceptions.
 

=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	2012-03-16 18:35:02 +0000
+++ src/maasserver/provisioning.py	2012-03-19 06:10:23 +0000
@@ -13,6 +13,7 @@
     'get_provisioning_api_proxy',
     ]
 
+from textwrap import dedent
 from urllib import urlencode
 from urlparse import urljoin
 import warnings
@@ -24,11 +25,13 @@
     post_save,
     )
 from django.dispatch import receiver
+from maasserver.exceptions import MissingProfileException
 from maasserver.models import (
     Config,
     MACAddress,
     Node,
     )
+from provisioningserver.enum import PSERV_FAULT
 
 
 def get_provisioning_api_proxy():
@@ -116,7 +119,18 @@
     profile = select_profile_for_node(instance, papi)
     power_type = instance.get_effective_power_type()
     metadata = compose_metadata(instance)
-    papi.add_node(instance.system_id, profile, power_type, metadata)
+    try:
+        papi.add_node(instance.system_id, profile, power_type, metadata)
+    except xmlrpclib.Fault as e:
+        if e.faultCode == PSERV_FAULT.NO_SUCH_PROFILE:
+            raise MissingProfileException(dedent("""
+                System profile %s does not exist.  Has the maas-import-isos
+                script been run?  This will run automatically from time to
+                time, but if it is failing, an administrator may need to run
+                it manually.
+                """ % profile).lstrip('\n'))
+        else:
+            raise
 
 
 def set_node_mac_addresses(node):

=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	2012-03-16 18:35:02 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-03-19 06:10:23 +0000
@@ -12,8 +12,10 @@
 __all__ = []
 
 from urlparse import parse_qs
+from xmlrpclib import Fault
 
 from maasserver import provisioning
+from maasserver.exceptions import MissingProfileException
 from maasserver.models import (
     ARCHITECTURE,
     Config,
@@ -31,7 +33,11 @@
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from metadataserver.models import NodeKey
-from provisioningserver.enum import POWER_TYPE
+from provisioningserver.enum import (
+    POWER_TYPE,
+    PSERV_FAULT,
+    )
+from testtools.testcase import ExpectedException
 
 
 class ProvisioningTests:
@@ -112,6 +118,33 @@
         pserv_node = self.papi.get_nodes_by_name([system_id])[system_id]
         self.assertEqual("maas-precise-i386", pserv_node["profile"])
 
+    def test_provision_post_save_Node_checks_for_missing_profile(self):
+        # If the required profile for a node is missing, MaaS reports
+        # that the maas-import-isos script may need running.
+
+        def raise_missing_profile(*args, **kwargs):
+            raise Fault(PSERV_FAULT.NO_SUCH_PROFILE, "Unknown profile.")
+
+        self.papi.add_node = raise_missing_profile
+        expectation = ExpectedException(
+            MissingProfileException, value_re='.*maas-import-isos.*')
+        with expectation:
+            node = factory.make_node(architecture='amd32k')
+            provisioning.provision_post_save_Node(
+                sender=Node, instance=node, created=True)
+
+    def test_provision_post_save_Node_passes_on_other_pserv_faults(self):
+        error_text = factory.getRandomString()
+
+        def raise_fault(*args, **kwargs):
+            raise Fault(PSERV_FAULT.NO_COBBLER, error_text)
+
+        self.papi.add_node = raise_fault
+        with ExpectedException(Fault, ".*%s.*" % error_text):
+            node = factory.make_node(architecture='amd32k')
+            provisioning.provision_post_save_Node(
+                sender=Node, instance=node, created=True)
+
     def test_provision_post_save_Node_registers_effective_power_type(self):
         power_types = list(map_enum(POWER_TYPE).values())
         nodes = {