launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06234
[Merge] lp:~jtv/maas/cobbler-reality-check into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/cobbler-reality-check into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/cobbler-reality-check/+merge/91397
More fixes for the Cobbler “reality check” — that is, running tests written against our fake cobbler against the real thing.
We've still got plenty of tests failing, for two reasons:
1. Cobbler “kernel not found” errors that I'm still working on.
2. Use of testing features specific to the fake.
In this branch I externalize the retrieval of CobblerObject attribute values. It's simpler than trying to cache anything we can in the object, and fetching them on demand. What led me to this change is the discovery that whereas Cobbler's get_* methods (for either a single object or all objects of a given type) return dicts containing the objects' attributes, the find_* methods return only lists of names.
You'll also see the tests creating some required items, similar to how the LaunchpadObjectFactory creates e.g. an owner for a distribution you tell it to make. And they create some files as well, because the API requires a distro to have a kernel and an initrd, both of which must be present on the filesystem. Obviously something's still missing there, witness the “kernel not found” problem above.
Jeroen
--
https://code.launchpad.net/~jtv/maas/cobbler-reality-check/+merge/91397
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/cobbler-reality-check into lp:maas.
=== modified file 'src/provisioningserver/cobblerclient.py'
--- src/provisioningserver/cobblerclient.py 2012-02-02 13:27:16 +0000
+++ src/provisioningserver/cobblerclient.py 2012-02-03 08:10:36 +0000
@@ -237,20 +237,17 @@
# What attributes does Cobbler require for this type of object?
required_attributes = []
- def __init__(self, session, name=None, values=None):
+ def __init__(self, session, name):
"""Reference an object in Cobbler.
:param session: A `CobblerSession`.
- :param name: Name for this object, if known.
- :param values: Attribute values for this object, if known.
+ :param name: Name for this object.
"""
- if values is None:
- values = {}
self.session = session
- # Cache the name; we need it when deleting objects.
- self.name = name or values.get('name')
+ self.name = name
def _get_handle(self):
+ """Retrieve the object's handle."""
method = self._name_method('get_%s_handle')
return self.session.call(
method, self.name, self.session.token_placeholder)
@@ -308,18 +305,35 @@
:param **kwargs: Optional search criteria, e.g.
hostname="*.maas3.example.com" to limit the search to items with
a hostname attribute that ends in ".maas3.example.com".
- :return: A list of `cls` objects.
- """
- if kwargs:
- method = cls._name_method("find_%s")
- criteria = dict(
- (cls._normalize_attribute(key), value)
- for key, value in kwargs.items())
- result = yield session.call(method, criteria)
- else:
- method = cls._name_method("get_%s", plural=True)
- result = yield session.call(method)
- returnValue([cls(session, values=item) for item in result])
+ :return: A list of matching `cls` objects.
+ """
+ method = cls._name_method("find_%s")
+ criteria = dict(
+ (cls._normalize_attribute(key), value)
+ for key, value in kwargs.items())
+ result = yield session.call(method, criteria)
+ returnValue([cls(session, name) for name in result])
+
+ @classmethod
+ @inlineCallbacks
+ def get_all_values(cls, session):
+ """Load the attributes for all objects of this type.
+
+ :return: A `Deferred` that delivers a dict, mapping objects' names
+ to dicts containing their respective attributes.
+ """
+ method = cls._name_method("get_%s", plural=True)
+ result = yield session.call(method)
+ returnValue(dict((obj['name'], obj) for obj in result))
+
+ def get_values(self):
+ """Load the object's attributes as a dict.
+
+ :return: A `Deferred` that delivers a dict containing the object's
+ attribute names and values.
+ """
+ d = self.session.call(self._name_method("get_%s"), self.name)
+ return d
@classmethod
@inlineCallbacks
@@ -352,7 +366,7 @@
raise RuntimeError(
"Cobbler refused to create %s '%s'. Attributes: %s"
% (cls.object_type, name, args))
- returnValue(cls(session, name=name, values=args))
+ returnValue(cls(session, name))
@inlineCallbacks
def delete(self, recurse=True):
=== modified file 'src/provisioningserver/testing/fakecobbler.py'
--- src/provisioningserver/testing/fakecobbler.py 2012-02-02 13:27:16 +0000
+++ src/provisioningserver/testing/fakecobbler.py 2012-02-03 08:10:36 +0000
@@ -193,19 +193,28 @@
return handle
def _api_find_objects(self, object_type, criteria):
- """Find objects in the saved store that match `criteria`.
+ """Find names of objects in the saved store that match `criteria`.
- :return: A list of object dicts. The dicts are copied from the
- saved store; they are not references to the originals in the
- store.
+ :return: A list of object names.
"""
# Assumption: these operations look only at saved objects.
location = self.store[None].get(object_type, {})
return [
- candidate.copy()
+ candidate['name']
for candidate in location.values()
if self._matches(candidate, criteria)]
+ def _api_get_object(self, object_type, name):
+ """Get object's attributes by name."""
+ location = self.store[None][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))
+ if len(matches) == 0:
+ return None
+ else:
+ return matches[0]
+
def _api_get_objects(self, object_type):
"""Return all saved objects of type `object_type`.
@@ -328,6 +337,9 @@
def find_distro(self, criteria):
return self._api_find_objects('distro', criteria)
+ def get_distro(self, name):
+ return self._api_get_object('distro', name)
+
def get_distros(self):
return self._api_get_objects('distro')
@@ -349,6 +361,9 @@
def find_image(self, criteria):
return self._api_find_objects('image', criteria)
+ def get_image(self, name):
+ return self._api_get_object('image', name)
+
def get_images(self):
return self._api_get_objects('image')
@@ -370,6 +385,9 @@
def find_profile(self, criteria):
return self._api_find_objects('profile', criteria)
+ def get_profile(self, name):
+ return self._api_get_object('profile', name)
+
def get_profiles(self):
return self._api_get_objects('profile')
@@ -391,6 +409,9 @@
def find_repo(self, criteria):
return self._api_find_objects('repo', criteria)
+ def get_repo(self, name):
+ return self._api_get_object('repo', name)
+
def get_repos(self):
return self._api_get_objects('repo')
@@ -425,6 +446,9 @@
def find_system(self, criteria):
return self._api_find_objects('system', criteria)
+ def get_system(self, name):
+ return self._api_get_object('system', name)
+
def get_systems(self):
return self._api_get_objects('system')
=== modified file 'src/provisioningserver/tests/test_fakecobbler.py'
--- src/provisioningserver/tests/test_fakecobbler.py 2012-02-02 13:46:45 +0000
+++ src/provisioningserver/tests/test_fakecobbler.py 2012-02-03 08:10:36 +0000
@@ -13,6 +13,7 @@
from itertools import count
from random import randint
+from tempfile import NamedTemporaryFile
import xmlrpclib
from provisioningserver.cobblerclient import (
@@ -74,6 +75,37 @@
returnValue(session)
+def make_file():
+ """Make a temporary file."""
+ temp_file = NamedTemporaryFile()
+ temp_file.write("Data here.")
+ temp_file.flush()
+ return temp_file
+
+
+def default_to_file(attributes, attribute, required_attrs):
+ """If `attributes[attribute]` is required but not set, make a file.
+
+ :return: A temporary file. Keep this alive as long as you need the file.
+ """
+ if attribute in required_attrs and attribute not in attributes:
+ temp_file = make_file()
+ attributes[attribute] = temp_file.name
+ return temp_file
+ else:
+ return None
+
+
+@inlineCallbacks
+def default_to_object(attributes, attribute, required_attrs, session,
+ cobbler_class):
+ """If `attributes[attribute]` is required but not set, make an object."""
+ if attribute in required_attrs and attribute not in attributes:
+ other_obj = yield fake_cobbler_object(session, cobbler_class)
+ attributes[attribute] = other_obj.name
+
+
+@inlineCallbacks
def fake_cobbler_object(session, object_class, name=None, attributes=None):
"""Create a fake Cobbler object.
@@ -85,16 +117,30 @@
if attributes is None:
attributes = {}
else:
- attributes = dict(attributes)
+ attributes = attributes.copy()
unique_int = next(unique_ints)
if name is None:
name = 'name-%s-%d' % (object_class.object_type, unique_int)
attributes['name'] = name
+ temp_files = [
+ default_to_file(
+ attributes, 'kernel', object_class.required_attributes),
+ default_to_file(
+ attributes, 'initrd', object_class.required_attributes),
+ ]
+ yield default_to_object(
+ attributes, 'profile', object_class.required_attributes, session,
+ CobblerProfile)
+ yield default_to_object(
+ attributes, 'distro', object_class.required_attributes, session,
+ CobblerDistro)
for attr in object_class.required_attributes:
if attr not in attributes:
attributes[attr] = '%s-%d' % (attr, unique_int)
- d = object_class.new(session, name, attributes)
- return d
+ new_object = yield object_class.new(session, name, attributes)
+ # Keep the temporary files alive for the lifetime of the object.
+ new_object._hold_these_files_please = temp_files
+ returnValue(new_object)
class TestFakeCobbler(TestCase):
@@ -123,7 +169,7 @@
# Use of the token will now fail with an "invalid token"
# error. The Cobbler client notices this, re-authenticates, and
# re-runs the method.
- yield fake_cobbler_object(session, CobblerSystem)
+ yield fake_cobbler_object(session, CobblerRepo)
# The re-authentication results in a fresh token.
self.assertNotEqual(old_token, session.token)
@@ -132,7 +178,7 @@
def test_valid_token_does_not_raise_auth_error(self):
session = yield fake_cobbler_session()
old_token = session.token
- yield fake_cobbler_object(session, CobblerSystem)
+ yield fake_cobbler_object(session, CobblerRepo)
self.assertEqual(old_token, session.token)
@@ -190,25 +236,49 @@
session = yield fake_cobbler_session()
name = self.make_name()
yield fake_cobbler_object(session, self.cobbler_class, name)
- [by_comment] = yield self.cobbler_class.find(session, name=name)
- self.assertEqual(name, by_comment.name)
+ by_name = yield self.cobbler_class.find(session, name=name)
+ self.assertEqual([name], [obj.name for obj in by_name])
@inlineCallbacks
def test_find_matches_attribute(self):
session = yield fake_cobbler_session()
name = self.make_name()
+ comment = "This is comment #%d" % next(unique_ints)
yield fake_cobbler_object(
- session, self.cobbler_class, name, {'comment': 'Hi'})
- [by_comment] = yield self.cobbler_class.find(session, comment='Hi')
- self.assertEqual(name, by_comment.name)
+ session, self.cobbler_class, name, {'comment': comment})
+ by_comment = yield self.cobbler_class.find(session, comment=comment)
+ self.assertEqual([name], [obj.name for obj in by_comment])
@inlineCallbacks
def test_find_without_args_finds_everything(self):
session = yield fake_cobbler_session()
name = self.make_name()
yield fake_cobbler_object(session, self.cobbler_class, name)
- found_items = yield self.cobbler_class.find(session)
- self.assertEqual([name], [item.name for item in found_items])
+ found_objects = yield self.cobbler_class.find(session)
+ self.assertIn(name, [obj.name for obj in found_objects])
+
+ @inlineCallbacks
+ def test_get_object_retrieves_attributes(self):
+ session = yield fake_cobbler_session()
+ comment = "This is comment #%d" % next(unique_ints)
+ name = self.make_name()
+ obj = yield fake_cobbler_object(
+ session, self.cobbler_class, name, {'comment': comment})
+ attributes = yield obj.get_values()
+ self.assertEqual(name, attributes['name'])
+ self.assertEqual(comment, attributes['comment'])
+
+ @inlineCallbacks
+ def test_get_objects_retrieves_attributes_for_all_objects(self):
+ session = yield fake_cobbler_session()
+ comment = "This is comment #%d" % next(unique_ints)
+ name = self.make_name()
+ yield fake_cobbler_object(
+ session, self.cobbler_class, name, {'comment': comment})
+ all_objects = yield self.cobbler_class.get_all_values(session)
+ found_obj = all_objects[name]
+ self.assertEqual(name, found_obj['name'])
+ self.assertEqual(comment, found_obj['comment'])
@inlineCallbacks
def test_get_handle_finds_handle(self):
@@ -240,7 +310,7 @@
session = yield fake_cobbler_session()
name = self.make_name()
obj = yield fake_cobbler_object(session, self.cobbler_class, name)
- obj.delete()
+ yield obj.delete()
matches = yield self.cobbler_class.find(session, name=name)
self.assertEqual([], matches)
@@ -433,7 +503,7 @@
def test_get_templates_lists_templates(self):
preseeds = yield self.make_preseeds_api()
unique_int = next(unique_ints)
- path = '/var/lib/cobbler/template/template-%d' % unique_int
+ path = '/var/lib/cobbler/kickstarts/template-%d' % unique_int
yield preseeds.write_template(path, "Text")
templates = yield preseeds.get_templates()
self.assertIn(path, templates)
@@ -445,16 +515,16 @@
path = '/var/lib/cobbler/snippets/snippet-%d' % unique_int
yield preseeds.write_snippet(path, "Text")
snippets = yield preseeds.get_snippets()
- self.assertEqual([path], snippets)
+ self.assertIn(path, snippets)
@inlineCallbacks
def test_templates_do_not_show_up_as_snippets(self):
preseeds = yield self.make_preseeds_api()
unique_int = next(unique_ints)
- path = '/var/lib/cobbler/template/template-%d' % unique_int
+ path = '/var/lib/cobbler/kickstarts/template-%d' % unique_int
yield preseeds.write_template(path, "Text")
snippets = yield preseeds.get_snippets()
- self.assertEqual([], snippets)
+ self.assertNotIn(path, snippets)
@inlineCallbacks
def test_snippets_do_not_show_up_as_templates(self):
@@ -463,4 +533,4 @@
path = '/var/lib/cobbler/snippets/snippet-%d' % unique_int
yield preseeds.write_snippet(path, "Text")
templates = yield preseeds.get_templates()
- self.assertEqual([], templates)
+ self.assertNotIn(path, templates)
