Merge pull request #32892 from isbm/isbm-zypper-env-variables

Resolve Zypper locks on asynchronous calls

salt/modules/zypper.py

@@ -18,10 +18,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
@@ -58,65 +61,226 @@ def __virtual__():
     return __virtualname__
 
 
-def _zypper(*opts):
+class _Zypper(object):
     '''
-    Return zypper command with default options as a list.
-
-    opts
-        additional options for zypper command
-
+    Zypper parallel caller.
+    Validates the result and either raises an exception or reports an error.
+    Allows serial zypper calls (first came, first won).
     '''
-    cmd = ['zypper', '--non-interactive']
-    cmd.extend(opts)
 
-    return cmd
+    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 _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 int(retcode) not in [0, 100, 101, 102, 103]
+    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
 
-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.
+        # Call config
+        self.__xml = False
+        self.__no_lock = False
+        self.__no_raise = False
+        self.__refresh = False
 
-    result
-        The result of a zypper command called with cmd.run_all
+    def __getattr__(self, item):
+        '''
+        Call configurator.
 
-    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 "")
+        :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:
-            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.__dict__[item]
 
-        raise CommandExecutionError("zypper command failed: {0}".format(
-            msg and os.linesep.join(msg) or "Check zypper logs"))
+        # Prevent the use of "refreshable" together with "nolock".
+        if self.__no_lock:
+            self.__no_lock = not self.__refresh
 
-    return result['stdout']
+        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.")
+
+            __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
+
+        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))
+
+        return self._is_xml_mode() and dom.parseString(self.__call_result['stdout']) or self.__call_result['stdout']
+
+
+__zypper__ = _Zypper()
 
 
 def list_upgrades(refresh=True):
@@ -136,10 +300,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')
 
@@ -197,7 +360,6 @@ def info_installed(*names, **kwargs):
                 t_nfo['source'] = value
             else:
                 t_nfo[key] = value
-
         ret[pkg_name] = t_nfo
 
     return ret
@@ -236,8 +398,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:
@@ -286,6 +448,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
@@ -299,7 +466,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()
@@ -317,10 +484,15 @@ 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
@@ -328,7 +500,7 @@ def upgrade_available(name):
         salt '*' pkg.upgrade_available <package name>
     '''
     # The "not not" tactic is intended here as it forces the return to be False.
-    return not not latest_version(name)  # pylint: disable=C0113
+    return not not latest_version(name, **kwargs)  # pylint: disable=C0113
 
 
 def version(*names, **kwargs):
@@ -361,7 +533,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):
@@ -404,12 +576,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)
@@ -421,6 +588,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
 
 
@@ -446,7 +614,7 @@ def _get_repo_info(alias, repos_cfg=None):
             if val == 'NONE':
                 ret[key] = None
         return ret
-    except (ValueError, configparser.NoSectionError) as error:
+    except (ValueError, configparser.NoSectionError):
         return {}
 
 
@@ -494,9 +662,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 {
@@ -586,14 +752,7 @@ def mod_repo(repo, **kwargs):
                 )
 
         # Add new repo
-        _zypper_check_result(
-            __salt__['cmd.run_all'](
-                _zypper('-x', 'ar', url, repo),
-                python_shell=False,
-                output_loglevel='trace',
-                ),
-            xml=True
-        )
+        __zypper__.xml.call('ar', url, repo)
 
         # Verify the repository has been added
         repos_cfg = _get_configured_repos()
@@ -629,12 +788,7 @@ def mod_repo(repo, **kwargs):
 
     if cmd_opt:
         cmd_opt.append(repo)
-        ret = __salt__['cmd.run_all'](
-            _zypper('-x', 'mr', *cmd_opt),
-            python_shell=False,
-            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:
@@ -657,9 +811,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:
@@ -799,8 +952,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:
@@ -810,9 +962,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))
@@ -820,8 +970,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()
@@ -857,18 +1006,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
 
 
@@ -888,8 +1034,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)
 
@@ -1002,9 +1147,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]
@@ -1041,8 +1184,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}
 
@@ -1071,8 +1213,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}
 
@@ -1205,10 +1346,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')] = {
@@ -1271,20 +1409,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
 
 
@@ -1329,16 +1463,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
 
@@ -1391,10 +1523,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"),

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)
+        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
 
-        cmd_out = {
-                'retcode': 0,
-                'stdout': 'result',
-                'stderr': ''
-        }
-        out = zypper._zypper_check_result(cmd_out)
-        self.assertEqual(out, "result")
+            def __call__(self, *args, **kwargs):
+                self.calls.append({'args': args, 'kwargs': kwargs})
+                return {'stdout': self._stdout,
+                        'stderr': self._stderr,
+                        'retcode': self._retcode}
 
-        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)
+        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)
 
-        cmd_out = {
-                'retcode': 1,
-                'stdout': '',
-                'stderr': ''
-        }
-        with self.assertRaisesRegexp(CommandExecutionError, "^zypper command failed: Check zypper logs$"):
-            zypper._zypper_check_result(cmd_out, xml=True)
+            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'])
 
-        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)
+            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'))