state pkg.installed not working on FreeBSD when version is specified

[angeloudy]

I have the following state file.

mariadb_pkg:
  pkg.installed:
    - name: mariadb103-server
    - version: {{ salt['pillar.get']('mariadb:version', default_db_version) }}

It worked fine for two year until I recently upgraded salt to 2019.2.2

When I run it in debug mode, I got the following message:

16:02:31,455 [DEBUG   ] Current version ({'origin': 'databases/mariadb103-server', 'version': ['10.3.20']}) did not match desired version specification (10.3.20), adding to installation targets

It seems that salt was trying to compare a dict with str and decided that it didn't match.

Versions Report

Salt Version:
           Salt: 2019.2.2

Dependency Versions:
           cffi: 1.13.2
       cherrypy: Not Installed
       dateutil: 2.8.0
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.1
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: 1.4.2.post1
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.9 (default, Nov 12 2019, 23:28:35)
   python-gnupg: Not Installed
         PyYAML: 5.1
          PyZMQ: 18.1.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist:
         locale: UTF-8
        machine: amd64
        release: 12.1-RELEASE
         system: FreeBSD
        version: Not Installed

[waynew]

@angeloudy what version were you previously on?

[angeloudy]

@waynew
My previous version is 2018.3.3

[asomers]

This issue is addressed by PR #54074 . However, I don't think that the currently posted PR contains the best possible solution.

[waynew]

I'm working on verifying this, still getting Salt up & running on a BSD box.

[waynew]

I was able to reproduce this, and discovered the actual cause (discussed at #54074). There's a bit more digging to decide on the appropriate fix, but for a workaround, one could patch states/pkg.py removing these two lines:

https://github.com/saltstack/salt/blob/4fed7e0/salt/states/pkg.py#L1844-L1845

I'm not sure why that was introduced a very long time ago, but it appears that the pkgng.py state ends out doing the wrong thing in this case.

[asomers]

I think I know why that change was introduced. Every FreeBSD port has an "origin", which is basically the category of the port. Categories include stuff like "graphics", "devel", "astro", etc. Though rare, two ports can have the same name but different origins. Examples include devel/atlas and math/atlas or multimedia/dragon and devel/dragon. When installing such a port through pkg(8), you must specify the origin in order to unambiguously identify the correct package. So I would say we cannot simply remove the two lines that set with_origin.

[waynew]

@asomers that makes a lot of sense.

I wonder - would it make sense to allow passing with_origin in? With a default of True on FreeBSD?

Another thought is that if we can unambiguously return an installed version there's really no reason for us to require a with_origin, right?

[asomers]

My idea is to replace list_pkgs's with_origin flag with a verbose flag, that would be recognized by all pkg implementations. Without verbose set, the output would look like it does today without with_origin set. But with verbose, the output would look like it does today when with_origin is set, for all package implementations. But the origin field would only be present where it makes sense. So For FreeBSD it might return something like {'dragon': {'version': '1.0.0', 'origin': 'devel'}} but on Debian it would exclude origin, like this: {'dragon': {'version': '1.0.0'}}. The nice thing about this approach is that it would minimize inconsistencies between different OSes.

[fzipi]

Is there an expected timeline for this?

[asomers]

@fzipi I don't have any time allocated for it right now.

[whytewolf]

zd-4827

[whytewolf]

So the workaround in #55630 (comment) does work, however, it introduces other issues. So it is correct that is not a valid fix.

Issue created by the workaround.

----------
ID: freebsd_packages_installed_java/openjdk7
Function: pkg.installed
Name: java/openjdk7
Result: False
Comment: An exception occurred in this state: Traceback (most recent call last):
File "/usr/local/lib/python2.7/site-packages/salt/state.py", line 1933, in call
**cdata['kwargs'])
File "/usr/local/lib/python2.7/site-packages/salt/loader.py", line 1951, in wrapper
return f(*args, **kwargs)
File "/var/cache/salt/minion/extmods/states/pkg.py", line 1787, in installed
new_caps=new_caps)
File "/var/cache/salt/minion/extmods/states/pkg.py", line 857, in _verify_install
cver = [k for k, v in six.iteritems(new_pkgs) if v['origin'] == pkgname]
TypeError: list indices must be integers, not unicode
Started: 12:09:51.413210
Duration: 58826.337 ms
Changes:
Name: apr - Function: pkg.installed - Result: Clean Started: - 12:10:50.244422 Duration: 40.43 ms

[s0undt3ch]

@krionbsd is looking into this one

[sagetherage]

this still needs a test case written, I removed Aluminium label from the PR, but will leave it on the issue to see if we can find time to write tests next week. If not, this will likely be de-scoped from the Aluminium release.