pkgrepo.managed does not disable a yum repo with "disabled: True"

[murzick]

pkgrepo.managed does not disable a yum repo with "disabled: True"

percona:
  pkgrepo.managed:
    - humanname: Percona-Release YUM repository - $basearch
    - baseurl: http://repo.percona.com/release/$releasever/RPMS/$basearch
    - key_url: https://www.percona.com/downloads/RPM-GPG-KEY-percona
    - gpgcheck: 0
    - disabled: True <==

The result is a /etc/yum.repos/percona.repo as follows:

[percona]
gpgcheck=0
key_url=https://www.percona.com/downloads/RPM-GPG-KEY-percona
enabled=1 <==
name=Percona-Release YUM repository - $basearch
baseurl=http://repo.percona.com/release/$releasever/RPMS/$basearch

From minion log:
2016-05-26 07:57:59,586 [salt.state ][INFO ][27072] Running state [percona] at time 07:57:59.586271
2016-05-26 07:57:59,587 [salt.state ][INFO ][27072] Executing state pkgrepo.managed for percona
2016-05-26 07:57:59,588 [py.warnings ][WARNING ][27072] /usr/lib/python2.7/site-packages/salt/states/pkgrepo.py:310: DeprecationWarning: The enabled argument has been deprecated in favor of disabled.
2016-05-26 07:57:59,637 [salt.state ][INFO ][27072] {'repo': 'percona'}
2016-05-26 07:57:59,637 [salt.state ][INFO ][27072] Completed state [percona] at time 07:57:59.637187 duration_in_ms=50.916

Salt on master and minion is version 2016.3.0. Issue is also reproducible on previous 2015.8.10 version.

Salt Version:
           Salt: 2016.3.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.3
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.6.6 (r266:84292, Jul 23 2015, 15:22:56)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.5.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: centos 6.8 Final
        machine: x86_64
        release: 2.6.32-358.18.1.el6.x86_64
         system: Linux
        version: CentOS 6.8 Final

[Ch3LL]

@murzick I am able to replicate this on 2015.8.10 as well.

Looks like the issue is here in states/pkgrepo.py:

        if 'disabled' in kwargs:
            if 'enabled' in kwargs:
                ret['result'] = False
                ret['comment'] = 'Only one of enabled/disabled is permitted'
                return ret
            _reverse = lambda x: '1' if x == '0' else '0'
            kwargs['enabled'] = _reverse(_val(kwargs.pop('disabled')))
        elif 'enabled' in kwargs:
            kwargs['enabled'] = _val(kwargs['enabled'])
        if 'name' not in kwargs:
            # Fall back to the repo name if humanname not provided
            kwargs['name'] = repo

    if kwargs.pop('enabled', None):
        kwargs['disabled'] = False
        salt.utils.warn_until(
            'Carbon',
            'The `enabled` argument has been deprecated in favor of '
            '`disabled`.'
        )

Looks like it is actually changing disabled to enabled. Then if enabled is set it sets disabled=False. Looks like we need to get this fixed up. Thanks!

[stardust85]

How could you release Boron with this bug?!

[galet]

Should be fixed by the above merge request (#35055). It would be great if someone could verify.

[Ch3LL]

@galet i just tested on the head of 2016.3 and its working for me. I'll go ahead and close this issue.

@stardust85 apologies for the bug. I think the best way to ensure this does not happen again is to write a test. I will label this as needstestcase and add an issue in our qa repo to get a test written for this issue.

[damon-atkins]

I am running Salt-Minion 2016.3.3 CentOS7 its still or is now adding disabled=True into the file if you set a state file enabled: 0 or even if an existing entry in the file has enabled=0 when you have multiple repo in a single file.

[Ch3LL]

@damon-atkins would you mind opening a separate issue for this with the exact state file/command you are using please? Thanks Looks like i forgot to close this issue earlier. So i'll close this and we will track what you are reporting in a separate issue. please mention this issue in the issue report.