Salt: Use yum lib

[gdubroeucq]

Description of Issue/Question

I was wondering why Salt don't use the yum lib for CentOS.

With the yum lib, my salt-call lasts less than 7.5 s instead of 11 s . (can be probably more if I implement totally but the big gain is already here)

The patch makes a gain of 32%, not negligible at all.

Edit: Each run is with "test=True"

Steps to Reproduce Issue

To rely, I made a highstate of ~120 actions (file,package, service mainly) and I manage ~50 packages. We focus on the package's actions and more precisely in the module _yumpkg_.

Steps during the salt-call:

• 1x yum clean expire-cache & yum check update
  Without patch: ~3.5s
  With patch: ~1.6s

• 8x yum list available _packages_
  Gain of 200ms per call (Total: ~1.6s )

The gain of using Salt is clearly lost in this part for CentOS

Is there a specific reason not to use the yum lib?

yum lib: http://yum.baseurl.org/api/yum/yum/__init__.html#yum.__init__._YumPreRepoConf

I can show you the modifications who i made if you ask me. I just wanted to have a clear topic first.

Versions Report

Salt Version:
Salt: 2016.11.3

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: Not Installed
gitdb: Not Installed
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.7.2
libgit2: Not Installed
libnacl: Not Installed
M2Crypto: Not Installed
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.4.8
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pygit2: Not Installed
Python: 2.7.5 (default, Jun 17 2014, 18:11:42)
python-gnupg: Not Installed
PyYAML: 3.10
PyZMQ: 15.3.0
RAET: Not Installed
smmap: Not Installed
timelib: Not Installed
Tornado: 4.2.1
ZMQ: 4.1.4

System Versions:
dist: centos 7.1.1503 Core
machine: x86_64
release: 3.10.0-229.el7.x86_64
system: Linux
version: CentOS Linux 7.1.1503 Core

[gtmanfred]

@terminalmage wanna take this one?

Thanks,
Daniel

[terminalmage]

Salt used to use the yum Python module, but moved away from it for a few reasons. Firstly, its documentation is quite poor. Secondly, there were several bugs which caused serious problems. For instance, in certain RHEL/CentOS releases, using the yum module to remove a package would just force the removal, so one could do a pkg.remove gzip and uninstall most of the packages on the system, making it unusable. It's possible that this is due to incorrect usage of the Python module, but again, that goes back to its terrible documentation.

One other reason why it's not necessarily a good idea to use it is that it's only typically installed for a single version of Python. So, when Salt is run on a different release this module isn't available. This happened on RHEL/CentOS 5 where the system Python was version 2.4, and would happen now if Salt is run under Python 3.

[gdubroeucq]

Thanks for your answer.

I agree, the documentation is terrible. With the yum lib, we should normally only gain the duration of multiple system call and the python interpreter. But when I used it, I found the gain was too much important (compared without the yum lib).

So without using the lib, I found a probably unused feature in the _check_update_ that slow the _salt_call_

What is the feature

_/usr/share/yum-cli/yumcommands.py:_

class CheckUpdateCommand(YumCommand):
[...]
    def doCommand(self, base, basecmd, extcmds, repoid=None):
    [...]
        # Add check_running_kernel call, if updateinfo is available.
  ---> if (base.conf.autocheck_running_kernel and  
            updateinfo._repos_downloaded(base.repos.listEnabled())):
            def _msg(x):
                base.verbose_logger.info("%s", x)
            updateinfo._check_running_kernel(base, base.upinfo, _msg)

This code tells the user when a critical/security update is available for the kernel. Salt seems not using it.

This feature adds more than 1 second when using large repo like EPEL.

How to disable the feature

To disable the feature without change the code, we can use this:

yum --setopt=autocheck_running_kernel=false check-updates

instead of

yum check-updates

_salt/modules/yumpkg.py:_

def refresh_db(**kwargs):

    retcodes = {
        100: True,
        0: None,
        1: False,
    }
    check_update_ = kwargs.pop('check_update', True)

    repo_arg = _get_repo_options(**kwargs)
    exclude_arg = _get_excludes_option(**kwargs)
    branch_arg = _get_branch_option(**kwargs)

    clean_cmd = [_yum(), '--quiet', 'clean', 'expire-cache']
--> update_cmd = [_yum(), '--quiet', '--setopt=autocheck_running_kernel=false', 'check-update']  
    for args in (repo_arg, exclude_arg, branch_arg):
        if args:
            clean_cmd.extend(args)
            update_cmd.extend(args)

    __salt__['cmd.run'](clean_cmd, python_shell=False)
    if check_update_:
        result = __salt__['cmd.retcode'](update_cmd,
                                         output_loglevel='trace',
                                         ignore_retcode=True,
                                         python_shell=False)
        return retcodes.get(result, False)
    return True

What do you think?

[terminalmage]

Given that the refresh_db function doesn't need this particular feature, I'm comfortable with adding that option to the command. Are you comfortable opening a pull request to make this change?

[damon-atkins]

Please check if older OS support the option e.g. CentOS 5

[gtmanfred]

CentOS 5 has received its last update for salt, so we don't need to check
that one.

On Wed, Jul 26, 2017 at 10:05 AM, Damon Atkins [e-mail]
wrote:

Please check if older OS support the option e.g. CentOS 5

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/issues/42456#issuecomment-318101123,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAssoVKd8ngafk-N0lr0Su4lblGImfgRks5sR2O0gaJpZM4OfhPY
.

[terminalmage]

It looks like this is specific to Cent 7, I can't find in 6, nor in Fedora 24 or Fedora 25's version of dnf. It doesn't keep the check-update from completing, however it does set an exit status that denotes an error occurred during the refresh, which will mess with the pkg.refresh_db functions return value. So it's best to limit this via something like:

if __grains__['os_family'] == 'RedHat' and __grains__['osmajorrelease'] == 7: