File 0044-Unblock-Zypper.-Modify-environment.patch of Package salt.2642

Overview Repositories Revisions Requests Users Attributes Meta

File 0044-Unblock-Zypper.-Modify-environment.patch of Package salt.2642

From f7158843ee58cade84bf3d789bf2f3abfa34d384 Mon Sep 17 00:00:00 2001
From: Bo Maryniuk <bo@suse.de>
Date: Fri, 22 Apr 2016 14:59:14 +0200
Subject: [PATCH 44/44] Unblock Zypper. Modify environment.

* Bugfix: version_cmp crashes in CLI if there are versions, that looks like integer or float.

* Standarize zypper call to "run_all"

* Remove verbose wrapping

* Remove an empty line

* Remove an unused variable

* Remove one-char variables

* Implement block-proof Zypper call implementation

* Remove blocking-prone Zypper call implementation

* Use new Zypper call implementation

* Fire an event to the Master about blocked Zypper.

* Add Zypper lock constant

* Check if zypper lock exists and add more debug logging

* Replace string values with the constants

* Fire an event about released Zypper with its result

* Bugfix: accept refresh override param

* Update docstrings according to the bugfix

* Make Zypper caller module-level reusable

* Bugfix: inverted logic on raising (or not) exceptions

* Add Zypper Call mock

* Remove an obsolete test case

* Fix tests according to the new calling model

* Bugfix: always trigger __getattr__ to reset and increment the configuration before the call.

* Add Zypper caller test suite

* Parse DOM out of the box, when XML mode is called

* Add exception handling test

* Test DOM parsing

* Rename tags

* Fix PID file path for SLE11

* Move log message down to the point where it actually sleeps. Rephrase the message.

* Remove unused variable in a constructor. Adjust the docstring accordingly.

* Prevent the use of "refreshable" together with "nolock" option.
---
 salt/modules/zypper.py            | 404 +++++++++++++++++++++++++-------------
 tests/unit/modules/zypper_test.py | 133 +++++++------
 2 files changed, 346 insertions(+), 191 deletions(-)

diff --git a/salt/modules/zypper.py b/salt/modules/zypper.py
index 1cb8a43..53b5d9f 100644
--- a/salt/modules/zypper.py
+++ b/salt/modules/zypper.py
@@ -11,10 +11,13 @@ import copy
 import logging
 import re
 import os
+import time
+import datetime
 
 # Import 3rd-party libs
 # pylint: disable=import-error,redefined-builtin,no-name-in-module
 import salt.ext.six as six
+import salt.utils.event
 from salt.ext.six.moves import configparser
 from salt.ext.six.moves.urllib.parse import urlparse as _urlparse
 # pylint: enable=import-error,redefined-builtin,no-name-in-module
@@ -51,65 +54,226 @@ def __virtual__():
     return __virtualname__
 
 
-def _zypper(*opts):
-    '''
-    Return zypper command with default options as a list.
-
-    opts
-        additional options for zypper command
-
-    '''
-    cmd = ['zypper', '--non-interactive']
-    cmd.extend(opts)
-
-    return cmd
-
-
-def _is_zypper_error(retcode):
-    '''
-    Return True in case the exist code indicate a zypper errror.
-    Otherwise False
-    '''
-    # see man zypper for existing exit codes
-    return not int(retcode) in [0, 100, 101, 102, 103]
+class _Zypper(object):
+    '''
+    Zypper parallel caller.
+    Validates the result and either raises an exception or reports an error.
+    Allows serial zypper calls (first came, first won).
+    '''
+
+    SUCCESS_EXIT_CODES = [0, 100, 101, 102, 103]
+    LOCK_EXIT_CODE = 7
+    XML_DIRECTIVES = ['-x', '--xmlout']
+    ZYPPER_LOCK = '/var/run/zypp.pid'
+    TAG_RELEASED = 'zypper/released'
+    TAG_BLOCKED = 'zypper/blocked'
+
+    def __init__(self):
+        '''
+        Constructor
+        '''
+        self.__called = False
+        self._reset()
+
+    def _reset(self):
+        '''
+        Resets values of the call setup.
+
+        :return:
+        '''
+        self.__cmd = ['zypper', '--non-interactive']
+        self.__exit_code = 0
+        self.__call_result = dict()
+        self.__error_msg = ''
+        self.__env = {'SALT_RUNNING': "1"}  # Subject to change
+
+        # Call config
+        self.__xml = False
+        self.__no_lock = False
+        self.__no_raise = False
+        self.__refresh = False
+
+    def __getattr__(self, item):
+        '''
+        Call configurator.
+
+        :param item:
+        :return:
+        '''
+        # Reset after the call
+        if self.__called:
+            self._reset()
+            self.__called = False
+
+        if item == 'xml':
+            self.__xml = True
+        elif item == 'nolock':
+            self.__no_lock = True
+        elif item == 'noraise':
+            self.__no_raise = True
+        elif item == 'refreshable':
+            self.__refresh = True
+        elif item == 'call':
+            return self.__call
+        else:
+            return self.__dict__[item]
+
+        # Prevent the use of "refreshable" together with "nolock".
+        if self.__no_lock:
+            self.__no_lock = not self.__refresh
+
+        return self
+
+    @property
+    def exit_code(self):
+        return self.__exit_code
+
+    @exit_code.setter
+    def exit_code(self, exit_code):
+        self.__exit_code = int(exit_code or '0')
+
+    @property
+    def error_msg(self):
+        return self.__error_msg
+
+    @error_msg.setter
+    def error_msg(self, msg):
+        if self._is_error():
+            self.__error_msg = msg and os.linesep.join(msg) or "Check Zypper's logs."
+
+    def stdout(self):
+        return self.__call_result.get('stdout', '')
+
+    def stderr(self):
+        return self.__call_result.get('stderr', '')
+
+    def _is_error(self):
+        '''
+        Is this is an error code?
+
+        :return:
+        '''
+        return self.exit_code not in self.SUCCESS_EXIT_CODES
+
+    def _is_lock(self):
+        '''
+        Is this is a lock error code?
+
+        :return:
+        '''
+        return self.exit_code == self.LOCK_EXIT_CODE
+
+    def _is_xml_mode(self):
+        '''
+        Is Zypper's output is in XML format?
+
+        :return:
+        '''
+        return [itm for itm in self.XML_DIRECTIVES if itm in self.__cmd] and True or False
+
+    def _check_result(self):
+        '''
+        Check and set the result of a zypper command. In case of an error,
+        either raise a CommandExecutionError or extract the error.
+
+        result
+            The result of a zypper command called with cmd.run_all
+        '''
+        if not self.__call_result:
+            raise CommandExecutionError('No output result from Zypper?')
+
+        self.exit_code = self.__call_result['retcode']
+        if self._is_lock():
+            return False
+
+        if self._is_error():
+            _error_msg = list()
+            if not self._is_xml_mode():
+                msg = self.__call_result['stderr'] and self.__call_result['stderr'].strip() or ""
+                if msg:
+                    _error_msg.append(msg)
+            else:
+                try:
+                    doc = dom.parseString(self.__call_result['stdout'])
+                except ExpatError as err:
+                    log.error(err)
+                    doc = None
+                if doc:
+                    msg_nodes = doc.getElementsByTagName('message')
+                    for node in msg_nodes:
+                        if node.getAttribute('type') == 'error':
+                            _error_msg.append(node.childNodes[0].nodeValue)
+                elif self.__call_result['stderr'].strip():
+                    _error_msg.append(self.__call_result['stderr'].strip())
+            self.error_msg = _error_msg
+        return True
+
+    def __call(self, *args, **kwargs):
+        '''
+        Call Zypper.
+
+        :param state:
+        :return:
+        '''
+        self.__called = True
+        if self.__xml:
+            self.__cmd.append('--xmlout')
+        if not self.__refresh:
+            self.__cmd.append('--no-refresh')
+
+        self.__cmd.extend(args)
+        kwargs['output_loglevel'] = 'trace'
+        kwargs['python_shell'] = False
+        kwargs['env'] = self.__env.copy()
+        if self.__no_lock:
+            kwargs['env']['ZYPP_READONLY_HACK'] = "1"  # Disables locking for read-only operations. Do not try that at home!
+
+        # Zypper call will stuck here waiting, if another zypper hangs until forever.
+        # However, Zypper lock needs to be always respected.
+        was_blocked = False
+        while True:
+            log.debug("Calling Zypper: " + ' '.join(self.__cmd))
+            self.__call_result = __salt__['cmd.run_all'](self.__cmd, **kwargs)
+            if self._check_result():
+                break
+
+            if os.path.exists(self.ZYPPER_LOCK):
+                try:
+                    data = __salt__['ps.proc_info'](int(open(self.ZYPPER_LOCK).readline()),
+                                                    attrs=['pid', 'name', 'cmdline', 'create_time'])
+                    data['cmdline'] = ' '.join(data['cmdline'])
+                    data['info'] = 'Blocking process created at {0}.'.format(
+                        datetime.datetime.utcfromtimestamp(data['create_time']).isoformat())
+                    data['success'] = True
+                except Exception as err:
+                    data = {'info': 'Unable to retrieve information about blocking process: {0}'.format(err.message),
+                            'success': False}
+            else:
+                data = {'info': 'Zypper is locked, but no Zypper lock has been found.', 'success': False}
 
+            if not data['success']:
+                log.debug("Unable to collect data about blocking process.")
+            else:
+                log.debug("Collected data about blocking process.")
 
-def _zypper_check_result(result, xml=False):
-    '''
-    Check the result of a zypper command. In case of an error, it raise
-    a CommandExecutionError. Otherwise it returns stdout string of the
-    command.
+            __salt__['event.fire_master'](data, self.TAG_BLOCKED)
+            log.debug("Fired a Zypper blocked event to the master with the data: {0}".format(str(data)))
+            log.debug("Waiting 5 seconds for Zypper gets released...")
+            time.sleep(5)
+            if not was_blocked:
+                was_blocked = True
 
-    result
-        The result of a zypper command called with cmd.run_all
+        if was_blocked:
+            __salt__['event.fire_master']({'success': not len(self.error_msg),
+                                           'info': self.error_msg or 'Zypper has been released'},
+                                          self.TAG_RELEASED)
+        if self.error_msg and not self.__no_raise:
+            raise CommandExecutionError('Zypper command failure: {0}'.format(self.error_msg))
 
-    xml
-        Set to True if zypper command was called with --xmlout.
-        In this case it try to read an error message out of the XML
-        stream. Default is False.
-    '''
-    if _is_zypper_error(result['retcode']):
-        msg = list()
-        if not xml:
-            msg.append(result['stderr'] and result['stderr'] or "")
-        else:
-            try:
-                doc = dom.parseString(result['stdout'])
-            except ExpatError as err:
-                log.error(err)
-                doc = None
-            if doc:
-                msg_nodes = doc.getElementsByTagName('message')
-                for node in msg_nodes:
-                    if node.getAttribute('type') == 'error':
-                        msg.append(node.childNodes[0].nodeValue)
-            elif result['stderr'].strip():
-                msg.append(result['stderr'].strip())
+        return self._is_xml_mode() and dom.parseString(self.__call_result['stdout']) or self.__call_result['stdout']
 
-        raise CommandExecutionError("zypper command failed: {0}".format(
-            msg and os.linesep.join(msg) or "Check zypper logs"))
 
-    return result['stdout']
+__zypper__ = _Zypper()
 
 
 def list_upgrades(refresh=True):
@@ -129,10 +293,9 @@ def list_upgrades(refresh=True):
     '''
     if refresh:
         refresh_db()
+
     ret = dict()
-    run_data = __salt__['cmd.run_all'](_zypper('-x', 'list-updates'), output_loglevel='trace')
-    doc = dom.parseString(_zypper_check_result(run_data, xml=True))
-    for update_node in doc.getElementsByTagName('update'):
+    for update_node in __zypper__.nolock.xml.call('list-updates').getElementsByTagName('update'):
         if update_node.getAttribute('kind') == 'package':
             ret[update_node.getAttribute('name')] = update_node.getAttribute('edition')
 
@@ -191,7 +354,6 @@ def info_installed(*names, **kwargs):
                 t_nfo['source'] = value
             else:
                 t_nfo[key] = value
-
         ret[pkg_name] = t_nfo
 
     return ret
@@ -230,8 +392,8 @@ def info_available(*names, **kwargs):
 
     # Run in batches
     while batch:
-        cmd = _zypper('info', '-t', 'package', *batch[:batch_size])
-        pkg_info.extend(re.split(r"Information for package*", __salt__['cmd.run_stdout'](cmd, output_loglevel='trace')))
+        pkg_info.extend(re.split(r"Information for package*",
+                                 __zypper__.nolock.call('info', '-t', 'package', *batch[:batch_size])))
         batch = batch[batch_size:]
 
     for pkg_data in pkg_info:
@@ -280,6 +442,11 @@ def latest_version(*names, **kwargs):
     If the latest version of a given package is already installed, an empty
     dict will be returned for that package.
 
+    refresh
+        force a refresh if set to True (default).
+        If set to False it depends on zypper if a refresh is
+        executed or not.
+
     CLI example:
 
     .. code-block:: bash
@@ -293,7 +460,7 @@ def latest_version(*names, **kwargs):
         return ret
 
     names = sorted(list(set(names)))
-    package_info = info_available(*names)
+    package_info = info_available(*names, **kwargs)
     for name in names:
         pkg_info = package_info.get(name, {})
         status = pkg_info.get('status', '').lower()
@@ -311,17 +478,23 @@ def latest_version(*names, **kwargs):
 available_version = salt.utils.alias_function(latest_version, 'available_version')
 
 
-def upgrade_available(name):
+def upgrade_available(name, **kwargs):
     '''
     Check whether or not an upgrade is available for a given package
 
+    refresh
+        force a refresh if set to True (default).
+        If set to False it depends on zypper if a refresh is
+        executed or not.
+
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' pkg.upgrade_available <package name>
     '''
-    return not not latest_version(name)
+    # The "not not" tactic is intended here as it forces the return to be False.
+    return not not latest_version(name, **kwargs)  # pylint: disable=C0113
 
 
 def version(*names, **kwargs):
@@ -354,7 +527,7 @@ def version_cmp(ver1, ver2):
 
         salt '*' pkg.version_cmp '0.2-001' '0.2.0.1-002'
     '''
-    return __salt__['lowpkg.version_cmp'](ver1, ver2)
+    return __salt__['lowpkg.version_cmp'](str(ver1), str(ver2))
 
 
 def list_pkgs(versions_as_list=False, **kwargs):
@@ -397,12 +570,7 @@ def list_pkgs(versions_as_list=False, **kwargs):
 
     cmd = ['rpm', '-qa', '--queryformat', '%{NAME}_|-%{VERSION}_|-%{RELEASE}_|-%|EPOCH?{%{EPOCH}}:{}|\\n']
     ret = {}
-    out = __salt__['cmd.run'](
-        cmd,
-        output_loglevel='trace',
-        python_shell=False
-    )
-    for line in out.splitlines():
+    for line in __salt__['cmd.run'](cmd, output_loglevel='trace', python_shell=False).splitlines():
         name, pkgver, rel, epoch = line.split('_|-')
         if epoch:
             pkgver = '{0}:{1}'.format(epoch, pkgver)
@@ -414,6 +582,7 @@ def list_pkgs(versions_as_list=False, **kwargs):
     __context__['pkg.list_pkgs'] = copy.deepcopy(ret)
     if not versions_as_list:
         __salt__['pkg_resource.stringify'](ret)
+
     return ret
 
 
@@ -433,15 +602,13 @@ def _get_repo_info(alias, repos_cfg=None):
     Get one repo meta-data.
     '''
     try:
-        meta = dict((repos_cfg or _get_configured_repos()).items(alias))
-        meta['alias'] = alias
-        for key, val in six.iteritems(meta):
-            if val in ['0', '1']:
-                meta[key] = int(meta[key]) == 1
-            elif val == 'NONE':
-                meta[key] = None
-        return meta
-    except (ValueError, configparser.NoSectionError) as error:
+        ret = dict((repos_cfg or _get_configured_repos()).items(alias))
+        ret['alias'] = alias
+        for key, val in six.iteritems(ret):
+            if val == 'NONE':
+                ret[key] = None
+        return ret
+    except (ValueError, configparser.NoSectionError):
         return {}
 
 
@@ -489,9 +656,7 @@ def del_repo(repo):
     repos_cfg = _get_configured_repos()
     for alias in repos_cfg.sections():
         if alias == repo:
-            cmd = _zypper('-x', 'rr', '--loose-auth', '--loose-query', alias)
-            ret = __salt__['cmd.run_all'](cmd, output_loglevel='trace')
-            doc = dom.parseString(_zypper_check_result(ret, xml=True))
+            doc = __zypper__.xml.call('rr', '--loose-auth', '--loose-query', alias)
             msg = doc.getElementsByTagName('message')
             if doc.getElementsByTagName('progress') and msg:
                 return {
@@ -575,8 +740,7 @@ def mod_repo(repo, **kwargs):
                     'Repository \'{0}\' already exists as \'{1}\'.'.format(repo, alias))
 
         # Add new repo
-        _zypper_check_result(__salt__['cmd.run_all'](_zypper('-x', 'ar', url, repo),
-                                                     output_loglevel='trace'), xml=True)
+        __zypper__.xml.call('ar', url, repo)
 
         # Verify the repository has been added
         repos_cfg = _get_configured_repos()
@@ -612,9 +776,7 @@ def mod_repo(repo, **kwargs):
 
     if cmd_opt:
         cmd_opt.append(repo)
-        ret = __salt__['cmd.run_all'](_zypper('-x', 'mr', *cmd_opt),
-                                      output_loglevel='trace')
-        _zypper_check_result(ret, xml=True)
+        __zypper__.refreshable.xml.call('mr', *cmd_opt)
 
     # If repo nor added neither modified, error should be thrown
     if not added and not cmd_opt:
@@ -636,9 +798,8 @@ def refresh_db():
 
         salt '*' pkg.refresh_db
     '''
-    cmd = _zypper('refresh', '--force')
     ret = {}
-    out = _zypper_check_result(__salt__['cmd.run_all'](cmd, output_loglevel='trace'))
+    out = __zypper__.refreshable.call('refresh', '--force')
 
     for line in out.splitlines():
         if not line:
@@ -778,8 +939,7 @@ def install(name=None,
         log.info('Targeting repo {0!r}'.format(fromrepo))
     else:
         fromrepoopt = ''
-    cmd_install = _zypper()
-    cmd_install += ['install', '--name', '--auto-agree-with-licenses']
+    cmd_install = ['install', '--name', '--auto-agree-with-licenses']
     if downloadonly:
         cmd_install.append('--download-only')
     if fromrepo:
@@ -789,9 +949,7 @@ def install(name=None,
     while targets:
         cmd = cmd_install + targets[:500]
         targets = targets[500:]
-        call = __salt__['cmd.run_all'](cmd, output_loglevel='trace', python_shell=False)
-        out = _zypper_check_result(call)
-        for line in out.splitlines():
+        for line in __zypper__.call(*cmd).splitlines():
             match = re.match(r"^The selected package '([^']+)'.+has lower version", line)
             if match:
                 downgrades.append(match.group(1))
@@ -799,8 +957,7 @@ def install(name=None,
     while downgrades:
         cmd = cmd_install + ['--force'] + downgrades[:500]
         downgrades = downgrades[500:]
-
-        _zypper_check_result(__salt__['cmd.run_all'](cmd, output_loglevel='trace', python_shell=False))
+        __zypper__.call(*cmd)
 
     __context__.pop('pkg.list_pkgs', None)
     new = list_pkgs()
@@ -836,18 +993,15 @@ def upgrade(refresh=True):
     if refresh:
         refresh_db()
     old = list_pkgs()
-    cmd = _zypper('update', '--auto-agree-with-licenses')
-    call = __salt__['cmd.run_all'](cmd, output_loglevel='trace')
-    if _is_zypper_error(call['retcode']):
+    __zypper__.noraise.call('update', '--auto-agree-with-licenses')
+    if __zypper__.exit_code not in __zypper__.SUCCESS_EXIT_CODES:
         ret['result'] = False
-        if 'stderr' in call:
-            ret['comment'] += call['stderr']
-        if 'stdout' in call:
-            ret['comment'] += call['stdout']
+        ret['comment'] = (__zypper__.stdout() + os.linesep + __zypper__.stderr()).strip()
     else:
         __context__.pop('pkg.list_pkgs', None)
         new = list_pkgs()
         ret['changes'] = salt.utils.compare_dicts(old, new)
+
     return ret
 
 
@@ -867,8 +1021,7 @@ def _uninstall(name=None, pkgs=None):
         return {}
 
     while targets:
-        _zypper_check_result(__salt__['cmd.run_all'](_zypper('remove', *targets[:500]),
-                                                     output_loglevel='trace'))
+        __zypper__.call('remove', *targets[:500])
         targets = targets[500:]
     __context__.pop('pkg.list_pkgs', None)
 
@@ -981,9 +1134,7 @@ def clean_locks():
     if not os.path.exists("/etc/zypp/locks"):
         return out
 
-    ret = __salt__['cmd.run_all'](_zypper('-x', 'cl'), output_loglevel='trace')
-    doc = dom.parseString(_zypper_check_result(ret, xml=True))
-    for node in doc.getElementsByTagName("message"):
+    for node in __zypper__.xml.call('cl').getElementsByTagName("message"):
         text = node.childNodes[0].nodeValue.lower()
         if text.startswith(LCK):
             out[LCK] = text.split(" ")[1]
@@ -1020,8 +1171,7 @@ def remove_lock(packages, **kwargs):  # pylint: disable=unused-argument
             missing.append(pkg)
 
     if removed:
-        _zypper_check_result(__salt__['cmd.run_all'](_zypper('rl', *removed),
-                                                     output_loglevel='trace'))
+        __zypper__.call('rl', *removed)
 
     return {'removed': len(removed), 'not_found': missing}
 
@@ -1050,8 +1200,7 @@ def add_lock(packages, **kwargs):  # pylint: disable=unused-argument
             added.append(pkg)
 
     if added:
-        _zypper_check_result(__salt__['cmd.run_all'](_zypper('al', *added),
-                                                     output_loglevel='trace'))
+        __zypper__.call('al', *added)
 
     return {'added': len(added), 'packages': added}
 
@@ -1184,10 +1333,7 @@ def _get_patterns(installed_only=None):
     '''
     patterns = {}
 
-    ret = __salt__['cmd.run_all'](_zypper('--xmlout', 'se', '-t', 'pattern'),
-                                  output_loglevel='trace')
-    doc = dom.parseString(_zypper_check_result(ret, xml=True))
-    for element in doc.getElementsByTagName('solvable'):
+    for element in __zypper__.nolock.xml.call('se', '-t', 'pattern').getElementsByTagName('solvable'):
         installed = element.getAttribute('status') == 'installed'
         if (installed_only and installed) or not installed_only:
             patterns[element.getAttribute('name')] = {
@@ -1250,20 +1396,16 @@ def search(criteria, refresh=False):
     if refresh:
         refresh_db()
 
-    ret = __salt__['cmd.run_all'](_zypper('--xmlout', 'se', criteria),
-                                  output_loglevel='trace')
-    doc = dom.parseString(_zypper_check_result(ret, xml=True))
-    solvables = doc.getElementsByTagName('solvable')
+    solvables = __zypper__.nolock.xml.call('se', criteria).getElementsByTagName('solvable')
     if not solvables:
         raise CommandExecutionError('No packages found by criteria "{0}".'.format(criteria))
 
     out = {}
-    for solvable in [s for s in solvables
-                     if s.getAttribute('status') == 'not-installed' and
-                        s.getAttribute('kind') == 'package']:
-        out[solvable.getAttribute('name')] = {
-            'summary': solvable.getAttribute('summary')
-        }
+    for solvable in [slv for slv in solvables
+                     if slv.getAttribute('status') == 'not-installed'
+                         and slv.getAttribute('kind') == 'package']:
+        out[solvable.getAttribute('name')] = {'summary': solvable.getAttribute('summary')}
+
     return out
 
 
@@ -1308,16 +1450,14 @@ def list_products(all=False, refresh=False):
 
     ret = list()
     OEM_PATH = "/var/lib/suseRegister/OEM"
-    cmd = _zypper()
+    cmd = list()
     if not all:
         cmd.append('--disable-repos')
-    cmd.extend(['-x', 'products'])
+    cmd.append('products')
     if not all:
         cmd.append('-i')
 
-    call = __salt__['cmd.run_all'](cmd, output_loglevel='trace')
-    doc = dom.parseString(_zypper_check_result(call, xml=True))
-    product_list = doc.getElementsByTagName('product-list')
+    product_list = __zypper__.nolock.xml.call(*cmd).getElementsByTagName('product-list')
     if not product_list:
         return ret  # No products found
 
@@ -1370,10 +1510,8 @@ def download(*packages, **kwargs):
     if refresh:
         refresh_db()
 
-    ret = __salt__['cmd.run_all'](_zypper('-x', 'download', *packages), output_loglevel='trace')
-    doc = dom.parseString(_zypper_check_result(ret, xml=True))
     pkg_ret = {}
-    for dld_result in doc.getElementsByTagName("download-result"):
+    for dld_result in __zypper__.xml.call('download', *packages).getElementsByTagName("download-result"):
         repo = dld_result.getElementsByTagName("repository")[0]
         pkg_info = {
             'repository-name': repo.getAttribute("name"),
diff --git a/tests/unit/modules/zypper_test.py b/tests/unit/modules/zypper_test.py
index 97e42ef..16e8542 100644
--- a/tests/unit/modules/zypper_test.py
+++ b/tests/unit/modules/zypper_test.py
@@ -23,6 +23,17 @@ from salttesting.helpers import ensure_in_syspath
 ensure_in_syspath('../../')
 
 
+class ZyppCallMock(object):
+    def __init__(self, return_value=None):
+        self.__return_value = return_value
+
+    def __getattr__(self, item):
+        return self
+
+    def __call__(self, *args, **kwargs):
+        return MagicMock(return_value=self.__return_value)()
+
+
 def get_test_data(filename):
     '''
     Return static test data
@@ -64,56 +75,63 @@ class ZypperTestCase(TestCase):
                 self.assertIn(pkg, upgrades)
                 self.assertEqual(upgrades[pkg], version)
 
-    def test_zypper_check_result(self):
+    def test_zypper_caller(self):
         '''
-        Test zypper check result function
+        Test Zypper caller.
+        :return:
         '''
-        cmd_out = {
-                'retcode': 1,
-                'stdout': '',
-                'stderr': 'This is an error'
-        }
-        with self.assertRaisesRegexp(CommandExecutionError, "^zypper command failed: This is an error$"):
-            zypper._zypper_check_result(cmd_out)
-
-        cmd_out = {
-                'retcode': 0,
-                'stdout': 'result',
-                'stderr': ''
-        }
-        out = zypper._zypper_check_result(cmd_out)
-        self.assertEqual(out, "result")
-
-        cmd_out = {
-                'retcode': 1,
-                'stdout': '',
-                'stderr': 'This is an error'
-        }
-        with self.assertRaisesRegexp(CommandExecutionError, "^zypper command failed: This is an error$"):
-            zypper._zypper_check_result(cmd_out, xml=True)
-
-        cmd_out = {
-                'retcode': 1,
-                'stdout': '',
-                'stderr': ''
-        }
-        with self.assertRaisesRegexp(CommandExecutionError, "^zypper command failed: Check zypper logs$"):
-            zypper._zypper_check_result(cmd_out, xml=True)
-
-        cmd_out = {
-            'stdout': '''<?xml version='1.0'?>
-<stream>
- <message type="info">Refreshing service &apos;container-suseconnect&apos;.</message>
- <message type="error">Some handled zypper internal error</message>
- <message type="error">Another zypper internal error</message>
-</stream>
-            ''',
-            'stderr': '',
-            'retcode': 1
-        }
-        with self.assertRaisesRegexp(CommandExecutionError,
-                "^zypper command failed: Some handled zypper internal error\nAnother zypper internal error$"):
-            zypper._zypper_check_result(cmd_out, xml=True)
+        class RunSniffer(object):
+            def __init__(self, stdout=None, stderr=None, retcode=None):
+                self.calls = list()
+                self._stdout = stdout or ''
+                self._stderr = stderr or ''
+                self._retcode = retcode or 0
+
+            def __call__(self, *args, **kwargs):
+                self.calls.append({'args': args, 'kwargs': kwargs})
+                return {'stdout': self._stdout,
+                        'stderr': self._stderr,
+                        'retcode': self._retcode}
+
+        stdout_xml_snippet = '<?xml version="1.0"?><test foo="bar"/>'
+        sniffer = RunSniffer(stdout=stdout_xml_snippet)
+        with patch.dict('salt.modules.zypper.__salt__', {'cmd.run_all': sniffer}):
+            self.assertEqual(zypper.__zypper__.call('foo'), stdout_xml_snippet)
+            self.assertEqual(len(sniffer.calls), 1)
+
+            zypper.__zypper__.call('bar')
+            self.assertEqual(len(sniffer.calls), 2)
+            self.assertEqual(sniffer.calls[0]['args'][0], ['zypper', '--non-interactive', '--no-refresh', 'foo'])
+            self.assertEqual(sniffer.calls[1]['args'][0], ['zypper', '--non-interactive', '--no-refresh', 'bar'])
+
+            dom = zypper.__zypper__.xml.call('xml-test')
+            self.assertEqual(sniffer.calls[2]['args'][0], ['zypper', '--non-interactive', '--xmlout',
+                                                           '--no-refresh', 'xml-test'])
+            self.assertEqual(dom.getElementsByTagName('test')[0].getAttribute('foo'), 'bar')
+
+            zypper.__zypper__.refreshable.call('refresh-test')
+            self.assertEqual(sniffer.calls[3]['args'][0], ['zypper', '--non-interactive', 'refresh-test'])
+
+            zypper.__zypper__.nolock.call('no-locking-test')
+            self.assertEqual(sniffer.calls[4].get('kwargs', {}).get('env', {}).get('ZYPP_READONLY_HACK'), "1")
+            self.assertEqual(sniffer.calls[4].get('kwargs', {}).get('env', {}).get('SALT_RUNNING'), "1")
+
+            zypper.__zypper__.call('locking-test')
+            self.assertEqual(sniffer.calls[5].get('kwargs', {}).get('env', {}).get('ZYPP_READONLY_HACK'), None)
+            self.assertEqual(sniffer.calls[5].get('kwargs', {}).get('env', {}).get('SALT_RUNNING'), "1")
+
+        # Test exceptions
+        stdout_xml_snippet = '<?xml version="1.0"?><stream><message type="error">Booya!</message></stream>'
+        sniffer = RunSniffer(stdout=stdout_xml_snippet, retcode=1)
+        with patch.dict('salt.modules.zypper.__salt__', {'cmd.run_all': sniffer}):
+            with self.assertRaisesRegexp(CommandExecutionError, '^Zypper command failure: Booya!$'):
+                zypper.__zypper__.xml.call('crashme')
+
+            with self.assertRaisesRegexp(CommandExecutionError, "^Zypper command failure: Check Zypper's logs.$"):
+                zypper.__zypper__.call('crashme again')
+
+            zypper.__zypper__.noraise.call('stay quiet')
+            self.assertEqual(zypper.__zypper__.error_msg, "Check Zypper's logs.")
 
     def test_list_upgrades_error_handling(self):
         '''
@@ -129,11 +147,12 @@ class ZypperTestCase(TestCase):
  <message type="error">Another zypper internal error</message>
 </stream>
             ''',
-            'retcode': 1
+            'stderr': '',
+            'retcode': 1,
         }
-        with patch.dict(zypper.__salt__, {'cmd.run_all': MagicMock(return_value=ref_out)}):
+        with patch.dict('salt.modules.zypper.__salt__', {'cmd.run_all': MagicMock(return_value=ref_out)}):
             with self.assertRaisesRegexp(CommandExecutionError,
-                    "^zypper command failed: Some handled zypper internal error\nAnother zypper internal error$"):
+                    "^Zypper command failure: Some handled zypper internal error\nAnother zypper internal error$"):
                 zypper.list_upgrades(refresh=False)
 
         # Test unhandled error
@@ -142,8 +161,8 @@ class ZypperTestCase(TestCase):
             'stdout': '',
             'stderr': ''
         }
-        with patch.dict(zypper.__salt__, {'cmd.run_all': MagicMock(return_value=ref_out)}):
-            with self.assertRaisesRegexp(CommandExecutionError, '^zypper command failed: Check zypper logs$'):
+        with patch.dict('salt.modules.zypper.__salt__', {'cmd.run_all': MagicMock(return_value=ref_out)}):
+            with self.assertRaisesRegexp(CommandExecutionError, "^Zypper command failure: Check Zypper's logs.$"):
                 zypper.list_upgrades(refresh=False)
 
     def test_list_products(self):
@@ -260,8 +279,7 @@ class ZypperTestCase(TestCase):
         :return:
         '''
         test_pkgs = ['vim', 'emacs', 'python']
-        ref_out = get_test_data('zypper-available.txt')
-        with patch.dict(zypper.__salt__, {'cmd.run_stdout': MagicMock(return_value=ref_out)}):
+        with patch('salt.modules.zypper.__zypper__', ZyppCallMock(return_value=get_test_data('zypper-available.txt'))):
             available = zypper.info_available(*test_pkgs, refresh=False)
             self.assertEqual(len(available), 3)
             for pkg_name, pkg_info in available.items():
@@ -286,8 +304,7 @@ class ZypperTestCase(TestCase):
 
         :return:
         '''
-        ref_out = get_test_data('zypper-available.txt')
-        with patch.dict(zypper.__salt__, {'cmd.run_stdout': MagicMock(return_value=ref_out)}):
+        with patch('salt.modules.zypper.__zypper__', ZyppCallMock(return_value=get_test_data('zypper-available.txt'))):
             self.assertEqual(zypper.latest_version('vim'), '7.4.326-2.62')
 
     @patch('salt.modules.zypper.refresh_db', MagicMock(return_value=True))
@@ -298,7 +315,7 @@ class ZypperTestCase(TestCase):
         :return:
         '''
         ref_out = get_test_data('zypper-available.txt')
-        with patch.dict(zypper.__salt__, {'cmd.run_stdout': MagicMock(return_value=ref_out)}):
+        with patch('salt.modules.zypper.__zypper__', ZyppCallMock(return_value=get_test_data('zypper-available.txt'))):
             for pkg_name in ['emacs', 'python']:
                 self.assertFalse(zypper.upgrade_available(pkg_name))
             self.assertTrue(zypper.upgrade_available('vim'))
-- 
2.8.1

Places

File 0044-Unblock-Zypper.-Modify-environment.patch of Package salt.2642

Places