launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06393
[Merge] lp:~allenap/maas/use-real-cobbler into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/use-real-cobbler into lp:maas with lp:~allenap/maas/pserv-config-file as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/use-real-cobbler/+merge/93298
Use a real Cobbler instead of the fake in the maas-pserv Twisted plugin. This also makes logfile default to pserv.log.
--
https://code.launchpad.net/~allenap/maas/use-real-cobbler/+merge/93298
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/use-real-cobbler into lp:maas.
=== modified file 'HACKING.txt'
--- HACKING.txt 2012-02-10 16:59:35 +0000
+++ HACKING.txt 2012-02-16 12:20:11 +0000
@@ -72,6 +72,11 @@
$ make sampledata
+Add yourself as a user in Cobbler. For convenience, give yourself the
+password ``test``::
+
+ $ sudo htdigest /etc/cobbler/users.digest Cobbler $USER
+
Run the development webserver::
$ make run
=== modified file 'etc/pserv.yaml'
--- etc/pserv.yaml 2012-02-15 17:13:37 +0000
+++ etc/pserv.yaml 2012-02-16 12:20:11 +0000
@@ -1,20 +1,40 @@
-# Provisioning Server (pserv) configuration.
-
-# The port on which the Provisioning API will be made available.
-#port: 8001
-# Where to log. This log can be rotated by sending SIGUSR1 to the
-# running server.
-logfile: "pserv.log"
-
-# OOPS configuration (optional).
+##
+## Provisioning Server (pserv) configuration.
+##
+
+## The port on which the Provisioning API will be made available.
+#
+# port: 8001
+#
+
+## Where to log. This log can be rotated by sending SIGUSR1 to the
+## running server.
+#
+# logfile: "pserv.log"
+#
+
+## OOPS configuration (optional).
+#
# oops:
# directory: "/tmp/killed/by"
# reporter: "death"
+#
-# Message broker configuration (optional, not currently used).
+## Message broker configuration (optional, not currently used).
+#
# broker:
# host: "localhost"
# port: 1543
# username: "lemmy"
# password: "overkill"
# vhost: "/hammersmith"
+#
+
+## Cobbler configuration.
+#
+# cobbler:
+# url: "localhost"
+# username: "admin"
+# password: "test"
+# fake: false
+#
=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py 2012-02-14 10:42:20 +0000
+++ src/maasserver/provisioning.py 2012-02-16 12:20:11 +0000
@@ -13,7 +13,7 @@
'get_provisioning_api_proxy',
]
-from uuid import uuid1
+import warnings
import xmlrpclib
from django.conf import settings
@@ -29,14 +29,24 @@
def get_provisioning_api_proxy():
- """Return a proxy to the Provisioning API."""
- # FIXME: This is a little ugly.
+ """Return a proxy to the Provisioning API.
+
+ If ``PSERV_URL`` is not set, we attempt to return a handle to a fake proxy
+ implementation. This will not be available in a packaged version of MaaS,
+ in which case an error is raised.
+ """
url = settings.PSERV_URL
if url is None:
- from provisioningserver.testing.fakeapi import (
- FakeSynchronousProvisioningAPI,
- )
- return FakeSynchronousProvisioningAPI()
+ try:
+ from maasserver import testing
+ except ImportError:
+ # This is probably in a package.
+ raise RuntimeError("PSERV_URL must be defined.")
+ else:
+ warnings.warn(
+ "PSERV_URL is None; using the fake Provisioning API.",
+ RuntimeWarning)
+ return testing.get_fake_provisioning_api_proxy()
else:
return xmlrpclib.ServerProxy(
url, allow_none=True, use_datetime=True)
@@ -50,13 +60,12 @@
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)
+ # TODO: Choose a sensible profile.
+ profiles = papi.get_profiles()
+ assert len(profiles) >= 1, (
+ "No profiles defined in Cobbler; has "
+ "cobbler-ubuntu-import been run?")
+ profile = sorted(profiles)[0]
papi.add_node(instance.system_id, profile)
=== modified file 'src/maasserver/testing/__init__.py'
--- src/maasserver/testing/__init__.py 2012-01-24 17:09:34 +0000
+++ src/maasserver/testing/__init__.py 2012-02-16 12:20:11 +0000
@@ -6,13 +6,43 @@
unicode_literals,
)
-"""Test maasserver API."""
+"""Tests for `maasserver`."""
__metaclass__ = type
-__all__ = []
-
+__all__ = [
+ "get_fake_provisioning_api_proxy",
+ "LoggedInTestCase",
+ "TestCase",
+ ]
+
+from uuid import uuid1
+
+from fixtures import MonkeyPatch
from maasserver.testing.factory import factory
-from maastesting import TestCase
+import maastesting
+from provisioningserver.testing import fakeapi
+
+
+def get_fake_provisioning_api_proxy():
+ papi_fake = fakeapi.FakeSynchronousProvisioningAPI()
+ distro = papi_fake.add_distro(
+ "distro-%s" % uuid1().get_hex(),
+ "initrd", "kernel")
+ papi_fake.add_profile(
+ "profile-%s" % uuid1().get_hex(),
+ distro)
+ return papi_fake
+
+
+class TestCase(maastesting.TestCase):
+
+ def setUp(self):
+ super(TestCase, self).setUp()
+ papi_fake = get_fake_provisioning_api_proxy()
+ papi_fake_fixture = MonkeyPatch(
+ "maasserver.provisioning.get_provisioning_api_proxy",
+ lambda: papi_fake)
+ self.useFixture(papi_fake_fixture)
class LoggedInTestCase(TestCase):
=== added directory 'src/maasserver/testing/tests'
=== added file 'src/maasserver/testing/tests/__init__.py'
=== added file 'src/maasserver/testing/tests/test_module.py'
--- src/maasserver/testing/tests/test_module.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/testing/tests/test_module.py 2012-02-16 12:20:11 +0000
@@ -0,0 +1,52 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `maasserver.testing`."""
+
+from __future__ import (
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver import provisioning
+from maasserver.testing import TestCase
+from provisioningserver.testing import fakeapi
+
+
+class TestTestCase(TestCase):
+ """Tests for `TestCase`."""
+
+ def test_patched_in_fake_papi(self):
+ # TestCase.setUp() patches in a fake provisioning API so that we can
+ # observe what the signal handlers are doing.
+ papi_fake = provisioning.get_provisioning_api_proxy()
+ self.assertIsInstance(
+ papi_fake, fakeapi.FakeSynchronousProvisioningAPI)
+ # The fake has some limited, automatically generated, sample
+ # data. This is required for many tests to run. First there is a
+ # sample distro.
+ self.assertEqual(1, len(papi_fake.distros))
+ [distro_name] = papi_fake.distros
+ expected_distros = {
+ distro_name: {
+ u'initrd': u'initrd',
+ u'kernel': u'kernel',
+ u'name': distro_name,
+ },
+ }
+ self.assertEqual(expected_distros, papi_fake.distros)
+ # Second there is a sample profile, referring to the distro.
+ self.assertEqual(1, len(papi_fake.profiles))
+ [profile_name] = papi_fake.profiles
+ expected_profiles = {
+ profile_name: {
+ u'distro': distro_name,
+ u'name': profile_name,
+ },
+ }
+ self.assertEqual(expected_profiles, papi_fake.profiles)
+ # There are no nodes.
+ self.assertEqual({}, papi_fake.nodes)
=== modified file 'src/maasserver/tests/test_auth.py'
--- src/maasserver/tests/test_auth.py 2012-02-14 15:53:08 +0000
+++ src/maasserver/tests/test_auth.py 2012-02-16 12:20:11 +0000
@@ -19,8 +19,8 @@
Node,
NODE_STATUS,
)
+from maasserver.testing import TestCase
from maasserver.testing.factory import factory
-from maastesting import TestCase
class LoginLogoutTest(TestCase):
=== modified file 'src/maasserver/tests/test_macaddressfield.py'
--- src/maasserver/tests/test_macaddressfield.py 2012-01-23 12:43:28 +0000
+++ src/maasserver/tests/test_macaddressfield.py 2012-02-16 12:20:11 +0000
@@ -14,8 +14,8 @@
from django.core.exceptions import ValidationError
from maasserver.macaddress import validate_mac
from maasserver.models import MACAddress
+from maasserver.testing import TestCase
from maasserver.testing.factory import factory
-from maastesting import TestCase
class TestMACAddressField(TestCase):
=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py 2012-02-14 15:53:08 +0000
+++ src/maasserver/tests/test_models.py 2012-02-16 12:20:11 +0000
@@ -26,8 +26,8 @@
NODE_STATUS_CHOICES_DICT,
UserProfile,
)
+from maasserver.testing import TestCase
from maasserver.testing.factory import factory
-from maastesting import TestCase
from piston.models import (
Consumer,
KEY_SIZE,
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py 2012-02-13 16:21:23 +0000
+++ src/maasserver/tests/test_provisioning.py 2012-02-16 12:20:11 +0000
@@ -11,12 +11,10 @@
__metaclass__ = type
__all__ = []
-from fixtures import MonkeyPatch
from maasserver import provisioning
from maasserver.models import Node
+from maasserver.testing import TestCase
from maasserver.testing.factory import factory
-from maastesting import TestCase
-from provisioningserver.testing.fakeapi import FakeSynchronousProvisioningAPI
class ProvisioningTests:
@@ -97,35 +95,9 @@
self.assertEqual([], node["mac_addresses"])
-def patch_in_fake_papi(test):
- """Patch in a fake Provisioning API for the duration of the test."""
- papi_fake = FakeSynchronousProvisioningAPI()
- patch = MonkeyPatch(
- "maasserver.provisioning.get_provisioning_api_proxy",
- lambda: papi_fake)
- test.useFixture(patch)
- return papi_fake
-
-
-class TestProvisioningFake(TestCase):
- """Tests for `patch_in_fake_papi`."""
-
- 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 = patch_in_fake_papi(self)
- self.assertIsNot(provisioning.get_provisioning_api_proxy(), papi)
- self.assertIs(provisioning.get_provisioning_api_proxy(), papi_fake)
- # The fake is pristine; it does not contain sample data.
- self.assertEqual({}, papi_fake.distros)
- self.assertEqual({}, papi_fake.profiles)
- self.assertEqual({}, papi_fake.nodes)
-
-
class TestProvisioningWithFake(ProvisioningTests, TestCase):
"""Tests for the Provisioning API using a fake."""
def setUp(self):
super(TestProvisioningWithFake, self).setUp()
- self.papi = patch_in_fake_papi(self)
+ self.papi = provisioning.get_provisioning_api_proxy()
=== modified file 'src/maasserver/tests/test_runserver.py'
--- src/maasserver/tests/test_runserver.py 2012-02-07 16:11:46 +0000
+++ src/maasserver/tests/test_runserver.py 2012-02-16 12:20:11 +0000
@@ -12,7 +12,7 @@
__all__ = []
from maasserver.management.commands.runserver import render_error
-from maastesting import TestCase
+from maasserver.testing import TestCase
class TestRunServer(TestCase):
=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py 2012-02-15 17:20:40 +0000
+++ src/provisioningserver/plugin.py 2012-02-16 12:20:11 +0000
@@ -11,12 +11,15 @@
__metaclass__ = type
__all__ = []
+from getpass import getuser
+
from amqpclient import AMQFactory
from formencode import Schema
from formencode.validators import (
Int,
RequireIfPresent,
String,
+ URL,
)
from provisioningserver.cobblerclient import CobblerSession
from provisioningserver.remote import ProvisioningAPI_XMLRPC
@@ -24,10 +27,6 @@
LogService,
OOPSService,
)
-from provisioningserver.testing.fakecobbler import (
- FakeCobbler,
- FakeTwistedProxy,
- )
from twisted.application.internet import (
TCPClient,
TCPServer,
@@ -67,20 +66,34 @@
host = String(if_missing=b"localhost")
port = Int(min=1, max=65535, if_missing=5673)
- username = String(if_missing=None)
- password = String(if_missing=None)
+ username = String(if_missing=getuser())
+ password = String(if_missing=b"test")
vhost = String(if_missing="/")
+class ConfigCobbler(Schema):
+ """Configuration validator for connecting to Cobbler."""
+
+ if_key_missing = None
+
+ url = URL(
+ add_http=True, require_tld=False,
+ if_missing=b"http://localhost/cobbler_api";,
+ )
+ username = String(if_missing=getuser())
+ password = String(if_missing=b"test")
+
+
class Config(Schema):
"""Configuration validator."""
if_key_missing = None
port = Int(min=1, max=65535, if_missing=8001)
- logfile = String(not_empty=True)
+ logfile = String(if_empty=b"pserv.log", if_missing=b"pserv.log")
oops = ConfigOops
broker = ConfigBroker
+ cobbler = ConfigCobbler
@classmethod
def parse(cls, stream):
@@ -118,10 +131,7 @@
services = MultiService()
config_file = options["config-file"]
- if config_file is None:
- config = Config.parse(b"")
- else:
- config = Config.load(config_file)
+ config = Config.load(config_file)
log_service = LogService(config["logfile"])
log_service.setServiceParent(services)
@@ -139,9 +149,9 @@
broker_password = broker_config["password"]
broker_vhost = broker_config["vhost"]
- # Connecting to RabbitMQ is optional; it is not yet a required
- # component of a running MaaS installation.
- if broker_username is not None and broker_password is not None:
+ # Connecting to RabbitMQ is not yet a required component of a running
+ # MaaS installation; skip unless the password has been set explicitly.
+ if broker_password is not b"test":
cb_connected = lambda ignored: None # TODO
cb_disconnected = lambda ignored: None # TODO
cb_failed = lambda (connector, reason): (
@@ -154,17 +164,14 @@
client_service.setName("amqp")
client_service.setServiceParent(services)
- session = CobblerSession(
- # TODO: Get these values from command-line arguments.
- "http://localhost/does/not/exist";, "user", "password")
-
- # TODO: Remove this.
- fake_cobbler = FakeCobbler({"user": "password"})
- fake_cobbler_proxy = FakeTwistedProxy(fake_cobbler)
- session.proxy = fake_cobbler_proxy
+ cobbler_config = config["cobbler"]
+ cobbler_session = CobblerSession(
+ cobbler_config["url"], cobbler_config["username"],
+ cobbler_config["password"])
+ papi_xmlrpc = ProvisioningAPI_XMLRPC(cobbler_session)
site_root = Resource()
- site_root.putChild("api", ProvisioningAPI_XMLRPC(session))
+ site_root.putChild("api", papi_xmlrpc)
site = Site(site_root)
site_port = config["port"]
site_service = TCPServer(site_port, site)
=== modified file 'src/provisioningserver/testing/fakecobbler.py'
--- src/provisioningserver/testing/fakecobbler.py 2012-02-13 14:54:34 +0000
+++ src/provisioningserver/testing/fakecobbler.py 2012-02-16 12:20:11 +0000
@@ -20,7 +20,10 @@
from fnmatch import fnmatch
from itertools import count
from random import randint
-from xmlrpclib import Fault
+from xmlrpclib import (
+ dumps,
+ Fault,
+ )
from provisioningserver.cobblerclient import CobblerSession
from twisted.internet.defer import (
@@ -53,6 +56,11 @@
@inlineCallbacks
def callRemote(self, method, *args):
+ # Dump the call information as an XML-RPC request to ensure that it
+ # will travel over the wire. Cobbler does not allow None so we forbid
+ # it here too.
+ dumps(args, method, allow_none=False)
+ # Continue to forward the call to fake_cobbler.
callee = getattr(self.fake_cobbler, method, None)
assert callee is not None, "Unknown Cobbler method: %s" % method
result = yield callee(*args)
=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py 2012-02-16 09:24:41 +0000
+++ src/provisioningserver/tests/test_plugin.py 2012-02-16 12:20:11 +0000
@@ -11,6 +11,7 @@
__metaclass__ = type
__all__ = []
+from getpass import getuser
from functools import partial
import os
@@ -38,28 +39,29 @@
expected = {
'broker': {
'host': 'localhost',
- 'password': None,
'port': 5673,
- 'username': None,
+ 'username': getuser(),
+ 'password': 'test',
'vhost': u'/',
},
- 'logfile': '/some/where.log',
+ 'cobbler': {
+ 'url': 'http://localhost/cobbler_api',
+ 'username': getuser(),
+ 'password': 'test',
+ },
+ 'logfile': 'pserv.log',
'oops': {
'directory': '',
'reporter': '',
},
'port': 8001,
}
- mandatory_arguments = {
- "logfile": "/some/where.log",
- }
- observed = Config.to_python(mandatory_arguments)
+ observed = Config.to_python({})
self.assertEqual(expected, observed)
def test_parse(self):
# Configuration can be parsed from a snippet of YAML.
- observed = Config.parse(
- b'logfile: "/some/where.log"')
+ observed = Config.parse(b'logfile: "/some/where.log"')
self.assertEqual("/some/where.log", observed["logfile"])
def test_load(self):
@@ -81,7 +83,6 @@
def test_oops_directory_without_reporter(self):
# It is an error to omit the OOPS reporter if directory is specified.
config = (
- 'logfile: "/some/where.log"\n'
'oops:\n'
' directory: /tmp/oops\n'
)
@@ -121,9 +122,6 @@
self.tempdir = self.useFixture(TempDir()).path
def write_config(self, config):
- if "logfile" not in config:
- logfile = os.path.join(self.tempdir, "pserv.log")
- config = dict(config, logfile=logfile)
config_filename = os.path.join(self.tempdir, "config.yaml")
with open(config_filename, "wb") as stream:
yaml.dump(config, stream)
