Salt: [BUG] pkgrepo.managed always reported as "changed" on Centos7

Created on 24 Jun 2020 · 12Comments · Source: saltstack/salt

Description
We use several (15+) yum repositories, managed by saltstack. After upgrading to 3001, all repos are reported as changed on every salt-call.

Setup
e.g. salt repo (salt/repo.sls):

saltstack_repo:
  pkgrepo.managed:
    - humanname: Artifactory Saltstack Repository
    - name: saltstack
    - baseurl: {{ pillar.common.arti_url }}/rpm-saltstack-py3/$releasever/$basearch/latest/
    - gpgkey: {{ pillar.common.arti_url }}/repo/gpg/RPM-GPG-KEY-SALTSTACK

Steps to Reproduce the behavior

[root@machine /]# salt-call state.sls salt/repo
local:
----------
          ID: saltstack_repo
    Function: pkgrepo.managed
        Name: saltstack
      Result: True
     Comment: Configured package repo 'saltstack'
     Started: 07:25:36.722061
    Duration: 1225.71 ms
     Changes:
              ----------
              repo:
                  saltstack

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   1.226 s
[root@machine /]# salt-call state.sls salt/repo
local:
----------
          ID: saltstack_repo
    Function: pkgrepo.managed
        Name: saltstack
      Result: True
     Comment: Configured package repo 'saltstack'
     Started: 07:30:23.282547
    Duration: 1226.098 ms
     Changes:
              ----------
              repo:
                  saltstack

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   1.226 s

Don't know if it has something to do with issue, but on debug logs I've got:
[DEBUG ] Could not LazyLoad pkg.expand_repo_def: 'pkg.expand_repo_def' is not available.

Expected behavior
No changes should be reported:
Succeeded: 1 (changed=0)

Versions Report

salt --versions-report

Salt Version:
Salt: 3001

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 2.8.1
docker-py: 4.2.1
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 2.11.2
libgit2: Not Installed
M2Crypto: 0.35.2
Mako: Not Installed
msgpack-pure: Not Installed
msgpack-python: 0.6.2
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 3.6.8 (default, Apr 2 2020, 13:34:55)
python-gnupg: Not Installed
PyYAML: 5.3.1
PyZMQ: 18.0.2
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.1

System Versions:
dist: centos 7 Core
locale: UTF-8
machine: x86_64
release: 5.6.13-100.fc30.x86_64
system: Linux
version: CentOS Linux 7 Core

Bug Magnesium ZMQ salt-call severity-high

Source

ata-sql

👍3

Most helpful comment

@meelmeal thank you for the update, @cmcmarrow is working on isolating the issue, I have this as in progress in the project board, but that view is terribly obscure, apologies. I am looking into something better to communicate the status, today this is what I've got. But yes! thank you for the update.

sagetherage on 18 Aug 2020

👍2

All 12 comments

@ata-sql Thanks for the report. We'll get this one fixed up asap.

garethgreenaway on 25 Jun 2020

@ata-sql Thanks for the report. We'll get this one fixed up asap.

The get_repo isn't correctly matching the existing repository name with the one in the state file.

The repository name is passed to the get_repo by the "repo" named parameter, which is sanitized and generated by "name", or "humanname", or other property. The original state parameter "name" goes with the **kwargs. Previously only one of kwargs parameters was passed. Sending all of them, including the named parameter "name" in kwargs, creates the actual problem.

As other backend package modules are using "repo" to get the repository name and also receiving all kwargs, I did the equivalent changes in yumpkg.get_repo in PR #57806. It's "working for me", can you validate it?

piterpunk on 30 Jun 2020

Sure I can help, but I wasn't involved in developing saltstack earlier. How can I help? Build salt-minion rpm from that branch and test it? Or it is salt-master change rather?

ata-sql on 1 Jul 2020

Sure I can help, but I wasn't involved in developing saltstack earlier. How can I help? Build salt-minion rpm from that branch and test it? Or it is salt-master change rather?

You can build the salt-minion from that branch and test it. I guess that's the most correct approach.

But, as the change is very small, it's easier to just copy the file yumpkg.py over the one that you have in your minion (in my CentOS 7 test machine it's in /usr/lib/python3.6/site-packages/salt/modules/yumpkg.py)

piterpunk on 1 Jul 2020

Verified that way and bug is fixed. Thanks.

ata-sql on 1 Jul 2020

@garethgreenaway it seems that PR #57806 really fixes this issue.

piterpunk on 1 Jul 2020

@piterpunk Thanks. Just reviewed that PR. Once we're out of code freeze, we'll get that one merged.

garethgreenaway on 6 Jul 2020

I am not sure why this wasn't merged into the point release, but I am updating some thing to address our apparent process issue and we will get that into Magnesium release soon.

sagetherage on 23 Jul 2020

Just providing an update ... I've upgraded both Master and Minion to Salt 3001.1 but I'm still getting a "Changed" status for pkgrepo.managed states :(