documented packages backends

6 files changed, 431 insertions, 123 deletions

diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
index 5e3d86f02..11355e117 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
@@ -1,3 +1,5 @@
+""" APT backend for :mod:`Bcfg2.Server.Plugins.Packages` """
+
 import re
 import gzip
 from Bcfg2.Server.Plugins.Packages.Collection import Collection
@@ -5,7 +7,16 @@ from Bcfg2.Server.Plugins.Packages.Source import Source
 
 
 class AptCollection(Collection):
+    """ Handle collections of APT sources.  This is a no-op object
+    that simply inherits from
+    :class:`Bcfg2.Server.Plugins.Packages.Collection.Collection`,
+    overrides nothing, and defers all operations to :class:`PacSource`
+    """
+
     def get_config(self):
+        """ Get an APT configuration file (i.e., ``sources.list``).
+
+        :returns: string """
         lines = ["# This config was generated automatically by the Bcfg2 " \
                      "Packages plugin", '']
 
@@ -22,11 +33,19 @@ class AptCollection(Collection):
 
 
 class AptSource(Source):
+    """ Handle APT sources """
+
+    #: :ref:`server-plugins-generators-packages-magic-groups` for
+    #: ``AptSource`` are "apt", "debian", "ubuntu", and "nexenta"
     basegroups = ['apt', 'debian', 'ubuntu', 'nexenta']
+
+    #: AptSource sets the ``type`` on Package entries to "deb"
     ptype = 'deb'
 
     @property
     def urls(self):
+        """ A list of URLs to the base metadata file for each
+        repository described by this source. """
         if not self.rawurl:
             rv = []
             for part in self.components:
@@ -92,3 +111,4 @@ class AptSource(Source):
                             bprov[barch][dname] = set()
                         bprov[barch][dname].add(pkgname)
         self.process_files(bdeps, bprov)
+    read_files.__doc__ = Source.read_files.__doc__
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py b/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
index f5c035e00..d0e4a3665 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
@@ -109,6 +109,8 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         :type basepath: string
         :param debug: Enable debugging output
         :type debug: bool
+
+        .. autoattribute:: __package_groups__
         """
         Bcfg2.Server.Plugin.Debuggable.__init__(self)
         list.__init__(self, sources)
@@ -119,15 +121,11 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
 
         try:
             self.setup = sources[0].setup
-            self.cachepath = sources[0].basepath
             self.ptype = sources[0].ptype
         except IndexError:
             self.setup = None
-            self.cachepath = None
             self.ptype = "unknown"
 
-        self.cachefile = None
-
     @property
     def cachekey(self):
         """ A unique identifier for the set of sources contained in
@@ -141,8 +139,9 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         source type.  This should be a config appropriate for use on
         either the server (to resolve dependencies) or the client.
 
-        Subclasses must override this method.  By default it logs an
-        error and returns the empty string.
+        Subclasses must override this method in order to be able to
+        generate configs.  By default it logs an error and returns the
+        empty string.
 
         :returns: string """
         self.logger.error("Packages: Cannot generate config for host %s with "
@@ -188,7 +187,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
     get_relevant_groups.__doc__ = Source.get_relevant_groups.__doc__ + """
 
         The base implementation simply aggregates the results of
-        :func:`Bcfg2.Server.Plugins.Packages.Source.Source.get_relevant_groups`.
+        :func:`Bcfg2.Server.Plugins.Packages.Source.Source.get_relevant_groups`
         """
 
     @property
@@ -212,7 +211,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         The base implementation simply aggregates
         :attr:`Bcfg2.Server.Plugins.Packages.Source.Source.cachefile`
         attributes."""
-        cachefiles = set([self.cachepath])
+        cachefiles = set()
         for source in self:
             cachefiles.add(source.cachefile)
         return list(cachefiles)
@@ -393,7 +392,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         handled for a complete client configuration.
 
         :param independent: The XML tag to add extra entries to.  This
-                            should be modified in place.
+                            is modified in place.
         :type independent: lxml.etree._Element
         """
         pass
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py b/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
index 13090cd9f..ac58f4c99 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
@@ -1,18 +1,33 @@
+""" Pacman backend for :mod:`Bcfg2.Server.Plugins.Packages` """
+
 import tarfile
 from Bcfg2.Server.Plugins.Packages.Collection import Collection
 from Bcfg2.Server.Plugins.Packages.Source import Source
 
 
 class PacCollection(Collection):
+    """ Handle collections of Pacman sources.  This is a no-op object
+    that simply inherits from
+    :class:`Bcfg2.Server.Plugins.Packages.Collection.Collection`,
+    overrides nothing, and defers all operations to :class:`PacSource`
+    """
     pass
 
 
 class PacSource(Source):
+    """ Handle Pacman sources """
+
+    #: :ref:`server-plugins-generators-packages-magic-groups` for
+    #: ``PacSource`` are "arch" and "parabola"
     basegroups = ['arch', 'parabola']
+
+    #: PacSource sets the ``type`` on Package entries to "pacman"
     ptype = 'pacman'
 
     @property
     def urls(self):
+        """ A list of URLs to the base metadata file for each
+        repository described by this source. """
         if not self.rawurl:
             rv = []
             for part in self.components:
@@ -56,3 +71,4 @@ class PacSource(Source):
                                    tarinfo.name.rsplit("-", 2)[0])
             tar.close()
         self.process_files(bdeps, bprov)
+    read_files.__doc__ = Source.read_files.__doc__
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Source.py b/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
index 3269f8c08..7188394b3 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
@@ -119,6 +119,11 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
     #: default, excludes any package whose name starts with "choice"
     unknown_filter = lambda p: p.startswith("choice")
 
+    #: The Package type handled by this Source class.  The ``type``
+    #: attribute of Package entries will be set to the value ``ptype``
+    #: when they are handled by :mod:`Bcfg2.Server.Plugins.Packages`.
+    ptype = None
+
     def __init__(self, basepath, xsource, setup):
         """
         :param basepath: The base filesystem path under which cache
@@ -390,18 +395,15 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
 
         #. First, if the map contains a ``component`` key, use that as
            the name.
-
         #. If not, then try to match the repository URL against
            :attr:`Bcfg2.Server.Plugins.Packages.Source.REPO_RE`.  If
            that succeeds, use the first matched group; additionally,
            if the Source tag that describes this repo is contained in
            a ``<Group>`` tag, prepend that to the name.
-
         #. If :attr:`Bcfg2.Server.Plugins.Packages.Source.REPO_RE`
            does not match the repository, and the Source tag that
            describes this repo is contained in a ``<Group>`` tag, use
            the name of the group.
-
         #. Failing that, use the full URL to this repository, with the
            protocol and trailing slash stripped off if possible.
 
@@ -512,9 +514,12 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
         return os.path.join(self.basepath, url.replace('/', '@'))
 
     def read_files(self):
-        """ Read and parse locally downloaded metadata files.  This
-        function should call :func:`process_files` as its final
-        step. This should also populate :attr:`pkgnames`. """
+        """ Read and parse locally downloaded metadata files and
+        populates
+        :attr:`Bcfg2.Server.Plugins.Packages.Source.Source.pkgnames`. Should
+        call
+        :func:`Bcfg2.Server.Plugins.Packages.Source.Source.process_files`
+        as its final step."""
         pass
 
     def process_files(self, deps, prov):
@@ -672,10 +677,6 @@ class Source(Bcfg2.Server.Plugin.Debuggable):
     def is_package(self, metadata, package):  # pylint: disable=W0613
         """ Return True if a package is a package, False otherwise.
 
-        The base implementation returns True if any Source object's
-        :func:`Bcfg2.Server.Plugins.Packages.Source.Source.is_package`
-        returns True.
-
         :param package: The name of the package
         :type package: string
         :returns: bool
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
index c1dd80689..1aa0128c4 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
@@ -1,3 +1,55 @@
+""" Yum backend for :mod:`Bcfg2.Server.Plugins.Packages`.  This module
+is the most complex backend because it has to handle Yum sources
+without yum Python libraries, with yum Python libraries, and Pulp
+sources.  (See :ref:`native-yum-libraries` for details on using the
+yum Python libraries and :ref:`pulp-source-support` for details on
+Pulp sources.)
+
+.. _bcfg2-yum-helper:
+
+bcfg2-yum-helper
+~~~~~~~~~~~~~~~~
+
+If using the yum Python libraries, :class:`YumCollection` makes shell
+calls to an external command, ``bcfg2-yum-helper``, which performs the
+actual yum API calls.  This is done because the yum libs have horrific
+memory leaks, and apparently the right way to get around that in
+long-running processes it to have a short-lived helper.  This is how
+it's done by yum itself in ``yum-updatesd``, which is a long-running
+daemon that checks for and applies updates.
+
+.. _yum-pkg-objects:
+
+Package Objects
+~~~~~~~~~~~~~~~
+
+:class:`Bcfg2.Server.Plugins.Packages.Collection.Collection` objects
+have the option to translate from some backend-specific representation
+of packages to XML entries; see :ref:`pkg-objects` for more
+information on this.  If you are using the Python yum libraries,
+:class:`Bcfg2.Server.Plugins.Packages.Yum.YumCollection` opts to do
+this, using the yum tuple representation of packages, which is::
+
+    (<name>, <arch>, <epoch>, <version>, <release>)
+
+For shorthand this is occasionally abbrevated "naevr".  Any datum that
+is not defined is ``None``.  So a normal package entry that can be any
+version would be passed to :ref:`bcfg2-yum-helper` as::
+
+    ("somepackage", None, None, None, None)
+
+A package returned from the helper might look more like this::
+
+    ("somepackage", "x86_64", None, "1.2.3", "1.el6")
+
+We translate between this representation and the XML representation of
+packages with :func:`YumCollection.packages_from_entry` and
+:func:`YumCollection.packages_to_entry`.
+
+The Yum Backend
+~~~~~~~~~~~~~~~
+"""
+
 import os
 import re
 import sys
@@ -8,29 +60,31 @@ import lxml.etree
 from subprocess import Popen, PIPE
 import Bcfg2.Server.Plugin
 from Bcfg2.Compat import StringIO, cPickle, HTTPError, URLError, \
-    ConfigParser, json
+    ConfigParser, json, any
 from Bcfg2.Server.Plugins.Packages.Collection import Collection
 from Bcfg2.Server.Plugins.Packages.Source import SourceInitError, Source, \
      fetch_url
 
 logger = logging.getLogger(__name__)
 
+# pylint: disable=E0611
 try:
     from pulp.client.consumer.config import ConsumerConfig
     from pulp.client.api.repository import RepositoryAPI
     from pulp.client.api.consumer import ConsumerAPI
     from pulp.client.api import server
-    has_pulp = True
+    HAS_PULP = True
 except ImportError:
-    has_pulp = False
+    HAS_PULP = False
 
 try:
     import yum
-    has_yum = True
+    HAS_YUM = True
 except ImportError:
-    has_yum = False
+    HAS_YUM = False
     logger.info("Packages: No yum libraries found; forcing use of internal "
                 "dependency resolver")
+# pylint: enable=E0611
 
 XP = '{http://linux.duke.edu/metadata/common}'
 RP = '{http://linux.duke.edu/metadata/rpm}'
@@ -42,9 +96,18 @@ PULPCONFIG = None
 
 
 def _setup_pulp(setup):
+    """ Connect to a Pulp server and pass authentication credentials.
+    This only needs to be called once, but multiple calls won't hurt
+    anything.
+
+    :param setup: A Bcfg2 options dict
+    :type setup: dict
+    :returns: :class:`pulp.client.api.server.PulpServer`
+    """
     global PULPSERVER, PULPCONFIG
-    if not has_pulp:
-        msg = "Packages: Cannot create Pulp collection: Pulp libraries not found"
+    if not HAS_PULP:
+        msg = "Packages: Cannot create Pulp collection: Pulp libraries " + \
+            "not found"
         logger.error(msg)
         raise Bcfg2.Server.Plugin.PluginInitError(msg)
 
@@ -53,11 +116,12 @@ def _setup_pulp(setup):
             username = setup.cfp.get("packages:pulp", "username")
             password = setup.cfp.get("packages:pulp", "password")
         except ConfigParser.NoSectionError:
-            msg = "Packages: No [pulp] section found in Packages/packages.conf"
+            msg = "Packages: No [pulp] section found in bcfg2.conf"
             logger.error(msg)
             raise Bcfg2.Server.Plugin.PluginInitError(msg)
         except ConfigParser.NoOptionError:
-            msg = "Packages: Required option not found in Packages/packages.conf: %s" % sys.exc_info()[1]
+            msg = "Packages: Required option not found in bcfg2.conf: %s" % \
+                sys.exc_info()[1]
             logger.error(msg)
             raise Bcfg2.Server.Plugin.PluginInitError(msg)
 
@@ -74,8 +138,14 @@ def _setup_pulp(setup):
 
 
 class YumCollection(Collection):
-    #: YumCollections support package groups
-    __package_groups__ = True
+    """ Handle collections of Yum sources.  If we're using the yum
+    Python libraries, then this becomes a very full-featured
+    :class:`Bcfg2.Server.Plugins.Packages.Collection.Collection`
+    object; if not, then it defers to the :class:`YumSource`
+    object.
+
+    .. private-include: _add_gpg_instances, _get_pulp_consumer
+    """
 
     #: Options that are included in the [packages:yum] section of the
     #: config but that should not be included in the temporary
@@ -87,23 +157,45 @@ class YumCollection(Collection):
         self.keypath = os.path.join(self.basepath, "keys")
 
         if self.use_yum:
-            self.cachefile = os.path.join(self.cachepath,
+            #: Define a unique cache file for this collection to use
+            #: for cached yum metadata
+            self.cachefile = os.path.join(self.basepath,
                                          "cache-%s" % self.cachekey)
             if not os.path.exists(self.cachefile):
                 os.mkdir(self.cachefile)
 
+            #: The path to the server-side config file used when
+            #: resolving packages with the Python yum libraries
             self.cfgfile = os.path.join(self.cachefile, "yum.conf")
             self.write_config()
-        if has_pulp and self.has_pulp_sources:
+        else:
+            self.cachefile = None
+
+        if HAS_PULP and self.has_pulp_sources:
             _setup_pulp(self.setup)
 
         self._helper = None
 
     @property
+    def __package_groups__(self):
+        """ YumCollections support package groups only if
+        :attr:`use_yum` is True """
+        if self.use_yum:
+            return True
+        else:
+            return False
+
+    @property
     def helper(self):
+        """ The full path to :file:`bcfg2-yum-helper`.  First, we
+        check in the config file to see if it has been explicitly
+        specified; next we see if it's in $PATH (which we do by making
+        a call to it; I wish there was a way to do this without
+        forking, but apparently not); finally we check in /usr/sbin,
+        the default location. """
         try:
             return self.setup.cfp.get("packages:yum", "helper")
-        except:
+        except (ConfigParser.NoOptionError, ConfigParser.NoSectionError):
             pass
 
         if not self._helper:
@@ -118,19 +210,30 @@ class YumCollection(Collection):
 
     @property
     def use_yum(self):
-        return has_yum and self.setup.cfp.getboolean("packages:yum",
+        """ True if we should use the yum Python libraries, False
+        otherwise """
+        return HAS_YUM and self.setup.cfp.getboolean("packages:yum",
                                                      "use_yum_libraries",
                                                      default=False)
 
     @property
     def has_pulp_sources(self):
-        """ see if there are any pulp sources to handle """
-        for source in self:
-            if source.pulp_id:
-                return True
-        return False
+        """ True if there are any Pulp sources to handle, False
+        otherwise """
+        return any(s.pulp_id for s in self)
+
+    @property
+    def cachefiles(self):
+        """ A list of the full path to all cachefiles used by this
+        collection."""
+        cachefiles = set(Collection.cachefiles(self))
+        if self.cachefile:
+            cachefiles.add(self.cachefile)
+        return list(cachefiles)
 
     def write_config(self):
+        """ Write the server-side config file to :attr:`cfgfile` based
+        on the data from :func:`get_config`"""
         if not os.path.exists(self.cfgfile):
             yumconf = self.get_config(raw=True)
             yumconf.add_section("main")
@@ -163,6 +266,17 @@ class YumCollection(Collection):
             yumconf.write(open(self.cfgfile, 'w'))
 
     def get_config(self, raw=False):
+        """ Get the yum configuration for this collection.
+
+        :param raw: Return a :class:`ConfigParser.SafeConfigParser`
+                    object representing the configuration instead of a
+                    string.  This is useful if you need to modify the
+                    config before writing it (as :func:`write_config`
+                    does in order to produce a server-specific
+                    configuration).
+        :type raw: bool
+        :returns: string or ConfigParser.SafeConfigParser """
+
         config = ConfigParser.SafeConfigParser()
         for source in self:
             for url_map in source.url_map:
@@ -219,28 +333,46 @@ class YumCollection(Collection):
                    "Packages plugin\n\n" + buf.getvalue()
 
     def build_extra_structures(self, independent):
-        """ build list of gpg keys to be added to the specification by
-        validate_structures() """
+        """ Add additional entries to the ``<Independent/>`` section
+        of the final configuration.  This adds several kinds of
+        entries:
+
+        * For GPG keys, adds a ``Package`` entry that describes the
+          version and release of all expected ``gpg-pubkey`` packages;
+          and ``Path`` entries to copy all of the GPG keys to the
+          appropriate place on the client filesystem.  Calls
+          :func:`_add_gpg_instances`.
+
+        * For Pulp Sources, adds a ``Path`` entry for the consumer
+          certificate; and ``Action`` entries to update the
+          consumer-side Pulp config if the consumer is newly
+          registered.  Creates a new Pulp consumer from the Bcfg2
+          server as necessary.
+
+        :param independent: The XML tag to add extra entries to.  This
+                            is modified in place.
+        :type independent: lxml.etree._Element
+        """
         needkeys = set()
         for source in self:
             for key in source.gpgkeys:
                 needkeys.add(key)
 
         if len(needkeys):
-            if has_yum:
-                # this must be be has_yum, not use_yum, because
+            if HAS_YUM:
+                # this must be be HAS_YUM, not use_yum, because
                 # regardless of whether the user wants to use the yum
                 # resolver we want to include gpg key data
                 keypkg = lxml.etree.Element('BoundPackage', name="gpg-pubkey",
                                             type=self.ptype, origin='Packages')
             else:
-                self.logger.warning("GPGKeys were specified for yum sources in "
-                                    "sources.xml, but no yum libraries were "
-                                    "found")
+                self.logger.warning("GPGKeys were specified for yum sources "
+                                    "in sources.xml, but no yum libraries "
+                                    "were found")
                 self.logger.warning("GPG key version/release data cannot be "
                                     "determined automatically")
-                self.logger.warning("Install yum libraries, or manage GPG keys "
-                                    "manually")
+                self.logger.warning("Install yum libraries, or manage GPG "
+                                    "keys manually")
                 keypkg = None
 
             for key in needkeys:
@@ -260,7 +392,8 @@ class YumCollection(Collection):
                 keypath.text = kdata
 
                 # hook to add version/release info if possible
-                self._add_gpg_instances(keypkg, kdata, localkey, remotekey)
+                self._add_gpg_instances(keypkg, localkey, remotekey,
+                                        keydata=kdata)
                 independent.append(keypath)
             if keypkg is not None:
                 independent.append(keypkg)
@@ -290,6 +423,15 @@ class YumCollection(Collection):
             crt.text = consumerapi.certificate(self.metadata.hostname)
 
     def _get_pulp_consumer(self, consumerapi=None):
+        """ Get a Pulp consumer object for the client.
+
+        :param consumerapi: A Pulp ConsumerAPI object.  If none is
+                            passed, one will be instantiated.
+        :type consumerapi: pulp.client.api.consumer.ConsumerAPI
+        :returns: dict - the consumer.  Returns None on failure
+                  (including if there is no existing Pulp consumer for
+                  this client.
+        """
         if consumerapi is None:
             consumerapi = ConsumerAPI()
         consumer = None
@@ -304,19 +446,38 @@ class YumCollection(Collection):
                               err)
         except:
             err = sys.exc_info()[1]
-            self.logger.error("Packages: Unknown error querying Pulp server: %s"
-                              % err)
+            self.logger.error("Packages: Unknown error querying Pulp server: "
+                              "%s" % err)
         return consumer
 
-    def _add_gpg_instances(self, keyentry, keydata, localkey, remotekey):
-        """ add gpg keys to the specification to ensure they get
-        installed """
-        # this must be be has_yum, not use_yum, because regardless of
+    def _add_gpg_instances(self, keyentry, localkey, remotekey, keydata=None):
+        """ Add GPG keys instances to a ``Package`` entry.  This is
+        called from :func:`build_extra_structures` to add GPG keys to
+        the specification.
+
+        :param keyentry: The ``Package`` entry to add key instances
+                         to.  This will be modified in place.
+        :type keyentry: lxml.etree._Element
+        :param localkey: The full path to the key file on the Bcfg2 server
+        :type localkey: string
+        :param remotekey: The full path to the key file on the client.
+                          (If they key is not yet on the client, this
+                          will be the full path to where the key file
+                          will go eventually.)
+        :type remotekey: string
+        :param keydata: The contents of the key file.  If this is not
+                        provided, read the data from ``localkey``.
+        :type keydata: string
+        """
+        # this must be be HAS_YUM, not use_yum, because regardless of
         # whether the user wants to use the yum resolver we want to
         # include gpg key data
-        if not has_yum:
+        if not HAS_YUM:
             return
 
+        if keydata is None:
+            keydata = open(localkey).read()
+
         try:
             kinfo = yum.misc.getgpgkeyinfo(keydata)
             version = yum.misc.keyIdToRPMVer(kinfo['keyid'])
@@ -331,47 +492,29 @@ class YumCollection(Collection):
             self.logger.error("Packages: Could not read GPG key %s: %s" %
                               (localkey, err))
 
-    def is_package(self, package):
-        if not self.use_yum:
-            return Collection.is_package(self, package)
-        elif isinstance(package, tuple):
-            if package[1] is None and package[2] == (None, None, None):
-                package = package[0]
-            else:
-                return None
-        else:
-            # this should really never get called; it's just provided
-            # for API completeness
-            return self.call_helper("is_package", package)
-
-    def is_virtual_package(self, package):
-        if not self.use_yum:
-            return Collection.is_virtual_package(self, package)
-        else:
-            # this should really never get called; it's just provided
-            # for API completeness
-            return self.call_helper("is_virtual_package", package)
-
-    def get_deps(self, package):
-        if not self.use_yum:
-            return Collection.get_deps(self, package)
-        else:
-            # this should really never get called; it's just provided
-            # for API completeness
-            return self.call_helper("get_deps", package)
-
-    def get_provides(self, required, all=False, silent=False):
-        if not self.use_yum:
-            return Collection.get_provides(self, required)
-        else:
-            # this should really never get called; it's just provided
-            # for API completeness
-            return self.call_helper("get_provides", required)
-
     def get_groups(self, grouplist):
+        """ If using the yum libraries, given a list of package group
+        names, return a dict of ``<group name>: <list of packages>``.
+        This is much faster than implementing
+        :func:`Bcfg2.Server.Plugins.Packages.Collection.Collection.get_group`,
+        since we have to make a call to the bcfg2 Yum helper, and each
+        time we do that we make another call to yum, which means we
+        set up yum metadata from the cache (hopefully) each time.  So
+        resolving ten groups once is much faster than resolving one
+        group ten times.
+
+        If you are using the builtin yum parser, this raises a warning
+        and returns an empty dict.
+
+        :param grouplist: The list of groups to query
+        :type grouplist: list of strings - group names
+        :returns: dict of ``<group name>: <list of packages>``
+
+        In this implementation the packages may be strings or tuples.
+        See :ref:`yum-pkg-objects` for more information. """
         if not self.use_yum:
-            self.logger.warning("Packages: Package groups are not supported by "
-                                "Bcfg2's internal Yum dependency generator")
+            self.logger.warning("Packages: Package groups are not supported "
+                                "by Bcfg2's internal Yum dependency generator")
             return dict()
 
         if not grouplist:
@@ -387,18 +530,15 @@ class YumCollection(Collection):
 
         return self.call_helper("get_groups", gdicts)
 
-    def get_group(self, group, ptype="default"):
-        if not self.use_yum:
-            self.logger.warning("Packages: Package groups are not supported by "
-                                "Bcfg2's internal Yum dependency generator")
-            return []
-
-        if group.startswith("@"):
-            group = group[1:]
-
-        return self.call_helper("get_group", dict(group=group, type=ptype))
-
     def packages_from_entry(self, entry):
+        """ When using the Python yum libraries, convert a Package
+        entry to a list of package tuples.  See :ref:`yum-pkg-objects`
+        and :ref:`pkg-objects` for more information on this process.
+
+        :param entry: The Package entry to convert
+        :type entry: lxml.etree._Element
+        :returns: list of tuples
+        """
         rv = set()
         name = entry.get("name")
 
@@ -425,6 +565,24 @@ class YumCollection(Collection):
         return list(rv)
 
     def packages_to_entry(self, pkglist, entry):
+        """ When using the Python yum libraries, convert a list of
+        package tuples to a Package entry.  See :ref:`yum-pkg-objects`
+        and :ref:`pkg-objects` for more information on this process.
+
+        If pkglist contains only one package, then its data is
+        converted to a single ``BoundPackage`` entry that is added as
+        a subelement of ``entry``.  If pkglist contains more than one
+        package, then a parent ``BoundPackage`` entry is created and
+        child ``Instance`` entries are added to it.
+
+        :param pkglist: A list of package tuples to convert to an XML
+                         Package entry
+        :type pkglist: list of tuples
+        :param entry: The base XML entry to add Package entries to.
+                      This is modified in place.
+        :type entry: lxml.etree._Element
+        :returns: None
+        """
         def _get_entry_attrs(pkgtup):
             attrs = dict(version=self.setup.cfp.get("packages",
                                                     "version",
@@ -464,6 +622,18 @@ class YumCollection(Collection):
                 lxml.etree.SubElement(entry, 'BoundPackage', **attrs)
 
     def get_new_packages(self, initial, complete):
+        """ Compute the difference between the complete package list
+        (as returned by :func:`complete`) and the initial package list
+        computed from the specification, allowing for package tuples.
+        See :ref:`yum-pkg-objects` and :ref:`pkg-objects` for more
+        information on this process.
+
+        :param initial: The initial package list
+        :type initial: set of strings, but see :ref:`pkg-objects`
+        :param complete: The final package list
+        :type complete: set of strings, but see :ref:`pkg-objects`
+        :return: set of tuples
+        """
         initial_names = []
         for pkg in initial:
             if isinstance(pkg, tuple):
@@ -477,6 +647,22 @@ class YumCollection(Collection):
         return new
 
     def complete(self, packagelist):
+        """ Build a complete list of all packages and their dependencies.
+
+        When using the Python yum libraries, this defers to the
+        :ref:`bcfg2-yum-helper`; when using the builtin yum parser,
+        this defers to
+        :func:`Bcfg2.Server.Plugins.Packages.Collection.Collection.complete`.
+
+        :param packagelist: Set of initial packages computed from the
+                            specification.
+        :type packagelist: set of strings, but see :ref:`pkg-objects`
+        :returns: tuple of sets - The first element contains a set of
+                  strings (but see :ref:`pkg-objects`) describing the
+                  complete package list, and the second element is a
+                  set of symbols whose dependencies could not be
+                  resolved.
+        """
         if not self.use_yum:
             return Collection.complete(self, packagelist)
 
@@ -500,11 +686,21 @@ class YumCollection(Collection):
             return set(), set()
 
     def call_helper(self, command, input=None):
-        """ Make a call to bcfg2-yum-helper.  The yum libs have
+        """ Make a call to :ref:`bcfg2-yum-helper`.  The yum libs have
         horrific memory leaks, so apparently the right way to get
         around that in long-running processes it to have a short-lived
         helper.  No, seriously -- check out the yum-updatesd code.
-        It's pure madness. """
+        It's pure madness.
+
+        :param command: The :ref:`bcfg2-yum-helper` command to call.
+        :type command: string
+        :param input: The input to pass to ``bcfg2-yum-helper`` on
+                      stdin.  If this is None, no input will be given
+                      at all.
+        :type input: Any JSON-encodable data structure.
+        :returns: Varies depending on the return value of the
+                  ``bcfg2-yum-helper`` command.
+        """
         cmd = [self.helper, "-c", self.cfgfile]
         verbose = self.debug_flag or self.setup['verbose']
         if verbose:
@@ -540,6 +736,23 @@ class YumCollection(Collection):
             return None
 
     def setup_data(self, force_update=False):
+        """ Do any collection-level data setup tasks. This is called
+        when sources are loaded or reloaded by
+        :class:`Bcfg2.Server.Plugins.Packages.Packages`.
+
+        If the builtin yum parsers are in use, this defers to
+        :func:`Bcfg2.Server.Plugins.Packages.Collection.Collection.setup_data`.
+        If using the yum Python libraries, this cleans up cached yum
+        metadata, regenerates the server-side yum config (in order to
+        catch any new sources that have been added to this server),
+        and then cleans up cached yum metadata again, in case the new
+        config has any preexisting cache.
+
+        :param force_update: Ignore all local cache and setup data
+                             from its original upstream sources (i.e.,
+                             the package repositories)
+        :type force_update: bool
+        """
         if not self.use_yum:
             return Collection.setup_data(self, force_update)
 
@@ -556,14 +769,29 @@ class YumCollection(Collection):
 
 
 class YumSource(Source):
+    """ Handle yum sources """
+
+    #: :ref:`server-plugins-generators-packages-magic-groups` for
+    #: ``YumSource`` are "yum", "redhat", "centos", and "fedora"
     basegroups = ['yum', 'redhat', 'centos', 'fedora']
+
+    #: YumSource sets the ``type`` on Package entries to "yum"
     ptype = 'yum'
+
+    #: By default,
+    #: :class:`Bcfg2.Server.Plugins.Packages.Source.Source` filters
+    #: out unknown packages that start with "choice", but that doesn't
+    #: mean anything to Yum or RPM.  Instead, we filter out unknown
+    #: packages that start with "rpmlib", although this is likely
+    #: legacy behavior; that would seem to indicate that a package
+    #: required some RPM feature that isn't provided, which is a bad
+    #: thing.  This should probably go away at some point.
     unknown_filter = lambda u: u.startswith("rpmlib")
 
     def __init__(self, basepath, xsource, setup):
         Source.__init__(self, basepath, xsource, setup)
         self.pulp_id = None
-        if has_pulp and xsource.get("pulp_id"):
+        if HAS_PULP and xsource.get("pulp_id"):
             self.pulp_id = xsource.get("pulp_id")
 
             _setup_pulp(self.setup)
@@ -585,11 +813,12 @@ class YumSource(Source):
                 raise SourceInitError(msg)
             except socket.error:
                 err = sys.exc_info()[1]
-                raise SourceInitError("Could not contact Pulp server: %s" % err)
+                raise SourceInitError("Could not contact Pulp server: %s" %
+                                      err)
             except:
                 err = sys.exc_info()[1]
-                raise SourceInitError("Unknown error querying Pulp server: %s" %
-                                      err)
+                raise SourceInitError("Unknown error querying Pulp server: %s"
+                                      % err)
             self.rawurl = "%s/%s" % (PULPCONFIG.cds['baseurl'],
                                      self.repo['relative_path'])
             self.arches = [self.repo['arch']]
@@ -601,14 +830,20 @@ class YumSource(Source):
                              for x in ['global'] + self.arches])
         self.needed_paths = set()
         self.file_to_arch = dict()
+    __init__.__doc__ = Source.__init__.__doc__
 
     @property
     def use_yum(self):
-        return has_yum and self.setup.cfp.getboolean("packages:yum",
+        """ True if we should use the yum Python libraries, False
+        otherwise """
+        return HAS_YUM and self.setup.cfp.getboolean("packages:yum",
                                                      "use_yum_libraries",
                                                      default=False)
 
     def save_state(self):
+        """ If using the builtin yum parser, save state to
+        :attr:`cachefile`.  If using the Python yum libraries, yum
+        handles caching and state and this method is a no-op."""
         if not self.use_yum:
             cache = open(self.cachefile, 'wb')
             cPickle.dump((self.packages, self.deps, self.provides,
@@ -616,6 +851,9 @@ class YumSource(Source):
             cache.close()
 
     def load_state(self):
+        """ If using the builtin yum parser, load saved state from
+        :attr:`cachefile`.  If using the Python yum libraries, yum
+        handles caching and state and this method is a no-op."""
         if not self.use_yum:
             data = open(self.cachefile)
             (self.packages, self.deps, self.provides,
@@ -623,10 +861,26 @@ class YumSource(Source):
 
     @property
     def urls(self):
+        """ A list of URLs to the base metadata file for each
+        repository described by this source. """
         return [self._get_urls_from_repodata(m['url'], m['arch'])
                 for m in self.url_map]
 
     def _get_urls_from_repodata(self, url, arch):
+        """ When using the builtin yum parser, given the base URL of a
+        repository, return the URLs of the various repo metadata files
+        needed to get package data from the repo.
+
+        If using the yum Python libraries, this just returns ``url``
+        as it was passed in, but should realistically not be called.
+
+        :param url: The base URL to the repository (i.e., the
+                    directory that contains the ``repodata/`` directory)
+        :type url: string
+        :param arch: The architecture of the directory.
+        :type arch: string
+        :return: list of strings - URLs to metadata files
+        """
         if self.use_yum:
             return [url]
 
@@ -650,8 +904,8 @@ class YumSource(Source):
             xdata = lxml.etree.XML(repomd)
         except lxml.etree.XMLSyntaxError:
             err = sys.exc_info()[1]
-            self.logger.error("Packages: Failed to process metadata at %s: %s" %
-                              (rmdurl, err))
+            self.logger.error("Packages: Failed to process metadata at %s: %s"
+                              % (rmdurl, err))
             return []
 
         urls = []
@@ -664,6 +918,11 @@ class YumSource(Source):
         return urls
 
     def read_files(self):
+        """ When using the builtin yum parser, read and parse locally
+        downloaded metadata files.  This diverges from the stock
+        :func:`Bcfg2.Server.Plugins.Packages.Source.Source.read_files`
+        quite a bit. """
+
         # we have to read primary.xml first, and filelists.xml afterwards;
         primaries = list()
         filelists = list()
@@ -740,14 +999,15 @@ class YumSource(Source):
                         self.provides[arch][prov] = list()
                     self.provides[arch][prov].append(pkgname)
 
-    def is_package(self, metadata, item):
+    def is_package(self, metadata, package):
         arch = [a for a in self.arches if a in metadata.groups]
         if not arch:
             return False
-        return ((item in self.packages['global'] or
-                 item in self.packages[arch[0]]) and
-                item not in self.blacklist and
-                (len(self.whitelist) == 0 or item in self.whitelist))
+        return ((package in self.packages['global'] or
+                 package in self.packages[arch[0]]) and
+                package not in self.blacklist and
+                (len(self.whitelist) == 0 or package in self.whitelist))
+    is_package.__doc__ = Source.is_package.__doc__
 
     def get_vpkgs(self, metadata):
         if self.use_yum:
@@ -760,6 +1020,7 @@ class YumSource(Source):
             for filename, pkgs in list(fmdata.items()):
                 rv[filename] = pkgs
         return rv
+    get_vpkgs.__doc__ = Source.get_vpkgs.__doc__
 
     def filter_unknown(self, unknown):
         if self.use_yum:
@@ -777,12 +1038,25 @@ class YumSource(Source):
             unknown.difference_update(filtered)
         else:
             Source.filter_unknown(self, unknown)
+    filter_unknown.__doc__ = Source.filter_unknown.__doc__
 
     def setup_data(self, force_update=False):
         if not self.use_yum:
             Source.setup_data(self, force_update=force_update)
+    setup_data.__doc__ = \
+        "``setup_data`` is only used by the builtin yum parser.  " + \
+        Source.setup_data.__doc__
 
     def get_repo_name(self, url_map):
+        """ Try to find a sensible name for a repository.  First use a
+        repository's Pulp ID, if it has one; if not, then defer to
+        :class:`Bcfg2.Server.Plugins.Packages.Source.Source.get_repo_name`
+
+        :param url_map: A single :attr:`url_map` dict, i.e., any
+                        single element of :attr:`url_map`.
+        :type url_map: dict
+        :returns: string - the name of the repository.
+        """
         if self.pulp_id:
             return self.pulp_id
         else:
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
index f7932dd75..c74aa77d9 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
@@ -55,9 +55,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
                                     default=os.path.join(self.data, 'cache'))
 
         #: Where Packages should store downloaded GPG key files
-        self.keypath = \
-            self.core.setup.cfp.get("packages", "keycache",
-                                    default=os.path.join(self.data, 'keys'))
+        self.keypath = os.path.join(self.cachepath, 'keys')
         if not os.path.exists(self.keypath):
             # create key directory if needed
             os.makedirs(self.keypath)