Merge branch '1.4.x'

Conflicts:
	debian/bcfg2-server.install
	doc/server/plugins/grouping/metadata.txt
	src/lib/Bcfg2/Client/Client.py
	src/lib/Bcfg2/Client/Tools/Portage.py
	src/lib/Bcfg2/Client/Tools/RcUpdate.py
	src/lib/Bcfg2/Client/Tools/YUM24.py
	src/lib/Bcfg2/Client/Tools/__init__.py
	src/lib/Bcfg2/Client/Tools/launchd.py
	src/lib/Bcfg2/Options.py
	src/lib/Bcfg2/Server/Core.py
	src/lib/Bcfg2/Server/Plugin/helpers.py
	src/lib/Bcfg2/Server/Plugins/Metadata.py
	src/lib/Bcfg2/Server/models.py
	src/lib/Bcfg2/Utils.py
	src/sbin/bcfg2-info
	src/sbin/bcfg2-test
	testsuite/Testsrc/Testlib/TestClient/TestTools/Test_init.py
	testsuite/Testsrc/test_code_checks.py

7 files changed, 87 insertions, 186 deletions

diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
index 27f493677..57f802bb5 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Apt.py
@@ -13,8 +13,7 @@ class AptCollection(Collection):
     overrides nothing, and defers all operations to :class:`PacSource`
     """
 
-    def __init__(self, metadata, sources, cachepath, basepath, fam,
-                 debug=False):
+    def __init__(self, metadata, sources, cachepath, basepath, debug=False):
         # we define an __init__ that just calls the parent __init__,
         # so that we can set the docstring on __init__ to something
         # different from the parent __init__ -- namely, the parent
@@ -22,7 +21,7 @@ class AptCollection(Collection):
         # which we use to delineate the actual docs from the
         # .. autoattribute hacks we have to do to get private
         # attributes included in sphinx 1.0 """
-        Collection.__init__(self, metadata, sources, cachepath, basepath, fam,
+        Collection.__init__(self, metadata, sources, cachepath, basepath,
                             debug=debug)
     __init__.__doc__ = Collection.__init__.__doc__.split(".. -----")[0]
 
@@ -48,10 +47,6 @@ 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'
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py b/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
index b25cb0fc4..59eefe143 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Collection.py
@@ -78,7 +78,10 @@ import copy
 import logging
 import lxml.etree
 import Bcfg2.Server.Plugin
+from Bcfg2.Server.FileMonitor import get_fam
+from Bcfg2.Options import get_option_parser
 from Bcfg2.Compat import any, md5  # pylint: disable=W0622
+from Bcfg2.Server.Statistics import track_statistics
 
 LOGGER = logging.getLogger(__name__)
 
@@ -93,8 +96,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
     #: Whether or not this Packages backend supports package groups
     __package_groups__ = False
 
-    def __init__(self, metadata, sources, cachepath, basepath, fam,
-                 debug=False):
+    def __init__(self, metadata, sources, cachepath, basepath, debug=False):
         """
         :param metadata: The client metadata for this collection
         :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
@@ -111,9 +113,6 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
                          directory, where more permanent data can be
                          stored
         :type basepath: string
-        :param fam: A file monitor object to use if this Collection
-                    needs to monitor for file activity
-        :type fam: Bcfg2.Server.FileMonitor.FileMonitor
         :param debug: Enable debugging output
         :type debug: bool
 
@@ -127,13 +126,12 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         self.basepath = basepath
         self.cachepath = cachepath
         self.virt_pkgs = dict()
-        self.fam = fam
+        self.fam = get_fam()
+        self.setup = get_option_parser()
 
         try:
-            self.setup = sources[0].setup
             self.ptype = sources[0].ptype
         except IndexError:
-            self.setup = None
             self.ptype = "unknown"
 
     @property
@@ -204,19 +202,6 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         return sorted(list(set(groups)))
 
     @property
-    def basegroups(self):
-        """ Get a list of group names used by this Collection type in
-        resolution of
-        :ref:`server-plugins-generators-packages-magic-groups`.
-
-        The base implementation simply aggregates the results of
-        :attr:`Bcfg2.Server.Plugins.Packages.Source.Source.basegroups`."""
-        groups = set()
-        for source in self:
-            groups.update(source.basegroups)
-        return list(groups)
-
-    @property
     def cachefiles(self):
         """ A list of the full path to all cachefiles used by this
         collection.
@@ -229,7 +214,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
             cachefiles.add(source.cachefile)
         return list(cachefiles)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def get_groups(self, grouplist):
         """ Given a list of package group names, return a dict of
         ``<group name>: <list of packages>``.  This method is provided
@@ -250,7 +235,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
             rv[group] = self.get_group(group, ptype)
         return rv
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def get_group(self, group, ptype=None):
         """ Get the list of packages of the given type in a package
         group.
@@ -386,20 +371,6 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         for source in self:
             source.filter_unknown(unknown)
 
-    def magic_groups_match(self):
-        """ Returns True if the client's
-        :ref:`server-plugins-generators-packages-magic-groups` match
-        the magic groups for any of the sources contained in this
-        Collection.
-
-        The base implementation returns True if any source
-        :func:`Bcfg2.Server.Plugins.Packages.Source.Source.magic_groups_match`
-        returns True.
-
-        :returns: bool
-        """
-        return any(s.magic_groups_match(self.metadata) for s in self)
-
     def build_extra_structures(self, independent):
         """ Add additional entries to the ``<Independent/>`` section
         of the final configuration.  This can be used to handle, e.g.,
@@ -499,7 +470,7 @@ class Collection(list, Bcfg2.Server.Plugin.Debuggable):
         """
         return list(complete.difference(initial))
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def complete(self, packagelist):  # pylint: disable=R0912,R0914
         """ Build a complete list of all packages and their dependencies.
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py b/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
index 99aed5ce5..5f4d2ea41 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Pac.py
@@ -12,8 +12,7 @@ class PacCollection(Collection):
     overrides nothing, and defers all operations to :class:`PacSource`
     """
 
-    def __init__(self, metadata, sources, cachepath, basepath, fam,
-                 debug=False):
+    def __init__(self, metadata, sources, cachepath, basepath, debug=False):
         # we define an __init__ that just calls the parent __init__,
         # so that we can set the docstring on __init__ to something
         # different from the parent __init__ -- namely, the parent
@@ -21,7 +20,7 @@ class PacCollection(Collection):
         # which we use to delineate the actual docs from the
         # .. autoattribute hacks we have to do to get private
         # attributes included in sphinx 1.0 """
-        Collection.__init__(self, metadata, sources, cachepath, basepath, fam,
+        Collection.__init__(self, metadata, sources, cachepath, basepath,
                             debug=debug)
     __init__.__doc__ = Collection.__init__.__doc__.split(".. -----")[0]
 
@@ -29,10 +28,6 @@ class PacCollection(Collection):
 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'
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py b/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py
index 2735e389a..e9744c777 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/PackagesSources.py
@@ -4,6 +4,8 @@
 import os
 import sys
 import Bcfg2.Server.Plugin
+from Bcfg2.Options import get_option_parser
+from Bcfg2.Server.Statistics import track_statistics
 from Bcfg2.Server.Plugins.Packages.Source import SourceInitError
 
 
@@ -15,9 +17,7 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
     :class:`Bcfg2.Server.Plugins.Packages.Source.Source` object for
     each ``Source`` tag. """
 
-    __identifier__ = None
-
-    def __init__(self, filename, cachepath, fam, packages, setup):
+    def __init__(self, filename, cachepath, packages):
         """
         :param filename: The full path to ``sources.xml``
         :type filename: string
@@ -25,22 +25,17 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
                           :class:`Bcfg2.Server.Plugins.Packages.Source.Source`
                           data will be cached
         :type cachepath: string
-        :param fam: The file access monitor to use to create watches
-                    on ``sources.xml`` and any XIncluded files.
-        :type fam: Bcfg2.Server.FileMonitor.FileMonitor
         :param packages: The Packages plugin object ``sources.xml`` is
                          being parsed on behalf of (i.e., the calling
                          object)
         :type packages: Bcfg2.Server.Plugins.Packages.Packages
-        :param setup: A Bcfg2 options dict
-        :type setup: dict
 
         :raises: :class:`Bcfg2.Server.Plugin.exceptions.PluginInitError` -
                  If ``sources.xml`` cannot be read
         """
         Bcfg2.Server.Plugin.Debuggable.__init__(self)
         try:
-            Bcfg2.Server.Plugin.StructFile.__init__(self, filename, fam=fam,
+            Bcfg2.Server.Plugin.StructFile.__init__(self, filename,
                                                     should_monitor=True)
         except OSError:
             err = sys.exc_info()[1]
@@ -62,7 +57,7 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
                 self.logger.error("Could not create Packages cache at %s: %s" %
                                   (self.cachepath, err))
         #: The Bcfg2 options dict
-        self.setup = setup
+        self.setup = get_option_parser()
 
         #: The :class:`Bcfg2.Server.Plugins.Packages.Packages` that
         #: instantiated this ``PackagesSources`` object
@@ -111,7 +106,7 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
         load its data. """
         return sorted(list(self.parsed)) == sorted(self.extras)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def Index(self):
         Bcfg2.Server.Plugin.StructFile.Index(self)
         self.entries = []
@@ -124,7 +119,7 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
         ``Index`` is responsible for calling :func:`source_from_xml`
         for each ``Source`` tag in each file. """
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def source_from_xml(self, xsource):
         """ Create a
         :class:`Bcfg2.Server.Plugins.Packages.Source.Source` subclass
@@ -157,7 +152,7 @@ class PackagesSources(Bcfg2.Server.Plugin.StructFile,
             return None
 
         try:
-            source = cls(self.cachepath, xsource, self.setup)
+            source = cls(self.cachepath, xsource)
         except SourceInitError:
             err = sys.exc_info()[1]
             self.logger.error("Packages: %s" % err)
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Source.py b/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
index 985405e65..5cf90e188 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Source.py
@@ -27,7 +27,6 @@ in your ``Source`` subclass:
 
 * :func:`Source.urls`
 * :func:`Source.read_files`
-* :attr:`Source.basegroups`
 
 Additionally, you may want to consider overriding the following
 methods and attributes:
@@ -51,9 +50,11 @@ import os
 import re
 import sys
 import Bcfg2.Server.Plugin
+from Bcfg2.Options import get_option_parser
 from Bcfg2.Compat import HTTPError, HTTPBasicAuthHandler, \
      HTTPPasswordMgrWithDefaultRealm, install_opener, build_opener, \
      urlopen, cPickle, md5
+from Bcfg2.Server.Statistics import track_statistics
 
 
 def fetch_url(url):
@@ -106,25 +107,18 @@ class Source(Bcfg2.Server.Plugin.Debuggable):  # pylint: disable=R0902
     those features.
     """
 
-    #: The list of
-    #: :ref:`server-plugins-generators-packages-magic-groups` that
-    #: make sources of this type available to clients.
-    basegroups = []
-
     #: 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):  # pylint: disable=R0912
+    def __init__(self, basepath, xsource):  # pylint: disable=R0912
         """
         :param basepath: The base filesystem path under which cache
                          data for this source should be stored
         :type basepath: string
         :param xsource: The XML tag that describes this source
         :type source: lxml.etree._Element
-        :param setup: A Bcfg2 options dict
-        :type setup: dict
         :raises: :class:`Bcfg2.Server.Plugins.Packages.Source.SourceInitError`
         """
         Bcfg2.Server.Plugin.Debuggable.__init__(self)
@@ -137,7 +131,7 @@ class Source(Bcfg2.Server.Plugin.Debuggable):  # pylint: disable=R0902
         self.xsource = xsource
 
         #: A Bcfg2 options dict
-        self.setup = setup
+        self.setup = get_option_parser()
 
         #: A set of package names that are deemed "essential" by this
         #: source
@@ -304,8 +298,7 @@ class Source(Bcfg2.Server.Plugin.Debuggable):  # pylint: disable=R0902
         :return: list of strings - group names
         """
         return sorted(list(set([g for g in metadata.groups
-                                if (g in self.basegroups or
-                                    g in self.groups or
+                                if (g in self.groups or
                                     g in self.arches)])))
 
     def load_state(self):
@@ -328,7 +321,7 @@ class Source(Bcfg2.Server.Plugin.Debuggable):  # pylint: disable=R0902
                       self.essentialpkgs), cache, 2)
         cache.close()
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def setup_data(self, force_update=False):
         """ Perform all data fetching and setup tasks.  For most
         backends, this involves downloading all metadata from the
@@ -632,16 +625,15 @@ class Source(Bcfg2.Server.Plugin.Debuggable):  # pylint: disable=R0902
 
     def applies(self, metadata):
         """ Return true if this source applies to the given client,
-        i.e., the client is in all necessary groups and
-        :ref:`server-plugins-generators-packages-magic-groups`.
+        i.e., the client is in all necessary groups.
 
         :param metadata: The client metadata to check to see if this
                          source applies
         :type metadata: Bcfg2.Server.Plugins.Metadata.ClientMetadata
         :returns: bool
         """
-        # check base groups
-        if not self.magic_groups_match(metadata):
+        # check arch groups
+        if not self.arch_groups_match(metadata):
             return False
 
         # check Group/Client tags from sources.xml
@@ -712,29 +704,13 @@ class Source(Bcfg2.Server.Plugin.Debuggable):  # pylint: disable=R0902
         """
         return []
 
-    def magic_groups_match(self, metadata):
-        """ Returns True if the client's
-        :ref:`server-plugins-generators-packages-magic-groups` match
-        the magic groups this source.  Also returns True if magic
-        groups are off in the configuration and the client's
-        architecture matches (i.e., architecture groups are *always*
-        checked).
+    def arch_groups_match(self, metadata):
+        """ Returns True if the client is in an arch group that
+        matches the arch of this source.
 
         :returns: bool
         """
-        found_arch = False
         for arch in self.arches:
             if arch in metadata.groups:
-                found_arch = True
-                break
-        if not found_arch:
-            return False
-
-        if not self.setup.cfp.getboolean("packages", "magic_groups",
-                                         default=False):
-            return True
-        else:
-            for group in self.basegroups:
-                if group in metadata.groups:
-                    return True
-            return False
+                return True
+        return False
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
index 6b8ed1f7d..4535fb76d 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
@@ -58,8 +58,10 @@ import errno
 import socket
 import logging
 import lxml.etree
-from subprocess import Popen, PIPE
+import Bcfg2.Server.FileMonitor
 import Bcfg2.Server.Plugin
+from Bcfg2.Utils import Executor
+from Bcfg2.Options import get_option_parser
 # pylint: disable=W0622
 from Bcfg2.Compat import StringIO, cPickle, HTTPError, URLError, \
     ConfigParser, any
@@ -67,6 +69,7 @@ from Bcfg2.Compat import StringIO, cPickle, HTTPError, URLError, \
 from Bcfg2.Server.Plugins.Packages.Collection import Collection
 from Bcfg2.Server.Plugins.Packages.Source import SourceInitError, Source, \
      fetch_url
+from Bcfg2.Server.Statistics import track_statistics
 
 LOGGER = logging.getLogger(__name__)
 
@@ -102,17 +105,11 @@ FL = '{http://linux.duke.edu/metadata/filelists}'
 PULPSERVER = None
 PULPCONFIG = None
 
-#: The path to bcfg2-yum-helper
-HELPER = None
-
-
-def _setup_pulp(setup):
+def _setup_pulp():
     """ 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
@@ -123,6 +120,7 @@ def _setup_pulp(setup):
         raise Bcfg2.Server.Plugin.PluginInitError(msg)
 
     if PULPSERVER is None:
+        setup = get_option_parser()
         try:
             username = setup.cfp.get("packages:pulp", "username")
             password = setup.cfp.get("packages:pulp", "password")
@@ -174,7 +172,7 @@ class PulpCertificateSet(Bcfg2.Server.Plugin.EntrySet):
     #: The path to certificates on consumer machines
     certpath = "/etc/pki/consumer/cert.pem"
 
-    def __init__(self, path, fam):
+    def __init__(self, path):
         """
         :param path: The path to the directory where Pulp consumer
                      certificates will be stored
@@ -192,7 +190,7 @@ class PulpCertificateSet(Bcfg2.Server.Plugin.EntrySet):
                              important='true',
                              sensitive='true',
                              paranoid=self.metadata['paranoid'])
-        self.fam = fam
+        self.fam = Bcfg2.Server.FileMonitor.get_fam()
         self.fam.AddMonitor(path, self)
 
     def HandleEvent(self, event):
@@ -271,12 +269,12 @@ class YumCollection(Collection):
     #: :class:`PulpCertificateSet` object used to handle Pulp certs
     pulp_cert_set = None
 
-    def __init__(self, metadata, sources, cachepath, basepath, fam,
-                 debug=False):
-        Collection.__init__(self, metadata, sources, cachepath, basepath, fam,
+    def __init__(self, metadata, sources, cachepath, basepath, debug=False):
+        Collection.__init__(self, metadata, sources, cachepath, basepath,
                             debug=debug)
         self.keypath = os.path.join(self.cachepath, "keys")
 
+        self._helper = None
         if self.use_yum:
             #: Define a unique cache file for this collection to use
             #: for cached yum metadata
@@ -289,11 +287,13 @@ class YumCollection(Collection):
             #: resolving packages with the Python yum libraries
             self.cfgfile = os.path.join(self.cachefile, "yum.conf")
             self.write_config()
+            self.cmd = Executor()
         else:
             self.cachefile = None
+            self.cmd = None
 
         if HAS_PULP and self.has_pulp_sources:
-            _setup_pulp(self.setup)
+            _setup_pulp()
             if self.pulp_cert_set is None:
                 certdir = os.path.join(
                     self.basepath,
@@ -309,7 +309,7 @@ class YumCollection(Collection):
                         self.logger.error("Could not create Pulp consumer "
                                           "cert directory at %s: %s" %
                                           (certdir, err))
-                self.pulp_cert_set = PulpCertificateSet(certdir, self.fam)
+                self.pulp_cert_set = PulpCertificateSet(certdir)
 
     @property
     def __package_groups__(self):
@@ -323,20 +323,18 @@ class YumCollection(Collection):
         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. """
-        global HELPER
-        if not HELPER:
+        if not self._helper:
             try:
-                HELPER = self.setup.cfp.get("packages:yum", "helper")
+                self._helper = self.setup.cfp.get("packages:yum", "helper")
             except (ConfigParser.NoOptionError, ConfigParser.NoSectionError):
                 # first see if bcfg2-yum-helper is in PATH
                 try:
                     self.debug_log("Checking for bcfg2-yum-helper in $PATH")
-                    Popen(['bcfg2-yum-helper'],
-                          stdin=PIPE, stdout=PIPE, stderr=PIPE).wait()
-                    HELPER = 'bcfg2-yum-helper'
+                    self.cmd.run(['bcfg2-yum-helper'])
+                    self._helper = 'bcfg2-yum-helper'
                 except OSError:
-                    HELPER = "/usr/sbin/bcfg2-yum-helper"
-        return HELPER
+                    self._helper = "/usr/sbin/bcfg2-yum-helper"
+        return self._helper
 
     @property
     def use_yum(self):
@@ -361,7 +359,7 @@ class YumCollection(Collection):
             cachefiles.add(self.cachefile)
         return list(cachefiles)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def write_config(self):
         """ Write the server-side config file to :attr:`cfgfile` based
         on the data from :func:`get_config`"""
@@ -463,7 +461,7 @@ class YumCollection(Collection):
             return "# This config was generated automatically by the Bcfg2 " \
                    "Packages plugin\n\n" + buf.getvalue()
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def build_extra_structures(self, independent):
         """ Add additional entries to the ``<Independent/>`` section
         of the final configuration.  This adds several kinds of
@@ -570,7 +568,7 @@ class YumCollection(Collection):
                                         name=self.pulp_cert_set.certpath)
             self.pulp_cert_set.bind_entry(crt, self.metadata)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def _get_pulp_consumer(self, consumerapi=None):
         """ Get a Pulp consumer object for the client.
 
@@ -599,7 +597,7 @@ class YumCollection(Collection):
                               "%s" % err)
         return consumer
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     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
@@ -642,7 +640,7 @@ class YumCollection(Collection):
             self.logger.error("Packages: Could not read GPG key %s: %s" %
                               (localkey, err))
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     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>``.
@@ -816,7 +814,7 @@ class YumCollection(Collection):
                 new.append(pkg)
         return new
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def complete(self, packagelist):
         """ Build a complete list of all packages and their dependencies.
 
@@ -856,7 +854,7 @@ class YumCollection(Collection):
         else:
             return set(), set()
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def call_helper(self, command, inputdata=None):
         """ Make a call to :ref:`bcfg2-yum-helper`.  The yum libs have
         horrific memory leaks, so apparently the right way to get
@@ -879,28 +877,18 @@ class YumCollection(Collection):
             cmd.append("-v")
         cmd.append(command)
         self.debug_log("Packages: running %s" % " ".join(cmd), flag=verbose)
-        try:
-            helper = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
-        except OSError:
-            err = sys.exc_info()[1]
-            self.logger.error("Packages: Failed to execute %s: %s" %
-                              (" ".join(cmd), err))
-            return None
-
         if inputdata:
-            idata = json.dumps(inputdata)
-            (stdout, stderr) = helper.communicate(idata)
-        else:
-            (stdout, stderr) = helper.communicate()
-        rv = helper.wait()
-        if rv:
-            self.logger.error("Packages: error running bcfg2-yum-helper "
-                              "(returned %d): %s" % (rv, stderr))
+            result = self.cmd.run(cmd, inputdata=json.dumps(inputdata))
         else:
+            result = self.cmd.run(cmd)
+        if not result.success:
+            self.logger.error("Packages: error running bcfg2-yum-helper: %s" %
+                              result.error)
+        elif result.stderr:
             self.debug_log("Packages: debug info from bcfg2-yum-helper: %s" %
-                           stderr, flag=verbose)
+                           result.stderr, flag=verbose)
         try:
-            return json.loads(stdout)
+            return json.loads(result.stdout)
         except ValueError:
             err = sys.exc_info()[1]
             self.logger.error("Packages: error reading bcfg2-yum-helper "
@@ -943,20 +931,16 @@ 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'
 
-    def __init__(self, basepath, xsource, setup):
-        Source.__init__(self, basepath, xsource, setup)
+    def __init__(self, basepath, xsource):
+        Source.__init__(self, basepath, xsource)
         self.pulp_id = None
         if HAS_PULP and xsource.get("pulp_id"):
             self.pulp_id = xsource.get("pulp_id")
 
-            _setup_pulp(self.setup)
+            _setup_pulp()
             repoapi = RepositoryAPI()
             try:
                 self.repo = repoapi.repository(self.pulp_id)
@@ -1083,7 +1067,7 @@ class YumSource(Source):
                 self.file_to_arch[self.escape_url(fullurl)] = arch
         return urls
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def read_files(self):
         """ When using the builtin yum parser, read and parse locally
         downloaded metadata files.  This diverges from the stock
@@ -1131,7 +1115,7 @@ class YumSource(Source):
                 self.packages[key].difference(self.packages['global'])
         self.save_state()
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def parse_filelist(self, data, arch):
         """ parse filelists.xml.gz data """
         if arch not in self.filemap:
@@ -1145,7 +1129,7 @@ class YumSource(Source):
                         self.filemap[arch][fentry.text] = \
                             set([pkg.get('name')])
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def parse_primary(self, data, arch):
         """ parse primary.xml.gz data """
         if arch not in self.packages:
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
index 308a0efc4..2175cf0aa 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/__init__.py
@@ -14,6 +14,7 @@ from Bcfg2.Compat import ConfigParser, urlopen, HTTPError
 from Bcfg2.Server.Plugins.Packages.Collection import Collection, \
     get_collection_class
 from Bcfg2.Server.Plugins.Packages.PackagesSources import PackagesSources
+from Bcfg2.Server.Statistics import track_statistics
 
 #: The default path for generated yum configs
 YUM_CONFIG_DEFAULT = "/etc/yum.repos.d/bcfg2.repo"
@@ -65,14 +66,6 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
             # create key directory if needed
             os.makedirs(self.keypath)
 
-        # warn about deprecated magic groups
-        if self.core.setup.cfp.getboolean("packages", "magic_groups",
-                                          default=False):
-            self.logger.warning("Packages: Magic groups are deprecated and "
-                                "will be removed in a future release")
-            self.logger.warning("You can disable magic groups by setting "
-                                "magic_groups=0 in [packages] in bcfg2.conf")
-
         # pylint: disable=C0301
         #: The
         #: :class:`Bcfg2.Server.Plugins.Packages.PackagesSources.PackagesSources`
@@ -80,8 +73,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         #: :class:`Bcfg2.Server.Plugins.Packages.Source.Source` objects for
         #: this plugin.
         self.sources = PackagesSources(os.path.join(self.data, "sources.xml"),
-                                       self.cachepath, core.fam, self,
-                                       self.core.setup)
+                                       self.cachepath, self)
 
         #: We cache
         #: :class:`Bcfg2.Server.Plugins.Packages.Collection.Collection`
@@ -231,13 +223,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         :raises: :class:`Bcfg2.Server.Plugin.exceptions.PluginExecutionError`
         """
         if entry.tag == 'Package':
-            if self.core.setup.cfp.getboolean("packages", "magic_groups",
-                                              default=False):
-                collection = self.get_collection(metadata)
-                if collection.magic_groups_match():
-                    return True
-            else:
-                return True
+            return True
         elif entry.tag == 'Path':
             # managed entries for yum/apt configs
             if (entry.get("name") == \
@@ -251,7 +237,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
                 return True
         return False
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def validate_structures(self, metadata, structures):
         """ Do the real work of Packages.  This does two things:
 
@@ -286,7 +272,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         collection.build_extra_structures(indep)
         structures.append(indep)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def _build_packages(self, metadata, independent, structures,
                         collection=None):
         """ Perform dependency resolution and build the complete list
@@ -364,7 +350,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         newpkgs.sort()
         collection.packages_to_entry(newpkgs, independent)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def Refresh(self):
         """ Packages.Refresh() => True|False
 
@@ -372,7 +358,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         self._load_config(force_update=True)
         return True
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def Reload(self):
         """ Packages.Refresh() => True|False
 
@@ -468,7 +454,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
             if kfile not in keyfiles:
                 os.unlink(kfile)
 
-    @Bcfg2.Server.Plugin.track_statistics()
+    @track_statistics()
     def get_collection(self, metadata):
         """ Get a
         :class:`Bcfg2.Server.Plugins.Packages.Collection.Collection`
@@ -485,8 +471,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
         if not self.sources.loaded:
             # if sources.xml has not received a FAM event yet, defer;
             # instantiate a dummy Collection object
-            return Collection(metadata, [], self.cachepath, self.data,
-                              self.core.fam)
+            return Collection(metadata, [], self.cachepath, self.data)
 
         if metadata.hostname in self.clients:
             return self.collections[self.clients[metadata.hostname]]
@@ -517,7 +502,7 @@ class Packages(Bcfg2.Server.Plugin.Plugin,
                               "for %s" % (cclass.__name__, metadata.hostname))
 
         collection = cclass(metadata, relevant, self.cachepath, self.data,
-                            self.core.fam, debug=self.debug_flag)
+                            debug=self.debug_flag)
         ckey = collection.cachekey
         self.clients[metadata.hostname] = ckey
         self.collections[ckey] = collection