Repository: saltstack/salt
Pull request: Fix for #38838
### What does this PR do?
fix behavior of aptpkg.list_pkgs function. Now it works as in description "List the packages currently INSTALLED in a dict" 
### What issues does this PR fix or reference?
fix #38838
### Previous Behavior
return wrong data
check with #38838 issue

### New Behavior

returns info only about installed packages

### Tests written?

No

### Commits signed with GPG?

Yes

Please review [Salt's Contributing Guide](https://docs.saltstack.com/en/latest/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
Failing to remove nginx
### Description of Issue/Question
Hi there,

I am playing with SaltStack, trying to get a feel for everything. I've ran into what I think could possibly be a bug - removing nginx does not work correctly. All involved machines are running fully updated Ubuntu 16.04 and all packages are current. For what it's worth, the salt minions are all running as VMware virtual machines with 4GB of RAM each. None of the minions have any extra apt repos besides SaltStack (no DotDeb or anything like that).

### Setup
This is my top.sls file:
```
base:
  '*':
    - core
```

This is my core/init.sls file:
```
remove-things:
  pkg.removed:
    - name: nginx
```

### Steps to Reproduce Issue
- Install nginx on minion
- On master, run sudo salt 'vm1' state.apply core
- First result:
```
vm1:
----------
          ID: remove-things
    Function: pkg.removed
        Name: nginx
      Result: False
     Comment: The following packages failed to remove: nginx.
     Started: 21:21:08.401320
    Duration: 1315.336 ms
     Changes:
              ----------
              nginx:
                  ----------
                  new:
                      1
                  old:
                      1.10.0-0ubuntu0.16.04.4

```

It does succeed at removing the nginx package, but claims to have failed.

Now, run it again:
- On master, run sudo salt 'vm1' state.apply core
```

vm1:
----------
          ID: remove-things
    Function: pkg.removed
        Name: nginx
      Result: False
     Comment: The following packages failed to remove: nginx.
     Started: 21:22:14.792268
    Duration: 977.843 ms
     Changes:

```

The exact same process does NOT fail with simple packages like "sl" and "cowsay".

### Versions Report
```
root@nebula:/srv/salt# salt --versions-report
Salt Version:
           Salt: 2016.11.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: 3.5.0
       dateutil: 2.4.2
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   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.7.12 (default, Nov 19 2016, 06:48:10)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
        machine: x86_64
        release: 4.4.0-59-generic
         system: Linux
        version: Ubuntu 16.04 xenial
```

Hints from issue discussion:
@Zorlin looks like i'm able to replicate this. Looks like salt runs this command to check if its been removed:

`dpkg-query --showformat '${Status} ${Package} ${Version} ${Architecture}' -W ` If you manually uninstall nginx as well the following packages still show up in the output of that command: nginx-common and nginx-core. So it seems removing nginx does not unisntall those packages.

I will labe this as a bug as we could probably improve our detection of the exact package. Thanks

Here is a docker container for someone that wants to quickly replicate this:

1. `docker run -it -v /home/ch3ll/git/salt/:/testing/ ch3ll/issues:38838 salt-call --local state.sls test -ldebug` (where /home/ch3ll/git/salt is a local cloned git salt repo)
Any update on this?
There is not currently an update. We are more then willing to accept a PR for this though since its not currently assigned out
The issue is in aptpkg.list_pkgs function. It returns wrong information - marks uninstalled package as virtual.

Steps to reproduce:

```
# dpkg -l | grep -c nginx
0
```
Install nginx package
```
# salt-call pkg.install nginx

local:
    ----------
    httpd:
        ----------
        new:
            1
        old:
    httpd-cgi:
        ----------
        new:
            1
        old:
    nginx:
        ----------
        new:
            1.10.3-0ubuntu0.16.04.2
        old:
    nginx-common:
        ----------
        new:
            1.10.3-0ubuntu0.16.04.2
        old:
    nginx-core:
        ----------
        new:
            1.10.3-0ubuntu0.16.04.2
        old:
```
As you can see we have wrong data here too - in return we got httpd and httpd-cgi packages marked as virtual ( version 1)

check for installed packages
```
# dpkg -l | grep nginx
ii  nginx                            1.10.3-0ubuntu0.16.04.2                    all          small, powerful, scalable web/proxy server
ii  nginx-common                     1.10.3-0ubuntu0.16.04.2                    all          small, powerful, scalable web/proxy server - common files
ii  nginx-core                       1.10.3-0ubuntu0.16.04.2                    amd64        nginx web/proxy server (core version)
```

Remove package nginx

```
# salt-call pkg.remove nginx

local:
    ----------
    nginx:
        ----------
        new:
            1
        old:
            1.10.3-0ubuntu0.16.04.2
```

check 
```
# dpkg -l | grep nginx
ii  nginx-common                     1.10.3-0ubuntu0.16.04.2                    all          small, powerful, scalable web/proxy server - common files
ii  nginx-core                       1.10.3-0ubuntu0.16.04.2                    amd64        nginx web/proxy server (core version)
```

get packages list with list_pkgs fuction
```
# salt-call pkg.list_pkgs --output=json |  jq '."local" |=with_entries(select( .key | test("nginx") ))'
{
  "local": {
    "nginx-core": "1.10.3-0ubuntu0.16.04.2",
    "nginx": "1",
    "nginx-common": "1.10.3-0ubuntu0.16.04.2"
  }
}
```

So nginx marked as virtual and added to result. 

That code responsible for such behavior 

https://github.com/saltstack/salt/blob/8e99aeed6ac69461cfb3fa1549800ff4b8260d98/salt/modules/aptpkg.py#L1401-L1408


nginx-core - provides nginx and code above add nginx package to installed dict with version 1
Thanks for looking into it @samodid :)

I have nothing further to add just yet, but hopefully this can be solved.
@Zorlin 
please try my fix. It should help

Commit message: fix #38838

  update aptpkg.list_pkgs behavior

Patch to review:
diff --git a/salt/modules/aptpkg.py b/salt/modules/aptpkg.py
index b79e6fa22999..c1928da963f5 100644
--- a/salt/modules/aptpkg.py
+++ b/salt/modules/aptpkg.py
@@ -1401,7 +1401,9 @@ def list_pkgs(versions_as_list=False,
         for realpkg, provides in six.iteritems(virtpkgs_all):
             # grep-available returns info on all virtual packages. Ignore any
             # virtual packages that do not have the real package installed.
-            if realpkg in ret['installed']:
+            # _get_virtual() do not use grep-available if HAS_APT is true
+            # so we can skip loop below
+            if not HAS_APT and realpkg in ret['installed']:
                 virtpkgs.update(provides)
         for virtname in virtpkgs:
             # Set virtual package versions to '1'


Code review comments:
diff_hunk: @@ -1401,7 +1401,9 @@ def list_pkgs(versions_as_list=False,
         for realpkg, provides in six.iteritems(virtpkgs_all):
             # grep-available returns info on all virtual packages. Ignore any
             # virtual packages that do not have the real package installed.
-            if realpkg in ret['installed']:
+            # _get_virtual() do not use grep-available if HAS_APT is true
+            # so we can skip loop below
+            if not HAS_APT and realpkg in ret['installed']:
path: salt/modules/aptpkg.py
text: @user1:
Now if `not HAS_APT` all the `if not removed` case does nothing.
I mean it's better to add the `not HAS_APT` case to the enclosing `if not removed` 11 lines above:
```python
if not removed and not HAS_APT:
```

@author:
Hello Dmitry,
thanks for your comment and suggestion. It makes sense. 

I've checked that change - it works.  

```
# salt-call pkg.list_pkgs --output=json -l quiet |  jq '."local" |=with_entries(select( .key | test("nginx") ))'
{
  "local": {
    "nginx-core": "1.10.3-0ubuntu0.16.04.2",
    "nginx-common": "1.10.3-0ubuntu0.16.04.2"
  }
}
```