launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11023
[Merge] lp:~jtv/maas/remove-pserv-fault into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/remove-pserv-fault into lp:maas with lp:~jtv/maas/remove-use-real-pserv as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/remove-pserv-fault/+merge/119876
The PSERV_FAULT enum served a situation where we spoke to the provisioning server over XMLRPC, and it in turn spoke to Cobbler over XMLRPC. Both are gone now, so I'm removing PSERV_FAULT altogether. This greatly reduces the number of mentions of Cobbler in the tree, which is one of the things I'm going by in removing Cobbler.
The view mechanism for displaying persistent errors driven by PSERV_FAULT remains untouched. We may want to drive that from some other error-detection mechanism. I hear there is talk of replacing it altogether, but that's not my concern right now.
Jeroen
--
https://code.launchpad.net/~jtv/maas/remove-pserv-fault/+merge/119876
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/remove-pserv-fault into lp:maas.
=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py 2012-08-16 09:53:25 +0000
+++ src/maasserver/provisioning.py 2012-08-16 09:53:25 +0000
@@ -11,257 +11,15 @@
__metaclass__ = type
__all__ = [
- 'present_detailed_user_friendly_fault',
- 'ProvisioningProxy',
+ 'compose_preseed',
]
-from functools import partial
-from textwrap import dedent
from urllib import urlencode
-import xmlrpclib
-from maasserver.components import (
- COMPONENT,
- discard_persistent_error,
- register_persistent_error,
- )
from maasserver.enum import NODE_STATUS
-from maasserver.exceptions import ExternalComponentException
from maasserver.utils import absolute_reverse
-from provisioningserver.enum import PSERV_FAULT
import yaml
-# Presentation templates for various provisioning faults (will be used
-# for long-lasting warnings about failing components).
-DETAILED_PRESENTATIONS = {
- PSERV_FAULT.NO_COBBLER: """
- The provisioning server was unable to reach the Cobbler service:
- %(fault_string)s
-
- Check pserv.log, and restart MAAS if needed.
- """,
- PSERV_FAULT.COBBLER_AUTH_FAILED: """
- The provisioning server failed to authenticate with the Cobbler
- service: %(fault_string)s.
-
- This may mean that Cobbler's authentication configuration has
- changed. Check /var/log/cobbler/ and pserv.log.
- """,
- PSERV_FAULT.COBBLER_AUTH_ERROR: """
- The Cobbler server no longer accepts the provisioning server's
- authentication token. This should not happen; it may indicate
- that the server is under unsustainable load.
- """,
- PSERV_FAULT.NO_SUCH_PROFILE: """
- System profile does not exist: %(fault_string)s.
-
- This is a Cobbler error and should no longer occur now that Cobbler
- has been removed as a component of MAAS.
- """,
- PSERV_FAULT.GENERIC_COBBLER_ERROR: """
- The provisioning service encountered a problem with the Cobbler
- server, fault code %(fault_code)s: %(fault_string)s
-
- If the error message is not clear, you may need to check the
- Cobbler logs in /var/log/cobbler/ or pserv.log.
- """,
- PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR: """
- The provisioning server was unable to resolve the Cobbler server's
- DNS address: %(fault_string)s.
-
- Has Cobbler been properly installed and is it accessible by the
- provisioning server? Check /var/log/cobbler/ and pserv.log.
- """,
- 8002: """
- Unable to reach provisioning server (%(fault_string)s).
- """,
-}
-
-# Shorter presentation templates for various provisioning faults (will
-# be used for one-off messages).
-SHORT_PRESENTATIONS = {
- PSERV_FAULT.NO_COBBLER: """
- Unable to reach the Cobbler server.
- """,
- PSERV_FAULT.COBBLER_AUTH_FAILED: """
- Failed to authenticate with the Cobbler server.
- """,
- PSERV_FAULT.COBBLER_AUTH_ERROR: """
- Failed to authenticate with the Cobbler server.
- """,
- PSERV_FAULT.NO_SUCH_PROFILE: """
- Missing system profile: %(fault_string)s.
- """,
- PSERV_FAULT.GENERIC_COBBLER_ERROR: """
- Unknown problem encountered with the Cobbler server.
- """,
- PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR: """
- Unable to resolve the Cobbler server's DNS address:
- %(fault_string)s.
- """,
- 8002: """
- Unable to reach provisioning server.
- """,
-}
-
-
-def _present_user_friendly_fault(fault, presentations):
- """Return a more user-friendly exception to represent `fault`.
-
- :param fault: An exception raised by, or received across, xmlrpc.
- :type fault: :class:`xmlrpclib.Fault`
- :param presentations: A mapping error -> message.
- :type fault: dict
- :return: A more user-friendly exception, if one can be produced.
- Otherwise, this returns None and the original exception should be
- re-raised. (This is left to the caller in order to minimize
- erosion of the backtrace).
- :rtype: :class:`MAASAPIException`, or None.
- """
- params = {
- 'fault_code': fault.faultCode,
- 'fault_string': fault.faultString,
- }
- user_friendly_text = presentations.get(fault.faultCode)
- if user_friendly_text is None:
- return None
- else:
- return ExternalComponentException(dedent(
- user_friendly_text.lstrip('\n') % params))
-
-
-present_user_friendly_fault = partial(
- _present_user_friendly_fault, presentations=SHORT_PRESENTATIONS)
-"""Return a concise but user-friendly exception to represent `fault`.
-
-:param fault: An exception raised by, or received across, xmlrpc.
-:type fault: :class:`xmlrpclib.Fault`
-:return: A more user-friendly exception, if one can be produced.
- Otherwise, this returns None and the original exception should be
- re-raised. (This is left to the caller in order to minimize
- erosion of the backtrace).
-:rtype: :class:`MAASAPIException`, or None.
-"""
-
-
-present_detailed_user_friendly_fault = partial(
- _present_user_friendly_fault, presentations=DETAILED_PRESENTATIONS)
-"""Return a detailed and user-friendly exception to represent `fault`.
-
-:param fault: An exception raised by, or received across, xmlrpc.
-:type fault: :class:`xmlrpclib.Fault`
-:return: A more user-friendly exception, if one can be produced.
- Otherwise, this returns None and the original exception should be
- re-raised. (This is left to the caller in order to minimize
- erosion of the backtrace).
-:rtype: :class:`MAASAPIException`, or None.
-"""
-
-# A mapping method_name -> list of components.
-# For each method name, indicate the list of components that the method
-# uses. This way, when calling the method is a success, if means that
-# the related components are working properly.
-METHOD_COMPONENTS = {
- 'add_node': [
- COMPONENT.PSERV,
- COMPONENT.COBBLER,
- COMPONENT.IMPORT_PXE_FILES,
- ],
- 'modify_nodes': [COMPONENT.PSERV, COMPONENT.COBBLER],
- 'delete_nodes_by_name': [COMPONENT.PSERV, COMPONENT.COBBLER],
- 'get_profiles_by_name': [COMPONENT.PSERV, COMPONENT.COBBLER],
-}
-
-# A mapping exception -> component.
-# For each exception in this dict, the related component is there to
-# tell us which component will be marked as 'failing' when this
-# exception is raised.
-EXCEPTIONS_COMPONENTS = {
- PSERV_FAULT.NO_COBBLER: COMPONENT.COBBLER,
- PSERV_FAULT.COBBLER_AUTH_FAILED: COMPONENT.COBBLER,
- PSERV_FAULT.COBBLER_AUTH_ERROR: COMPONENT.COBBLER,
- PSERV_FAULT.NO_SUCH_PROFILE: COMPONENT.IMPORT_PXE_FILES,
- PSERV_FAULT.GENERIC_COBBLER_ERROR: COMPONENT.COBBLER,
- PSERV_FAULT.COBBLER_DNS_LOOKUP_ERROR: COMPONENT.COBBLER,
- 8002: COMPONENT.PSERV,
-}
-
-
-def register_working_components(method_name):
- """Register that the components related to the provided method
- (if any) are working.
- """
- components = METHOD_COMPONENTS.get(method_name, [])
- for component in components:
- discard_persistent_error(component)
-
-
-def register_failing_component(exception):
- """Register that the component corresponding to exception (if any)
- is failing.
- """
- component = EXCEPTIONS_COMPONENTS.get(exception.faultCode, None)
- if component is not None:
- detailed_friendly_fault = unicode(
- present_detailed_user_friendly_fault(exception))
- register_persistent_error(component, detailed_friendly_fault)
-
-
-class ProvisioningCaller:
- """Wrapper for an XMLRPC call.
-
- - Runs xmlrpc exceptions through `present_user_friendly_fault` for better
- presentation to the user.
- - Registers failing/working components.
- """
-
- def __init__(self, method_name, method):
- # Keep track of the method name; xmlrpclib does not take lightly
- # to us attempting to look it up as an attribute of the method
- # object.
- self.method_name = method_name
- self.method = method
-
- def __call__(self, *args):
- try:
- result = self.method(*args)
- except xmlrpclib.Fault as e:
- # Register failing component.
- register_failing_component(e)
- # Raise a more user-friendly error.
- friendly_fault = present_user_friendly_fault(e)
- if friendly_fault is None:
- raise
- else:
- raise friendly_fault
- else:
- # The call was a success, discard persistent errors for
- # components referenced by this method.
- register_working_components(self.method_name)
- return result
-
-
-class ProvisioningProxy:
- """Proxy for calling the provisioning service.
-
- This wraps an XMLRPC :class:`ServerProxy`, but translates exceptions
- coming in from, or across, the xmlrpc mechanism into more helpful ones
- for the user.
- """
-
- def __init__(self, xmlrpc_proxy):
- self.proxy = xmlrpc_proxy
-
- def __getattr__(self, attribute_name):
- """Return a wrapped version of the requested method."""
- attribute = getattr(self.proxy, attribute_name)
- if callable(attribute):
- # This attribute is callable. Wrap it in a caller.
- return ProvisioningCaller(attribute_name, attribute)
- else:
- # This is a regular attribute. Return it as-is.
- return attribute
-
def compose_cloud_init_preseed(token):
"""Compose the preseed value for a node in any state but Commissioning."""
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py 2012-08-16 09:53:25 +0000
+++ src/maasserver/tests/test_provisioning.py 2012-08-16 09:53:25 +0000
@@ -12,24 +12,12 @@
__metaclass__ = type
__all__ = []
-from xmlrpclib import Fault
-
from maasserver.enum import NODE_STATUS
-from maasserver.provisioning import (
- compose_preseed,
- DETAILED_PRESENTATIONS,
- present_detailed_user_friendly_fault,
- present_user_friendly_fault,
- SHORT_PRESENTATIONS,
- )
+from maasserver.provisioning import compose_preseed
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
-from maasserver.utils import (
- absolute_reverse,
- map_enum,
- )
+from maasserver.utils import absolute_reverse
from metadataserver.models import NodeKey
-from provisioningserver.enum import PSERV_FAULT
from testtools.matchers import (
KeysEqual,
StartsWith,
@@ -81,118 +69,3 @@
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_detailed_user_friendly_fault_describes_pserv_fault(self):
- self.assertIn(
- "provisioning server",
- present_user_friendly_fault(Fault(8002, 'error')).message)
-
- def test_present_detailed_fault_covers_all_pserv_faults(self):
- all_pserv_faults = set(map_enum(PSERV_FAULT).values())
- presentable_pserv_faults = set(DETAILED_PRESENTATIONS.keys())
- self.assertItemsEqual([], all_pserv_faults - presentable_pserv_faults)
-
- def test_present_detailed_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_detailed_user_friendly_fault(original_fault)
- self.assertNotEqual(fault_string, new_fault.message)
-
- def test_present_detailed_fault_describes_cobbler_fault(self):
- friendly_fault = present_detailed_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_detailed_fault_describes_cobbler_auth_fail(self):
- friendly_fault = present_detailed_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_detailed_fault_describes_cobbler_auth_error(self):
- friendly_fault = present_detailed_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_detailed_fault_describes_missing_profile(self):
- profile = factory.getRandomString()
- friendly_fault = present_detailed_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("profile", friendly_text)
-
- def test_present_detailed_fault_describes_generic_cobbler_fail(self):
- error_text = factory.getRandomString()
- friendly_fault = present_detailed_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_detailed_fault_returns_None_for_other_fault(self):
- self.assertIsNone(
- present_detailed_user_friendly_fault(Fault(9999, "!!!")))
-
- 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(SHORT_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 the Cobbler server", 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 with the Cobbler server", 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(
- "Failed to authenticate with the Cobbler server", 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)
-
- 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(
- "Unknown problem encountered with the Cobbler server.",
- friendly_text)
=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py 2012-06-11 12:25:37 +0000
+++ src/maasserver/tests/test_views.py 2012-08-16 09:53:25 +0000
@@ -13,6 +13,7 @@
__all__ = []
import httplib
+from random import randint
from xmlrpclib import Fault
from django.conf.urls.defaults import patterns
@@ -31,11 +32,9 @@
LoggedInTestCase,
TestCase,
)
-from maasserver.utils import map_enum
from maasserver.views import HelpfulDeleteView
from maasserver.views.nodes import NodeEdit
from maastesting.matchers import ContainsAll
-from provisioningserver.enum import PSERV_FAULT
class Test404500(LoggedInTestCase):
@@ -280,9 +279,12 @@
def test_permanent_error_displayed(self):
self.patch(components, '_PERSISTENT_ERRORS', {})
- pserv_fault = set(map_enum(PSERV_FAULT).values())
+ fault_codes = [
+ randint(1, 100),
+ randint(101, 200),
+ ]
errors = []
- for fault in pserv_fault:
+ for fault in fault_codes:
# Create component with getRandomString to be sure
# to display all the errors.
component = factory.getRandomString()
=== modified file 'src/provisioningserver/enum.py'
--- src/provisioningserver/enum.py 2012-07-30 09:29:09 +0000
+++ src/provisioningserver/enum.py 2012-08-16 09:53:25 +0000
@@ -14,32 +14,9 @@
'ARP_HTYPE',
'POWER_TYPE',
'POWER_TYPE_CHOICES',
- 'PSERV_FAULT',
]
-class PSERV_FAULT:
- """Fault codes for errors raised by the provisioning server."""
-
- # Could not communicate with Cobbler.
- NO_COBBLER = 2
-
- # Failed to authenticate with Cobbler.
- COBBLER_AUTH_FAILED = 3
-
- # Cobbler no longer accepts the provisioning server's login token.
- COBBLER_AUTH_ERROR = 4
-
- # Profile does not exist.
- NO_SUCH_PROFILE = 5
-
- # Error looking up cobbler server.
- COBBLER_DNS_LOOKUP_ERROR = 6
-
- # Non-specific error inside Cobbler.
- GENERIC_COBBLER_ERROR = 99
-
-
class POWER_TYPE:
"""Choice of mechanism to control a node's power."""
