Merge pull request #23915 from terminalmage/issue7772

pkg.installed: Check binary pkg metadata to determine action

salt/modules/pkg_resource.py

@@ -7,6 +7,7 @@ Resources needed by pkg providers
 from __future__ import absolute_import
 import fnmatch
 import logging
+import os
 import pprint
 
 # Import third party libs
@@ -15,17 +16,21 @@ import salt.ext.six as six
 
 # Import salt libs
 import salt.utils
+from salt.exceptions import SaltInvocationError
 
 log = logging.getLogger(__name__)
 __SUFFIX_NOT_NEEDED = ('x86_64', 'noarch')
 
 
-def _repack_pkgs(pkgs):
+def _repack_pkgs(pkgs, normalize=True):
     '''
     Repack packages specified using "pkgs" argument to pkg states into a single
     dictionary
     '''
-    _normalize_name = __salt__.get('pkg.normalize_name', lambda pkgname: pkgname)
+    if normalize and 'pkg.normalize_name' in __salt__:
+        _normalize_name = __salt__['pkg.normalize_name']
+    else:
+        _normalize_name = lambda pkgname: pkgname
     return dict(
         [
             (_normalize_name(str(x)), str(y) if y is not None else y)
@@ -34,7 +39,7 @@ def _repack_pkgs(pkgs):
     )
 
 
-def pack_sources(sources):
+def pack_sources(sources, normalize=True):
     '''
     Accepts list of dicts (or a string representing a list of dicts) and packs
     the key/value pairs into a single dict.
@@ -42,13 +47,27 @@ def pack_sources(sources):
     ``'[{"foo": "salt://foo.rpm"}, {"bar": "salt://bar.rpm"}]'`` would become
     ``{"foo": "salt://foo.rpm", "bar": "salt://bar.rpm"}``
 
+    normalize : True
+        Normalize the package name by removing the architecture, if the
+        architecture of the package is different from the architecture of the
+        operating system. The ability to disable this behavior is useful for
+        poorly-created packages which include the architecture as an actual
+        part of the name, such as kernel modules which match a specific kernel
+        version.
+
+        .. versionadded:: Beryllium
+
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' pkg_resource.pack_sources '[{"foo": "salt://foo.rpm"}, {"bar": "salt://bar.rpm"}]'
     '''
-    _normalize_name = __salt__.get('pkg.normalize_name', lambda pkgname: pkgname)
+    if normalize and 'pkg.normalize_name' in __salt__:
+        _normalize_name = __salt__['pkg.normalize_name']
+    else:
+        _normalize_name = lambda pkgname: pkgname
+
     if isinstance(sources, six.string_types):
         try:
             sources = yaml.safe_load(sources)
@@ -102,32 +121,38 @@ def parse_targets(name=None,
         return None, None
 
     elif pkgs:
-        pkgs = _repack_pkgs(pkgs)
+        pkgs = _repack_pkgs(pkgs, normalize=normalize)
         if not pkgs:
             return None, None
         else:
             return pkgs, 'repository'
 
     elif sources and __grains__['os'] != 'MacOS':
-        sources = pack_sources(sources)
+        sources = pack_sources(sources, normalize=normalize)
         if not sources:
             return None, None
 
         srcinfo = []
         for pkg_name, pkg_src in six.iteritems(sources):
             if __salt__['config.valid_fileproto'](pkg_src):
-                # Cache package from remote source (salt master, HTTP, FTP)
-                srcinfo.append((pkg_name,
-                                pkg_src,
-                               __salt__['cp.cache_file'](pkg_src, saltenv),
-                               'remote'))
+                # Cache package from remote source (salt master, HTTP, FTP) and
+                # append a tuple containing the cached path along with the
+                # specified version.
+                srcinfo.append((
+                    __salt__['cp.cache_file'](pkg_src[0], saltenv),
+                    pkg_src[1]
+                ))
             else:
-                # Package file local to the minion
-                srcinfo.append((pkg_name, pkg_src, pkg_src, 'local'))
+                # Package file local to the minion, just append the tuple from
+                # the pack_sources() return data.
+                if not os.path.isabs(pkg_src[0]):
+                    raise SaltInvocationError(
+                        'Path {0} for package {1} is either not absolute or '
+                        'an invalid protocol'.format(pkg_src[0], pkg_name)
+                    )
+                srcinfo.append(pkg_src)
 
-        # srcinfo is a 4-tuple (pkg_name,pkg_uri,pkg_path,pkg_type), so grab
-        # the package path (3rd element of tuple).
-        return [x[2] for x in srcinfo], 'file'
+        return srcinfo, 'file'
 
     elif name:
         if normalize:
@@ -180,7 +205,7 @@ def version(*names, **kwargs):
     return ret
 
 
-def add_pkg(pkgs, name, version):
+def add_pkg(pkgs, name, pkgver):
     '''
     Add a package to a dict of installed packages.
 
@@ -191,7 +216,7 @@ def add_pkg(pkgs, name, version):
         salt '*' pkg_resource.add_pkg '{}' bind 9
     '''
     try:
-        pkgs.setdefault(name, []).append(version)
+        pkgs.setdefault(name, []).append(pkgver)
     except AttributeError as exc:
         log.exception(exc)
 
@@ -237,7 +262,7 @@ def stringify(pkgs):
         log.exception(exc)
 
 
-def version_clean(version):
+def version_clean(verstr):
     '''
     Clean the version string removing extra data.
     This function will simply try to call ``pkg.version_clean``.
@@ -248,16 +273,16 @@ def version_clean(version):
 
         salt '*' pkg_resource.version_clean <version_string>
     '''
-    if version and 'pkg.version_clean' in __salt__:
-        return __salt__['pkg.version_clean'](version)
-
-    return version
+    if verstr and 'pkg.version_clean' in __salt__:
+        return __salt__['pkg.version_clean'](verstr)
+    return verstr
 
 
 def check_extra_requirements(pkgname, pkgver):
     '''
     Check if the installed package already has the given requirements.
-    This function will simply try to call "pkg.check_extra_requirements".
+    This function will return the result of ``pkg.check_extra_requirements`` if
+    this function exists for the minion, otherwise it will return True.
 
     CLI Example:
 

salt/modules/rpm.py

@@ -6,12 +6,20 @@ Support for rpm
 # Import python libs
 from __future__ import absolute_import
 import logging
+import os
 import re
 
 # Import Salt libs
 import salt.utils
 import salt.utils.decorators as decorators
-from salt.ext.six.moves import zip  # pylint: disable=import-error,redefined-builtin
+import salt.utils.pkg.rpm
+# pylint: disable=import-error,redefined-builtin
+from salt.ext.six.moves import (
+    zip,
+    shlex_quote as _cmd_quote
+)
+# pylint: enable=import-error,redefined-builtin
+from salt.exceptions import CommandExecutionError, SaltInvocationError
 
 log = logging.getLogger(__name__)
 
@@ -38,6 +46,60 @@ def __virtual__():
     return False
 
 
+def bin_pkg_info(path, saltenv='base'):
+    '''
+    .. versionadded:: Beryllium
+
+    Parses RPM metadata and returns a dictionary of information about the
+    package (name, version, etc.).
+
+    path
+        Path to the file. Can either be an absolute path to a file on the
+        minion, or a salt fileserver URL (e.g. ``salt://path/to/file.rpm``).
+        If a salt fileserver URL is passed, the file will be cached to the
+        minion so that it can be examined.
+
+    saltenv : base
+        Salt fileserver envrionment from which to retrieve the package. Ignored
+        if ``path`` is a local file path on the minion.
+    '''
+    # If the path is a valid protocol, pull it down using cp.cache_file
+    if __salt__['config.valid_fileproto'](path):
+        newpath = __salt__['cp.cache_file'](path, saltenv)
+        if not newpath:
+            raise CommandExecutionError(
+                'Unable to retrieve {0} from saltenv \'{1}'
+                .format(path, saltenv)
+            )
+        path = newpath
+    else:
+        if not os.path.exists(path):
+            raise CommandExecutionError(
+                '{0} does not exist on minion'.format(path)
+            )
+        elif not os.path.isabs(path):
+            raise SaltInvocationError(
+                '{0} does not exist on minion'.format(path)
+            )
+
+    # REPOID is not a valid tag for the rpm command. Remove it and replace it
+    # with 'none'
+    queryformat = salt.utils.pkg.rpm.QUERYFORMAT.replace('%{REPOID}', 'none')
+    output = __salt__['cmd.run_stdout'](
+        'rpm -qp --queryformat {0} {1}'.format(_cmd_quote(queryformat), path),
+        output_loglevel='trace',
+        ignore_retcode=True
+    )
+    ret = {}
+    pkginfo = salt.utils.pkg.rpm.parse_pkginfo(
+        output,
+        osarch=__grains__['osarch']
+    )
+    for field in pkginfo._fields:
+        ret[field] = getattr(pkginfo, field)
+    return ret
+
+
 def list_pkgs(*packages):
     '''
     List the packages currently installed in a dict::

salt/modules/yumpkg.py

@@ -38,32 +38,13 @@ except ImportError:
 # Import salt libs
 import salt.utils
 import salt.utils.decorators as decorators
+import salt.utils.pkg.rpm
 from salt.exceptions import (
     CommandExecutionError, MinionError, SaltInvocationError
 )
 
 log = logging.getLogger(__name__)
 
-__QUERYFORMAT = '%{NAME}_|-%{VERSION}_|-%{RELEASE}_|-%{ARCH}_|-%{REPOID}'
-
-# These arches compiled from the rpmUtils.arch python module source
-__ARCHES_64 = ('x86_64', 'athlon', 'amd64', 'ia32e', 'ia64', 'geode')
-__ARCHES_32 = ('i386', 'i486', 'i586', 'i686')
-__ARCHES_PPC = ('ppc', 'ppc64', 'ppc64iseries', 'ppc64pseries')
-__ARCHES_S390 = ('s390', 's390x')
-__ARCHES_SPARC = (
-    'sparc', 'sparcv8', 'sparcv9', 'sparcv9v', 'sparc64', 'sparc64v'
-)
-__ARCHES_ALPHA = (
-    'alpha', 'alphaev4', 'alphaev45', 'alphaev5', 'alphaev56',
-    'alphapca56', 'alphaev6', 'alphaev67', 'alphaev68', 'alphaev7'
-)
-__ARCHES_ARM = ('armv5tel', 'armv5tejl', 'armv6l', 'armv7l')
-__ARCHES_SH = ('sh3', 'sh4', 'sh4a')
-
-__ARCHES = __ARCHES_64 + __ARCHES_32 + __ARCHES_PPC + __ARCHES_S390 + \
-    __ARCHES_ALPHA + __ARCHES_ARM + __ARCHES_SH
-
 # Define the module's virtual name
 __virtualname__ = 'pkg'
 
@@ -87,41 +68,16 @@ def __virtual__():
     return False
 
 
-def _parse_pkginfo(line):
-    '''
-    A small helper to parse a repoquery; returns a namedtuple
-    '''
-    # Importing `collections` here since this function is re-namespaced into
-    # another module
-    import collections
-    pkginfo = collections.namedtuple(
-        'PkgInfo',
-        ('name', 'version', 'arch', 'repoid')
-    )
-
-    try:
-        name, pkg_version, release, arch, repoid = line.split('_|-')
-    # Handle unpack errors (should never happen with the queryformat we are
-    # using, but can't hurt to be careful).
-    except ValueError:
-        return None
-
-    if not _check_32(arch):
-        if arch not in (__grains__['osarch'], 'noarch'):
-            name += '.{0}'.format(arch)
-    if release:
-        pkg_version += '-{0}'.format(release)
-
-    return pkginfo(name, pkg_version, arch, repoid)
-
-
 def _repoquery_pkginfo(repoquery_args):
     '''
     Wrapper to call repoquery and parse out all the tuples
     '''
     ret = []
     for line in _repoquery(repoquery_args, ignore_stderr=True):
-        pkginfo = _parse_pkginfo(line)
+        pkginfo = salt.utils.pkg.rpm.parse_pkginfo(
+            line,
+            osarch=__grains__['osarch']
+        )
         if pkginfo is not None:
             ret.append(pkginfo)
     return ret
@@ -143,7 +99,9 @@ def _check_repoquery():
             raise CommandExecutionError('Unable to install yum-utils')
 
 
-def _repoquery(repoquery_args, query_format=__QUERYFORMAT, ignore_stderr=False):
+def _repoquery(repoquery_args,
+               query_format=salt.utils.pkg.rpm.QUERYFORMAT,
+               ignore_stderr=False):
     '''
     Runs a repoquery command and returns a list of namedtuples
     '''
@@ -154,8 +112,8 @@ def _repoquery(repoquery_args, query_format=__QUERYFORMAT, ignore_stderr=False):
     call = __salt__['cmd.run_all'](cmd, output_loglevel='trace')
     if call['retcode'] != 0:
         comment = ''
-        # when checking for packages some yum modules return data via
-        # stderr that don't cause non-zero return codes.  A perfect
+        # When checking for packages some yum modules return data via
+        # stderr that don't cause non-zero return codes. j perfect
         # example of this is when spacewalk is installed but not yet
         # registered. We should ignore those when getting pkginfo.
         if 'stderr' in call and not salt.utils.is_true(ignore_stderr):
@@ -241,40 +199,6 @@ def _get_branch_option(**kwargs):
     return branch_arg
 
 
-def _check_32(arch):
-    '''
-    Returns True if both the OS arch and the passed arch are 32-bit
-    '''
-    return all(x in __ARCHES_32 for x in (__grains__['osarch'], arch))
-
-
-def _rpm_pkginfo(name):
-    '''
-    Parses RPM metadata and returns a pkginfo namedtuple
-    '''
-    # REPOID is not a valid tag for the rpm command. Remove it and replace it
-    # with 'none'
-    queryformat = __QUERYFORMAT.replace('%{REPOID}', 'none')
-    output = __salt__['cmd.run_stdout'](
-        'rpm -qp --queryformat {0!r} {1}'.format(_cmd_quote(queryformat), name),
-        output_loglevel='trace',
-        ignore_retcode=True
-    )
-    return _parse_pkginfo(output)
-
-
-def _rpm_installed(name):
-    '''
-    Parses RPM metadata to determine if the RPM target is already installed.
-    Returns the name of the installed package if found, otherwise None.
-    '''
-    pkg = _rpm_pkginfo(name)
-    try:
-        return pkg.name if pkg.name in list_pkgs() else None
-    except AttributeError:
-        return None
-
-
 def _get_yum_config():
     '''
     Returns a dict representing the yum config options and values.
@@ -391,11 +315,12 @@ def normalize_name(name):
     '''
     try:
         arch = name.rsplit('.', 1)[-1]
-        if arch not in __ARCHES + ('noarch',):
+        if arch not in salt.utils.pkg.rpm.ARCHES + ('noarch',):
             return name
     except ValueError:
         return name
-    if arch in (__grains__['osarch'], 'noarch') or _check_32(arch):
+    if arch in (__grains__['osarch'], 'noarch') \
+            or salt.utils.pkg.rpm.check_32(arch, osarch=__grains__['osarch']):
         return name[:-(len(arch) + 1)]
     return name
 
@@ -449,7 +374,7 @@ def latest_version(*names, **kwargs):
         ret[name] = ''
         try:
             arch = name.rsplit('.', 1)[-1]
-            if arch not in __ARCHES:
+            if arch not in salt.utils.pkg.rpm.ARCHES:
                 arch = __grains__['osarch']
         except ValueError:
             arch = __grains__['osarch']
@@ -476,7 +401,7 @@ def latest_version(*names, **kwargs):
     for name in names:
         for pkg in (x for x in updates if x.name == name):
             if pkg.arch == 'noarch' or pkg.arch == namearch_map[name] \
-                    or _check_32(pkg.arch):
+                    or salt.utils.pkg.rpm.check_32(pkg.arch):
                 ret[name] = pkg.version
                 # no need to check another match, if there was one
                 break
@@ -926,14 +851,12 @@ def install(name=None,
         architecture as an actual part of the name such as kernel modules
         which match a specific kernel version.
 
+        .. code-block:: bash
+
+            salt -G role:nsd pkg.install gpfs.gplbin-2.6.32-279.31.1.el6.x86_64 normalize=False
+
         .. versionadded:: 2014.7.0
 
-    Example:
-
-    .. code-block:: bash
-
-        salt -G role:nsd pkg.install gpfs.gplbin-2.6.32-279.31.1.el6.x86_64 normalize=False
-
 
     Returns a dict containing the new package names and versions::
 
@@ -976,36 +899,63 @@ def install(name=None,
     else:
         pkg_params_items = []
         for pkg_source in pkg_params:
-            rpm_info = _rpm_pkginfo(pkg_source)
-            if rpm_info is not None:
-                pkg_params_items.append([rpm_info.name, rpm_info.version, pkg_source])
+            if 'lowpkg.bin_pkg_info' in __salt__:
+                rpm_info = __salt__['lowpkg.bin_pkg_info'](pkg_source)
             else:
-                pkg_params_items.append([pkg_source, None, pkg_source])
+                rpm_info = None
+            if rpm_info is None:
+                log.error(
+                    'pkg.install: Unable to get rpm information for {0}. '
+                    'Version comparisons will be unavailable, and return '
+                    'data may be inaccurate if reinstall=True.'
+                    .format(pkg_source)
+                )
+                pkg_params_items.append([pkg_source])
+            else:
+                pkg_params_items.append(
+                    [rpm_info['name'], pkg_source, rpm_info['version']]
+                )
 
     for pkg_item_list in pkg_params_items:
-        pkgname = pkg_item_list[0]
-        version_num = pkg_item_list[1]
-        if version_num is None:
-            if reinstall and pkg_type == 'repository' and pkgname in old:
-                to_reinstall[pkgname] = pkgname
-            else:
-                targets.append(pkgname)
+        if pkg_type == 'repository':
+            pkgname, version_num = pkg_item_list
         else:
-            cver = old.get(pkgname, '')
-            arch = ''
             try:
-                namepart, archpart = pkgname.rsplit('.', 1)
+                pkgname, pkgpath, version_num = pkg_item_list
             except ValueError:
-                pass
-            else:
-                if archpart in __ARCHES:
-                    arch = '.' + archpart
-                    pkgname = namepart
+                pkgname = None
+                pkgpath = pkg_item_list[0]
+                version_num = None
 
+        if version_num is None:
             if pkg_type == 'repository':
+                if reinstall and pkgname in old:
+                    to_reinstall[pkgname] = pkgname
+                else:
+                    targets.append(pkgname)
+            else:
+                targets.append(pkgpath)
+        else:
+            # If we are installing a package file and not one from the repo,
+            # and version_num is not None, then we can assume that pkgname is
+            # not None, since the only way version_num is not None is if RPM
+            # metadata parsing was successful.
+            if pkg_type == 'repository':
+                arch = ''
+                try:
+                    namepart, archpart = pkgname.rsplit('.', 1)
+                except ValueError:
+                    pass
+                else:
+                    if archpart in salt.utils.pkg.rpm.ARCHES:
+                        arch = '.' + archpart
+                        pkgname = namepart
+
                 pkgstr = '"{0}-{1}{2}"'.format(pkgname, version_num, arch)
             else:
-                pkgstr = pkg_item_list[2]
+                pkgstr = pkgpath
+
+            cver = old.get(pkgname, '')
             if reinstall and cver \
                     and salt.utils.compare_versions(ver1=version_num,
                                                     oper='==',
@@ -1051,25 +1001,14 @@ def install(name=None,
     __context__.pop('pkg.list_pkgs', None)
     new = list_pkgs()
 
-    versionName = pkgname
     ret = salt.utils.compare_dicts(old, new)
 
-    if sources is not None:
-        versionName = pkgname + '-' + new.get(pkgname, '')
-        if pkgname in ret:
-            ret[versionName] = ret.pop(pkgname)
     for pkgname in to_reinstall:
-        if not versionName not in old:
-            ret.update({versionName: {'old': old.get(pkgname, ''),
+        if pkgname not in ret or pkgname in old:
+            ret.update({pkgname: {'old': old.get(pkgname, ''),
                                   'new': new.get(pkgname, '')}})
-        else:
-            if versionName not in ret:
-                ret.update({versionName: {'old': old.get(pkgname, ''),
-                                      'new': new.get(pkgname, '')}})
     if ret:
         __context__.pop('pkg._avail', None)
-    elif sources is not None:
-        ret = {versionName: {}}
     return ret
 
 

salt/states/pkg.py

@@ -122,6 +122,27 @@ def __gen_rtag():
     return os.path.join(__opts__['cachedir'], 'pkg_refresh')
 
 
+def _get_comparison_spec(pkgver):
+    '''
+    Return a tuple containing the comparison operator and the version. If no
+    comparison operator was passed, the comparison is assumed to be an "equals"
+    comparison, and "==" will be the operator returned.
+    '''
+    match = re.match('^([<>])?(=)?([^<>=]+)$', pkgver)
+    if not match:
+        raise CommandExecutionError(
+            'Invalid version specification \'{0}\'.'.format(pkgver)
+        )
+    gt_lt, eq, verstr = match.groups()
+    oper = gt_lt or ''
+    oper += eq or ''
+    # A comparison operator of "=" is redundant, but possible.
+    # Change it to "==" so that the version comparison works
+    if oper in ('=', ''):
+        oper = '=='
+    return oper, verstr
+
+
 def _fulfills_version_spec(versions, oper, desired_version):
     '''
     Returns True if any of the installed versions match the specified version,
@@ -152,6 +173,7 @@ def _find_unpurge_targets(desired):
 def _find_remove_targets(name=None,
                          version=None,
                          pkgs=None,
+                         normalize=True,
                          **kwargs):
     '''
     Inspect the arguments to pkg.removed and discover what packages need to
@@ -159,7 +181,7 @@ def _find_remove_targets(name=None,
     '''
     cur_pkgs = __salt__['pkg.list_pkgs'](versions_as_list=True, **kwargs)
     if pkgs:
-        to_remove = _repack_pkgs(pkgs)
+        to_remove = _repack_pkgs(pkgs, normalize=normalize)
 
         if not to_remove:
             # Badly-formatted SLS
@@ -190,27 +212,19 @@ def _find_remove_targets(name=None,
             targets.append(pkgname)
             continue
         version_spec = True
-        match = re.match('^([<>])?(=)?([^<>=]+)$', pkgver)
-        if not match:
-            msg = 'Invalid version specification {0!r} for package ' \
-                  '{1!r}.'.format(pkgver, pkgname)
-            problems.append(msg)
+        try:
+            oper, verstr = _get_comparison_spec(pkgver)
+        except CommandExecutionError as exc:
+            problems.append(exc.strerror)
+            continue
+        if not _fulfills_version_spec(cver, oper, verstr):
+            log.debug(
+                'Current version ({0}) did not match desired version '
+                'specification ({1}), will not remove'
+                .format(cver, verstr)
+            )
         else:
-            gt_lt, eq, verstr = match.groups()
-            comparison = gt_lt or ''
-            comparison += eq or ''
-            # A comparison operator of "=" is redundant, but possible.
-            # Change it to "==" so that the version comparison works
-            if comparison in ['=', '']:
-                comparison = '=='
-            if not _fulfills_version_spec(cver, comparison, verstr):
-                log.debug(
-                    'Current version ({0}) did not match ({1}) specified '
-                    '({2}), skipping remove {3}'
-                    .format(cver, comparison, verstr, pkgname)
-                )
-            else:
-                targets.append(pkgname)
+            targets.append(pkgname)
 
     if problems:
         return {'name': name,
@@ -221,7 +235,7 @@ def _find_remove_targets(name=None,
     if not targets:
         # All specified packages are already absent
         msg = (
-            'All specified packages{0} are already absent.'
+            'All specified packages{0} are already absent'
             .format(' (matching specified versions)' if version_spec else '')
         )
         return {'name': name,
@@ -269,14 +283,17 @@ def _find_install_targets(name=None,
         if pkgs:
             desired = _repack_pkgs(pkgs)
         elif sources:
-            desired = __salt__['pkg_resource.pack_sources'](sources)
+            desired = __salt__['pkg_resource.pack_sources'](
+                sources,
+                normalize=normalize,
+            )
 
         if not desired:
             # Badly-formatted SLS
             return {'name': name,
                     'changes': {},
                     'result': False,
-                    'comment': 'Invalidly formatted {0!r} parameter. See '
+                    'comment': 'Invalidly formatted \'{0}\' parameter. See '
                                'minion log.'.format('pkgs' if pkgs
                                                     else 'sources')}
         to_unpurge = _find_unpurge_targets(desired)
@@ -308,8 +325,8 @@ def _find_install_targets(name=None,
                 return {'name': name,
                         'changes': {},
                         'result': True,
-                        'comment': 'Version {0} of package {1!r} is already '
-                                   'installed.'.format(version, name)}
+                        'comment': 'Version {0} of package \'{1}\' is already '
+                                   'installed'.format(version, name)}
 
             # if cver is not an empty string, the package is already installed
             elif cver and version is None and not pkg_verify:
@@ -318,25 +335,12 @@ def _find_install_targets(name=None,
                         'changes': {},
                         'result': True,
                         'comment': 'Package {0} is already '
-                                   'installed.'.format(name)}
+                                   'installed'.format(name)}
 
     version_spec = False
-    # Find out which packages will be targeted in the call to pkg.install
-    if sources:
-        targets = []
-        to_reinstall = []
-        for x in desired:
-            if x not in cur_pkgs:
-                targets.append(x)
-            elif pkg_verify:
-                retval = __salt__['pkg.verify'](x, ignore_types=ignore_types)
-                if retval:
-                    to_reinstall.append(x)
-                    altered_files[x] = retval
-    else:
+    if not sources:
         # Check for alternate package names if strict processing is not
-        # enforced.
-        # Takes extra time. Disable for improved performance
+        # enforced. Takes extra time. Disable for improved performance
         if not skip_suggestions:
             # Perform platform-specific pre-flight checks
             problems = _preflight_check(desired, **kwargs)
@@ -350,7 +354,7 @@ def _find_install_targets(name=None,
             if problems.get('suggest'):
                 for pkgname, suggestions in six.iteritems(problems['suggest']):
                     comments.append(
-                        'Package {0!r} not found (possible matches: {1})'
+                        'Package \'{0}\' not found (possible matches: {1})'
                         .format(pkgname, ', '.join(suggestions))
                     )
             if comments:
@@ -361,78 +365,103 @@ def _find_install_targets(name=None,
                         'result': False,
                         'comment': '. '.join(comments).rstrip()}
 
-        # Check current versions against desired versions
-        targets = {}
-        to_reinstall = {}
-        problems = []
-        for pkgname, pkgver in six.iteritems(desired):
-            cver = cur_pkgs.get(pkgname, [])
-            # Package not yet installed, so add to targets
-            if not cver:
-                targets[pkgname] = pkgver
+    # Find out which packages will be targeted in the call to pkg.install
+    targets = {}
+    to_reinstall = {}
+    problems = []
+    warnings = []
+    for key, val in six.iteritems(desired):
+        cver = cur_pkgs.get(key, [])
+        # Package not yet installed, so add to targets
+        if not cver:
+            targets[key] = val
+            continue
+        if sources:
+            if to_reinstall:
+                to_reinstall[key] = val
                 continue
-            elif not __salt__['pkg_resource.check_extra_requirements'](pkgname,
-                                                                       pkgver):
-                targets[pkgname] = pkgver
+            elif 'lowpkg.bin_pkg_info' not in __salt__:
+                continue
+            # Metadata parser is available, cache the file and derive the
+            # package's name and version
+            err = 'Unable to cache {0}: {1}'
+            try:
+                cached_path = __salt__['cp.cache_file'](val)
+            except CommandExecutionError as exc:
+                problems.append(err.format(val, exc))
+                continue
+            if not cached_path:
+                problems.append(err.format(val, 'file not found'))
+                continue
+            elif not os.path.exists(cached_path):
+                problems.append('{0} does not exist on minion'.format(val))
+                continue
+            source_info = __salt__['lowpkg.bin_pkg_info'](cached_path)
+            if source_info is None:
+                warnings.append('Failed to parse metadata for {0}'.format(val))
+                continue
+            else:
+                oper = '=='
+                verstr = source_info['version']
+        else:
+            if not __salt__['pkg_resource.check_extra_requirements'](key, val):
+                targets[key] = val
                 continue
             # No version specified and pkg is installed
-            elif __salt__['pkg_resource.version_clean'](pkgver) is None:
+            elif __salt__['pkg_resource.version_clean'](val) is None:
                 if pkg_verify:
-                    retval = __salt__['pkg.verify'](pkgname,
-                                                    ignore_types=ignore_types)
-                    if retval:
-                        to_reinstall[pkgname] = pkgver
-                        altered_files[pkgname] = retval
-                continue
-            version_spec = True
-            match = re.match('^([<>])?(=)?([^<>=]+)$', pkgver)
-            if not match:
-                msg = 'Invalid version specification {0!r} for package ' \
-                      '{1!r}.'.format(pkgver, pkgname)
-                problems.append(msg)
-            else:
-                gt_lt, eq, verstr = match.groups()
-                comparison = gt_lt or ''
-                comparison += eq or ''
-                # A comparison operator of "=" is redundant, but possible.
-                # Change it to "==" so that the version comparison works
-                if comparison in ['=', '']:
-                    comparison = '=='
-                if 'allow_updates' in kwargs:
-                    if kwargs['allow_updates']:
-                        comparison = '>='
-                if not _fulfills_version_spec(cver, comparison, verstr):
-                    log.debug(
-                        'Current version ({0}) did not match ({1}) desired '
-                        '({2}), add to targets'
-                        .format(cver, comparison, verstr)
+                    verify_result = __salt__['pkg.verify'](
+                        key,
+                        ignore_types=ignore_types,
                     )
-                    targets[pkgname] = pkgver
-                elif pkg_verify and comparison == '==':
-                    retval = __salt__['pkg.verify'](pkgname,
-                                                    ignore_types=ignore_types)
-                    if retval:
-                        to_reinstall[pkgname] = pkgver
-                        altered_files[pkgname] = retval
+                    if verify_result:
+                        to_reinstall[key] = val
+                        altered_files[key] = verify_result
+                continue
+            try:
+                oper, verstr = _get_comparison_spec(val)
+            except CommandExecutionError as exc:
+                problems.append(exc.strerror)
+                continue
 
-        if problems:
-            return {'name': name,
-                    'changes': {},
-                    'result': False,
-                    'comment': ' '.join(problems)}
+        # Compare desired version against installed version.
+        version_spec = True
+        if not sources and 'allow_updates' in kwargs:
+            if kwargs['allow_updates']:
+                oper = '>='
+        if not _fulfills_version_spec(cver, oper, verstr):
+            log.debug(
+                'Current version ({0}) did not match desired version '
+                'specification ({1}), adding to installation targets'
+                .format(cver, val)
+            )
+            targets[key] = val
+        elif pkg_verify and oper == '==':
+            verify_result = __salt__['pkg.verify'](key,
+                                                   ignore_types=ignore_types)
+            if verify_result:
+                to_reinstall[key] = val
+                altered_files[key] = verify_result
+
+    if problems:
+        return {'name': name,
+                'changes': {},
+                'result': False,
+                'comment': ' '.join(problems)}
 
     if not any((targets, to_unpurge, to_reinstall)):
         # All specified packages are installed
-        msg = (
-            'All specified packages are already installed{0}.'
-            .format(' and are at the desired version' if version_spec else '')
+        msg = 'All specified packages are already installed{0}'
+        msg = msg.format(
+            ' and are at the desired version' if version_spec and not sources
+            else ''
         )
         return {'name': name,
                 'changes': {},
                 'result': True,
                 'comment': msg}
 
-    return desired, targets, to_unpurge, to_reinstall, altered_files
+    return desired, targets, to_unpurge, to_reinstall, altered_files, warnings
 
 
 def _verify_install(desired, new_pkgs):
@@ -453,16 +482,8 @@ def _verify_install(desired, new_pkgs):
         elif pkgver.endswith("*") and cver[0].startswith(pkgver[:-1]):
             ok.append(pkgname)
             continue
-
-        match = re.match('^([<>])?(=)?([^<>=]+)$', pkgver)
-        gt_lt, eq, verstr = match.groups()
-        comparison = gt_lt or ''
-        comparison += eq or ''
-        # A comparison operator of "=" is redundant, but possible.
-        # Change it to "==" so that the version comparison works.
-        if comparison in ('=', ''):
-            comparison = '=='
-        if _fulfills_version_spec(cver, comparison, verstr):
+        oper, verstr = _get_comparison_spec(pkgver)
+        if _fulfills_version_spec(cver, oper, verstr):
             ok.append(pkgname)
         else:
             failed.append(pkgname)
@@ -702,11 +723,13 @@ def installed(
             - pkg_verify:
               - ignore_types: [config,doc]
 
-    normalize
-        Normalize the package name by removing the architecture. Default is
-        ``True``.  This is useful for poorly created packages which might
-        include the architecture as an actual part of the name such as kernel
-        modules which match a specific kernel version.
+    normalize : True
+        Normalize the package name by removing the architecture, if the
+        architecture of the package is different from the architecture of the
+        operating system. The ability to disable this behavior is useful for
+        poorly-created packages which include the architecture as an actual
+        part of the name, such as kernel modules which match a specific kernel
+        version.
 
         .. versionadded:: 2014.7.0
 
@@ -875,7 +898,8 @@ def installed(
                                    **kwargs)
 
     try:
-        desired, targets, to_unpurge, to_reinstall, altered_files = result
+        (desired, targets, to_unpurge,
+         to_reinstall, altered_files, warnings) = result
     except ValueError:
         # _find_install_targets() found no targets or encountered an error
 
@@ -931,10 +955,13 @@ def installed(
         return result
 
     if to_unpurge and 'lowpkg.unpurge' not in __salt__:
-        return {'name': name,
-                'changes': {},
-                'result': False,
-                'comment': 'lowpkg.unpurge not implemented'}
+        ret = {'name': name,
+               'changes': {},
+               'result': False,
+               'comment': 'lowpkg.unpurge not implemented'}
+        if warnings:
+            ret['comment'] += '.' + '. '.join(warnings) + '.'
+        return ret
 
     # Remove any targets not returned by _find_install_targets
     if pkgs:
@@ -966,20 +993,25 @@ def installed(
         if to_reinstall:
             # Add a comment for each package in to_reinstall with its
             # pkg.verify output
-            for x in to_reinstall:
+            for reinstall_pkg in to_reinstall:
                 if sources:
-                    pkgstr = x
+                    pkgstr = reinstall_pkg
                 else:
-                    pkgstr = _get_desired_pkg(x, to_reinstall)
+                    pkgstr = _get_desired_pkg(reinstall_pkg, to_reinstall)
                 comment.append(
                     '\nPackage {0} is set to be reinstalled because the '
                     'following files have been altered:'.format(pkgstr)
                 )
-                comment.append('\n' + _nested_output(altered_files[x]))
-        return {'name': name,
-                'changes': {},
-                'result': None,
-                'comment': ' '.join(comment)}
+                comment.append(
+                    '\n' + _nested_output(altered_files[reinstall_pkg])
+                )
+        ret = {'name': name,
+               'changes': {},
+               'result': None,
+               'comment': ' '.join(comment)}
+        if warnings:
+            ret['comment'] += '\n' + '. '.join(warnings) + '.'
+        return ret
 
     changes = {'installed': {}}
     modified_hold = None
@@ -1002,11 +1034,14 @@ def installed(
             if os.path.isfile(rtag) and refresh:
                 os.remove(rtag)
         except CommandExecutionError as exc:
-            return {'name': name,
-                    'changes': {},
-                    'result': False,
-                    'comment': 'An error was encountered while installing '
-                               'package(s): {0}'.format(exc)}
+            ret = {'name': name,
+                   'changes': {},
+                   'result': False,
+                   'comment': 'An error was encountered while installing '
+                              'package(s): {0}'.format(exc)}
+            if warnings:
+                ret['comment'] += '.' + '. '.join(warnings) + '.'
+            return ret
 
         if isinstance(pkg_ret, dict):
             changes['installed'].update(pkg_ret)
@@ -1026,18 +1061,24 @@ def installed(
                         )
                 except (CommandExecutionError, SaltInvocationError) as exc:
                     comment.append(str(exc))
-                    return {'name': name,
-                            'changes': changes,
-                            'result': False,
-                            'comment': ' '.join(comment)}
+                    ret = {'name': name,
+                           'changes': changes,
+                           'result': False,
+                           'comment': ' '.join(comment)}
+                    if warnings:
+                        ret['comment'] += '.' + '. '.join(warnings) + '.'
+                    return ret
                 else:
                     if 'result' in hold_ret and not hold_ret['result']:
-                        return {'name': name,
-                                'changes': {},
-                                'result': False,
-                                'comment': 'An error was encountered while '
-                                           'holding/unholding package(s): {0}'
-                                           .format(hold_ret['comment'])}
+                        ret = {'name': name,
+                               'changes': {},
+                               'result': False,
+                               'comment': 'An error was encountered while '
+                                          'holding/unholding package(s): {0}'
+                                          .format(hold_ret['comment'])}
+                        if warnings:
+                            ret['comment'] += '.' + '. '.join(warnings) + '.'
+                        return ret
                     else:
                         modified_hold = [hold_ret[x] for x in hold_ret
                                          if hold_ret[x]['changes']]
@@ -1116,7 +1157,7 @@ def installed(
                            '{0}'.format(summary))
         else:
             comment.append(
-                '{0} targeted package{1} {2} already installed.'.format(
+                '{0} targeted package{1} {2} already installed'.format(
                     len(not_modified),
                     's' if len(not_modified) > 1 else '',
                     'were' if len(not_modified) > 1 else 'was'
@@ -1159,45 +1200,49 @@ def installed(
     # Rerun pkg.verify for packages in to_reinstall to determine failed
     modified = []
     failed = []
-    for x in to_reinstall:
-        retval = __salt__['pkg.verify'](x, ignore_types=ignore_types)
-        if retval:
-            failed.append(x)
-            altered_files[x] = retval
+    for reinstall_pkg in to_reinstall:
+        verify_result = __salt__['pkg.verify'](reinstall_pkg,
+                                               ignore_types=ignore_types)
+        if verify_result:
+            failed.append(reinstall_pkg)
+            altered_files[reinstall_pkg] = verify_result
         else:
-            modified.append(x)
+            modified.append(reinstall_pkg)
 
     if modified:
         # Add a comment for each package in modified with its pkg.verify output
-        for x in modified:
+        for modified_pkg in modified:
             if sources:
-                pkgstr = x
+                pkgstr = modified_pkg
             else:
-                pkgstr = _get_desired_pkg(x, desired)
+                pkgstr = _get_desired_pkg(modified_pkg, desired)
             comment.append(
                 '\nPackage {0} was reinstalled. The following files were '
                 'remediated:'.format(pkgstr)
             )
-            comment.append(_nested_output(altered_files[x]))
+            comment.append(_nested_output(altered_files[modified_pkg]))
 
     if failed:
         # Add a comment for each package in failed with its pkg.verify output
-        for x in failed:
+        for failed_pkg in failed:
             if sources:
-                pkgstr = x
+                pkgstr = failed_pkg
             else:
-                pkgstr = _get_desired_pkg(x, desired)
+                pkgstr = _get_desired_pkg(failed_pkg, desired)
             comment.append(
                 '\nReinstall was not successful for package {0}. The '
                 'following files could not be remediated:'.format(pkgstr)
             )
-            comment.append(_nested_output(altered_files[x]))
+            comment.append(_nested_output(altered_files[failed_pkg]))
         result = False
 
-    return {'name': name,
-            'changes': changes,
-            'result': result,
-            'comment': ' '.join(comment)}
+    ret = {'name': name,
+           'changes': changes,
+           'result': result,
+           'comment': ' '.join(comment)}
+    if warnings:
+        ret['comment'] += '\n' + '. '.join(warnings) + '.'
+    return ret
 
 
 def latest(
@@ -1326,7 +1371,7 @@ def latest(
     for pkg in desired_pkgs:
         if not avail[pkg]:
             if not cur[pkg]:
-                msg = 'No information found for {0!r}.'.format(pkg)
+                msg = 'No information found for \'{0}\'.'.format(pkg)
                 log.error(msg)
                 problems.append(msg)
         elif not cur[pkg] \
@@ -1370,7 +1415,7 @@ def latest(
                         'up-to-date: {0}'
                     ).format(up_to_date_details)
                 else:
-                    comment += ' {0} packages are already up-to-date.'.format(
+                    comment += ' {0} packages are already up-to-date'.format(
                         up_to_date_nb
                     )
 
@@ -1419,7 +1464,7 @@ def latest(
                     msg = 'The following packages were already up-to-date: ' \
                         '{0}'.format(', '.join(sorted(up_to_date)))
                 else:
-                    msg = '{0} packages were already up-to-date. '.format(
+                    msg = '{0} packages were already up-to-date '.format(
                         len(up_to_date))
                 comments.append(msg)
 
@@ -1445,7 +1490,7 @@ def latest(
                         '{0}'.format(', '.join(sorted(up_to_date)))
                 else:
                     comment += '{0} packages were already ' \
-                        'up-to-date.'.format(len(up_to_date))
+                        'up-to-date'.format(len(up_to_date))
 
             return {'name': name,
                     'changes': changes,
@@ -1460,7 +1505,7 @@ def latest(
                 '({0}).'.format(', '.join(sorted(desired_pkgs)))
         else:
             comment = 'Package {0} is already ' \
-                'up-to-date.'.format(desired_pkgs[0])
+                'up-to-date'.format(desired_pkgs[0])
 
         return {'name': name,
                 'changes': {},
@@ -1468,7 +1513,13 @@ def latest(
                 'comment': comment}
 
 
-def _uninstall(action='remove', name=None, version=None, pkgs=None, **kwargs):
+def _uninstall(
+    action='remove',
+    name=None,
+    version=None,
+    pkgs=None,
+    normalize=True,
+    **kwargs):
     '''
     Common function for package removal
     '''
@@ -1476,18 +1527,21 @@ def _uninstall(action='remove', name=None, version=None, pkgs=None, **kwargs):
         return {'name': name,
                 'changes': {},
                 'result': False,
-                'comment': 'Invalid action {0!r}. '
+                'comment': 'Invalid action \'{0}\'. '
                            'This is probably a bug.'.format(action)}
 
     try:
-        pkg_params = __salt__['pkg_resource.parse_targets'](name, pkgs)[0]
+        pkg_params = __salt__['pkg_resource.parse_targets'](
+            name,
+            pkgs,
+            normalize=normalize)[0]
     except MinionError as exc:
         return {'name': name,
                 'changes': {},
                 'result': False,
                 'comment': 'An error was encountered while parsing targets: '
                            '{0}'.format(exc)}
-    targets = _find_remove_targets(name, version, pkgs, **kwargs)
+    targets = _find_remove_targets(name, version, pkgs, normalize, **kwargs)
     if isinstance(targets, dict) and 'result' in targets:
         return targets
     elif not isinstance(targets, list):
@@ -1551,7 +1605,7 @@ def _uninstall(action='remove', name=None, version=None, pkgs=None, **kwargs):
             'comment': ' '.join(comments)}
 
 
-def removed(name, version=None, pkgs=None, **kwargs):
+def removed(name, version=None, pkgs=None, normalize=True, **kwargs):
     '''
     Verify that a package is not installed, calling ``pkg.remove`` if necessary
     to remove the package.
@@ -1563,6 +1617,15 @@ def removed(name, version=None, pkgs=None, **kwargs):
         The version of the package that should be removed. Don't do anything if
         the package is installed with an unmatching version.
 
+    normalize : True
+        Normalize the package name by removing the architecture, if the
+        architecture of the package is different from the architecture of the
+        operating system. The ability to disable this behavior is useful for
+        poorly-created packages which include the architecture as an actual
+        part of the name, such as kernel modules which match a specific kernel
+        version.
+
+        .. versionadded:: Beryllium
 
     Multiple Package Options:
 
@@ -1575,7 +1638,7 @@ def removed(name, version=None, pkgs=None, **kwargs):
     '''
     try:
         return _uninstall(action='remove', name=name, version=version,
-                          pkgs=pkgs, **kwargs)
+                          pkgs=pkgs, normalize=normalize, **kwargs)
     except CommandExecutionError as exc:
         return {'name': name,
                 'changes': {},
@@ -1583,7 +1646,7 @@ def removed(name, version=None, pkgs=None, **kwargs):
                 'comment': str(exc)}
 
 
-def purged(name, version=None, pkgs=None, **kwargs):
+def purged(name, version=None, pkgs=None, normalize=True, **kwargs):
     '''
     Verify that a package is not installed, calling ``pkg.purge`` if necessary
     to purge the package. All configuration files are also removed.
@@ -1595,6 +1658,15 @@ def purged(name, version=None, pkgs=None, **kwargs):
         The version of the package that should be removed. Don't do anything if
         the package is installed with an unmatching version.
 
+    normalize : True
+        Normalize the package name by removing the architecture, if the
+        architecture of the package is different from the architecture of the
+        operating system. The ability to disable this behavior is useful for
+        poorly-created packages which include the architecture as an actual
+        part of the name, such as kernel modules which match a specific kernel
+        version.
+
+        .. versionadded:: Beryllium
 
     Multiple Package Options:
 
@@ -1607,7 +1679,7 @@ def purged(name, version=None, pkgs=None, **kwargs):
     '''
     try:
         return _uninstall(action='purge', name=name, version=version,
-                          pkgs=pkgs, **kwargs)
+                          pkgs=pkgs, normalize=normalize **kwargs)
     except CommandExecutionError as exc:
         return {'name': name,
                 'changes': {},
@@ -1649,11 +1721,11 @@ def uptodate(name, refresh=False, **kwargs):
             ret['comment'] = str(exc)
             return ret
     else:
-        ret['comment'] = 'refresh must be a boolean.'
+        ret['comment'] = 'refresh must be a boolean'
         return ret
 
     if not packages:
-        ret['comment'] = 'System is already up-to-date.'
+        ret['comment'] = 'System is already up-to-date'
         ret['result'] = True
         return ret
     elif __opts__['test']:

salt/utils/pkg/__init__.py

@@ -0,0 +1,4 @@
+# -*- coding: utf-8 -*-
+'''
+Helper modules used by lowpkg modules
+'''

salt/utils/pkg/rpm.py

@@ -0,0 +1,87 @@
+# -*- coding: utf-8 -*-
+'''
+Common
+'''
+
+# Import python libs
+from __future__ import absolute_import
+import logging
+
+# Import salt libs
+from salt._compat import subprocess
+
+log = logging.getLogger(__name__)
+
+# These arches compiled from the rpmUtils.arch python module source
+ARCHES_64 = ('x86_64', 'athlon', 'amd64', 'ia32e', 'ia64', 'geode')
+ARCHES_32 = ('i386', 'i486', 'i586', 'i686')
+ARCHES_PPC = ('ppc', 'ppc64', 'ppc64iseries', 'ppc64pseries')
+ARCHES_S390 = ('s390', 's390x')
+ARCHES_SPARC = (
+    'sparc', 'sparcv8', 'sparcv9', 'sparcv9v', 'sparc64', 'sparc64v'
+)
+ARCHES_ALPHA = (
+    'alpha', 'alphaev4', 'alphaev45', 'alphaev5', 'alphaev56',
+    'alphapca56', 'alphaev6', 'alphaev67', 'alphaev68', 'alphaev7'
+)
+ARCHES_ARM = ('armv5tel', 'armv5tejl', 'armv6l', 'armv7l')
+ARCHES_SH = ('sh3', 'sh4', 'sh4a')
+
+ARCHES = ARCHES_64 + ARCHES_32 + ARCHES_PPC + ARCHES_S390 + \
+    ARCHES_ALPHA + ARCHES_ARM + ARCHES_SH
+
+QUERYFORMAT = '%{NAME}_|-%{VERSION}_|-%{RELEASE}_|-%{ARCH}_|-%{REPOID}'
+
+
+def _osarch():
+    '''
+    Get the os architecture using rpm --eval
+    '''
+    ret = subprocess.Popen(
+        'rpm --eval "%{_host_cpu}"',
+        shell=True,
+        close_fds=True,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE).communicate()[0]
+    return ret or 'unknown'
+
+
+def check_32(arch, osarch=None):
+    '''
+    Returns True if both the OS arch and the passed arch are 32-bit
+    '''
+    if osarch is None:
+        osarch = _osarch()
+    return all(x in ARCHES_32 for x in (osarch, arch))
+
+
+def parse_pkginfo(line, osarch=None):
+    '''
+    A small helper to parse an rpm/repoquery command's output. Returns a
+    namedtuple
+    '''
+    # Importing `collections` here since this function is re-namespaced into
+    # another module
+    import collections
+    pkginfo = collections.namedtuple(
+        'PkgInfo',
+        ('name', 'version', 'arch', 'repoid')
+    )
+
+    try:
+        name, pkg_version, release, arch, repoid = line.split('_|-')
+    # Handle unpack errors (should never happen with the queryformat we are
+    # using, but can't hurt to be careful).
+    except ValueError:
+        return None
+
+    if osarch is None:
+        osarch = _osarch()
+
+    if not check_32(arch):
+        if arch not in (osarch, 'noarch'):
+            name += '.{0}'.format(arch)
+    if release:
+        pkg_version += '-{0}'.format(release)
+
+    return pkginfo(name, pkg_version, arch, repoid)