Repository: ansible/ansible
Pull request: apt: mark installed packages manual
##### SUMMARY
<!--- Describe the change, including rationale and design decisions -->
Fixes https://github.com/ansible/ansible/issues/28907
<!---
If you are fixing an existing issue, please include "Fixes #nnn" in your
commit message and your description; but you should still explain what
the change does.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bugfix Pull Request

##### COMPONENT NAME
apt

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes below -->
```
devel
```


##### ADDITIONAL INFORMATION

Problem statement:
apt module does not mark installed packages as manually installed.
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
apt

##### ANSIBLE VERSION
```
$ ansible --version
ansible 2.2.1.0
  config file = 
  configured module search path = Default w/o overrides
$ ansible-playbook --version
ansible-playbook 2.2.1.0
  config file = 
  configured module search path = Default w/o overrides
```

##### CONFIGURATION
Default configuration

##### OS / ENVIRONMENT
Ubuntu 14.04 (and maybe other Ubuntu versions)

##### SUMMARY
<!--- Explain the problem briefly -->
Packages installed by ansible's apt module may be accidentally removed later by autoremove, because ansible does not mark them as manually installed.

This can happen if an apt package was installed but marked as automatically installed before ansible was run. This usually happens if the requested package was installed as a dependency of another package.

##### STEPS TO REPRODUCE
This is a simplification of a larger system, but it could also represent a series of actions performed over a long period of time on the same host. These packages were chosen because their dependency structure demonstrates the issue; there is not special significance to the packages themselves.

Run `ansible-playbook` with the following playbook on Ubuntu 14.04

`libcaca-dev` depends on `libslang2-dev`. Installing `libcaca-dev` also installs 'libslang2-dev` and marks `libcaca-dev` as manually installed and `libslang2-dev` as automatically installed.

The apt step for `libslang2-dev` does nothing, because `libslang2-dev` is already installed (it should be marked manually installed at this point, but it isn't).

The uninstall step for `libcaca-dev` uninstalls `libcaca-dev`, and since `autoremove` is specified it also removes any automatically-installed packages that are no longer needed by manually installed packages; in this case that means removing `libslang2-dev`.

```yaml

---
- hosts: localhost
  tasks:
    - block:
      - name: "install_parent"
        apt:
          name: "{{ item }}"
          install_recommends: no
          state: present
        with_items:
          "libcaca-dev"
      - name: "install_child"
        apt:
          name: "{{ item }}"
          install_recommends: no
          state: present
        with_items:
          "libslang2-dev"
      - name: "uninstall_parent"
        apt:
          name: "{{ item }}"
          install_recommends: no
          state: absent
          autoremove: yes
          purge: yes
        with_items:
          "libcaca-dev"
```

##### EXPECTED RESULTS
`libslang2-dev` should be installed and marked as manually installed after running this playbook.

##### ACTUAL RESULTS
`libslang2-dev` is uninstalled by autoremove.

```
No config file found; using defaults
 [WARNING]: Host file not found: /etc/ansible/hosts

 [WARNING]: provided hosts list is empty, only localhost is available

Loading callback plugin default of type stdout, v2.0 from /usr/local/lib/python2.7/dist-packages/ansible/plugins/callback/__init__.pyc

PLAYBOOK: playbook.yml *********************************************************
1 plays in playbook.yml

PLAY [localhost] ***************************************************************

TASK [setup] *******************************************************************
Using module file /usr/local/lib/python2.7/dist-packages/ansible/modules/core/system/setup.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: root
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo ~/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890 `" && echo ansible-tmp-1504235772.78-38795745409890="` echo ~/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890 `" ) && sleep 0'
<127.0.0.1> PUT /tmp/tmpiIa3mI TO /home/austin/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890/setup.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/austin/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890/ /home/austin/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890/setup.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python2 /home/austin/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890/setup.py; rm -rf "/home/austin/.ansible/tmp/ansible-tmp-1504235772.78-38795745409890/" > /dev/null 2>&1 && sleep 0'
ok: [localhost]

TASK [install_parent] **********************************************************
task path: /home/austin/zoox/ansible-bug/playbook.yml:5
Using module file /usr/local/lib/python2.7/dist-packages/ansible/modules/core/packaging/os/apt.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: root
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo ~/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828 `" && echo ansible-tmp-1504235773.17-34190501703828="` echo ~/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828 `" ) && sleep 0'
<127.0.0.1> PUT /tmp/tmpfwLaGe TO /home/austin/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828/apt.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/austin/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828/ /home/austin/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828/apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python2 /home/austin/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828/apt.py; rm -rf "/home/austin/.ansible/tmp/ansible-tmp-1504235773.17-34190501703828/" > /dev/null 2>&1 && sleep 0'
changed: [localhost] => (item=[u'libcaca-dev']) => {
    "cache_update_time": 1504227095, 
    "cache_updated": false, 
    "changed": true, 
    "diff": {}, 
    "invocation": {
        "module_args": {
            "allow_unauthenticated": false, 
            "autoremove": false, 
            "cache_valid_time": 0, 
            "deb": null, 
            "default_release": null, 
            "dpkg_options": "force-confdef,force-confold", 
            "force": false, 
            "install_recommends": false, 
            "name": [
                "libcaca-dev"
            ], 
            "only_upgrade": false, 
            "package": [
                "libcaca-dev"
            ], 
            "purge": false, 
            "state": "present", 
            "update_cache": false, 
            "upgrade": null
        }, 
        "module_name": "apt"
    }, 
    "item": [
        "libcaca-dev"
    ], 
    "stderr": "", 
    "stdout": "Reading package lists...\nBuilding dependency tree...\nReading state information...\nThe following extra packages will be installed:\n  libslang2-dev\nThe following NEW packages will be installed:\n  libcaca-dev libslang2-dev\n0 upgraded, 2 newly installed, 0 to remove and 46 not upgraded.\nNeed to get 0 B/855 kB of archives.\nAfter this operation, 4397 kB of additional disk space will be used.\nSelecting previously unselected package libslang2-dev:amd64.\n(Reading database ... 276803 files and directories currently installed.)\nPreparing to unpack .../libslang2-dev_2.2.4-15ubuntu1_amd64.deb ...\nUnpacking libslang2-dev:amd64 (2.2.4-15ubuntu1) ...\nSelecting previously unselected package libcaca-dev.\nPreparing to unpack .../libcaca-dev_0.99.beta18-1ubuntu5_amd64.deb ...\nUnpacking libcaca-dev (0.99.beta18-1ubuntu5) ...\nProcessing triggers for man-db (2.6.7.1-1ubuntu1) ...\nSetting up libslang2-dev:amd64 (2.2.4-15ubuntu1) ...\nSetting up libcaca-dev (0.99.beta18-1ubuntu5) ...\n", 
    "stdout_lines": [
        "Reading package lists...", 
        "Building dependency tree...", 
        "Reading state information...", 
        "The following extra packages will be installed:", 
        "  libslang2-dev", 
        "The following NEW packages will be installed:", 
        "  libcaca-dev libslang2-dev", 
        "0 upgraded, 2 newly installed, 0 to remove and 46 not upgraded.", 
        "Need to get 0 B/855 kB of archives.", 
        "After this operation, 4397 kB of additional disk space will be used.", 
        "Selecting previously unselected package libslang2-dev:amd64.", 
        "(Reading database ... 276803 files and directories currently installed.)", 
        "Preparing to unpack .../libslang2-dev_2.2.4-15ubuntu1_amd64.deb ...", 
        "Unpacking libslang2-dev:amd64 (2.2.4-15ubuntu1) ...", 
        "Selecting previously unselected package libcaca-dev.", 
        "Preparing to unpack .../libcaca-dev_0.99.beta18-1ubuntu5_amd64.deb ...", 
        "Unpacking libcaca-dev (0.99.beta18-1ubuntu5) ...", 
        "Processing triggers for man-db (2.6.7.1-1ubuntu1) ...", 
        "Setting up libslang2-dev:amd64 (2.2.4-15ubuntu1) ...", 
        "Setting up libcaca-dev (0.99.beta18-1ubuntu5) ..."
    ]
}

TASK [install_child] ***********************************************************
task path: /home/austin/zoox/ansible-bug/playbook.yml:12
Using module file /usr/local/lib/python2.7/dist-packages/ansible/modules/core/packaging/os/apt.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: root
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo ~/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821 `" && echo ansible-tmp-1504235777.29-268289981866821="` echo ~/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821 `" ) && sleep 0'
<127.0.0.1> PUT /tmp/tmph21RuI TO /home/austin/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821/apt.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/austin/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821/ /home/austin/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821/apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python2 /home/austin/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821/apt.py; rm -rf "/home/austin/.ansible/tmp/ansible-tmp-1504235777.29-268289981866821/" > /dev/null 2>&1 && sleep 0'
ok: [localhost] => (item=[u'libslang2-dev']) => {
    "cache_update_time": 1504227095, 
    "cache_updated": false, 
    "changed": false, 
    "invocation": {
        "module_args": {
            "allow_unauthenticated": false, 
            "autoremove": false, 
            "cache_valid_time": 0, 
            "deb": null, 
            "default_release": null, 
            "dpkg_options": "force-confdef,force-confold", 
            "force": false, 
            "install_recommends": false, 
            "name": [
                "libslang2-dev"
            ], 
            "only_upgrade": false, 
            "package": [
                "libslang2-dev"
            ], 
            "purge": false, 
            "state": "present", 
            "update_cache": false, 
            "upgrade": null
        }, 
        "module_name": "apt"
    }, 
    "item": [
        "libslang2-dev"
    ]
}

TASK [uninstall_parent] ********************************************************
task path: /home/austin/zoox/ansible-bug/playbook.yml:19
Using module file /usr/local/lib/python2.7/dist-packages/ansible/modules/core/packaging/os/apt.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: root
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo ~/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301 `" && echo ansible-tmp-1504235778.26-183451794808301="` echo ~/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301 `" ) && sleep 0'
<127.0.0.1> PUT /tmp/tmpDHijbG TO /home/austin/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301/apt.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/austin/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301/ /home/austin/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301/apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python2 /home/austin/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301/apt.py; rm -rf "/home/austin/.ansible/tmp/ansible-tmp-1504235778.26-183451794808301/" > /dev/null 2>&1 && sleep 0'
changed: [localhost] => (item=[u'libcaca-dev']) => {
    "changed": true, 
    "diff": {}, 
    "invocation": {
        "module_args": {
            "allow_unauthenticated": false, 
            "autoremove": true, 
            "cache_valid_time": 0, 
            "deb": null, 
            "default_release": null, 
            "dpkg_options": "force-confdef,force-confold", 
            "force": false, 
            "install_recommends": false, 
            "name": [
                "libcaca-dev"
            ], 
            "only_upgrade": false, 
            "package": [
                "libcaca-dev"
            ], 
            "purge": true, 
            "state": "absent", 
            "update_cache": false, 
            "upgrade": null
        }, 
        "module_name": "apt"
    }, 
    "item": [
        "libcaca-dev"
    ], 
    "stderr": "", 
    "stdout": "Reading package lists...\nBuilding dependency tree...\nReading state information...\nThe following packages will be REMOVED:\n  libcaca-dev* libslang2-dev*\n0 upgraded, 0 newly installed, 2 to remove and 46 not upgraded.\nAfter this operation, 4397 kB disk space will be freed.\n(Reading database ... 277286 files and directories currently installed.)\nRemoving libcaca-dev (0.99.beta18-1ubuntu5) ...\nRemoving libslang2-dev:amd64 (2.2.4-15ubuntu1) ...\nProcessing triggers for man-db (2.6.7.1-1ubuntu1) ...\n", 
    "stdout_lines": [
        "Reading package lists...", 
        "Building dependency tree...", 
        "Reading state information...", 
        "The following packages will be REMOVED:", 
        "  libcaca-dev* libslang2-dev*", 
        "0 upgraded, 0 newly installed, 2 to remove and 46 not upgraded.", 
        "After this operation, 4397 kB disk space will be freed.", 
        "(Reading database ... 277286 files and directories currently installed.)", 
        "Removing libcaca-dev (0.99.beta18-1ubuntu5) ...", 
        "Removing libslang2-dev:amd64 (2.2.4-15ubuntu1) ...", 
        "Processing triggers for man-db (2.6.7.1-1ubuntu1) ..."
    ]
}

PLAY RECAP *********************************************************************
localhost                  : ok=4    changed=2    unreachable=0    failed=0   
```

Hints from issue discussion:
cc @mgwilliams
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
I have the same problem with ansible 2.4 on debian 9. Every package should be set to manually installed. This works for packages that are installed via ansible that weren't installed on the system yet. But if packages were installed before the ansible run as automatically-installed (as a dependency of a different packatge), it must be set to manually installed.

This behavior is dangerous if autoclean is used via ansible or manually. Packages can be removed that are needed. Of course they would be re-installed with the next ansible run and then correctly marked as auto-installed. But removal of a crucial package can lead to big problems. So i consider this dangerous bug that should be fixed fast.
I have the same bug in last time on ```Ubuntu 16.04.3 LTS``` server. I use ```unattended-upgrade``` with autoclean. Sometime ago I installed manually ```python-pip``` (which required ```python-dev```). Next to I mark by Ansible to install ```python-dev``` (no changes). Next to upgrade ```python-pip``` package( which changes ```python-dev``` to recommends), so autoclean remove ```python-dev``` non-required package. 

```python-dev``` package should be marked as manually installed so that it will not be removed automatically.

Commit message: apt: mark installed packages manual

Patch to review:
diff --git a/lib/ansible/modules/packaging/os/apt.py b/lib/ansible/modules/packaging/os/apt.py
index 7452dd6e7849da..80274180360fb6 100644
--- a/lib/ansible/modules/packaging/os/apt.py
+++ b/lib/ansible/modules/packaging/os/apt.py
@@ -477,6 +477,7 @@ def install(m, pkgspec, cache, upgrade=False, default_release=None,
     pkg_list = []
     packages = ""
     pkgspec = expand_pkgspec_from_fnmatches(m, pkgspec, cache)
+    package_names = []
     for package in pkgspec:
         if build_dep:
             # Let apt decide what to install
@@ -484,6 +485,7 @@ def install(m, pkgspec, cache, upgrade=False, default_release=None,
             continue
 
         name, version = package_split(package)
+        package_names.append(name)
         installed, upgradable, has_files = package_status(m, name, version, cache, state='install')
         if (not installed and not only_upgrade) or (upgrade and upgradable):
             pkg_list.append("'%s'" % package)
@@ -545,9 +547,22 @@ def install(m, pkgspec, cache, upgrade=False, default_release=None,
         if rc:
             status = False
             data = dict(msg="'%s' failed: %s" % (cmd, err), stdout=out, stderr=err, rc=rc)
-        return (status, data)
     else:
-        return (True, dict(changed=False))
+        status = True
+        data = dict(changed=False)
+
+    if not build_dep:
+        mark_installed_manually(m, package_names)
+
+    return (status, data)
+
+
+def mark_installed_manually(m, packages):
+    apt_mark_cmd_path = m.get_bin_path("apt-mark", required=True)
+    cmd = "%s manual %s" % (apt_mark_cmd_path, ' '.join(packages))
+    rc, out, err = m.run_command(cmd)
+    if rc != 0:
+        m.fail_json(msg="'%s' failed: %s" % (cmd, err), stdout=out, stderr=err, rc=rc)
 
 
 def get_field_of_deb(m, deb_file, field="Version"):
diff --git a/test/integration/targets/apt/tasks/apt.yml b/test/integration/targets/apt/tasks/apt.yml
index 22ecd608b97bff..4d01cead40ea23 100644
--- a/test/integration/targets/apt/tasks/apt.yml
+++ b/test/integration/targets/apt/tasks/apt.yml
@@ -185,6 +185,41 @@
     name: /etc/apt/sources.list.d/non-existing.list
     state: absent
 
+# https://github.com/ansible/ansible/issues/28907
+- name: Install parent package
+  apt:
+    name: libcaca-dev
+
+- name: Install child package
+  apt:
+    name: libslang2-dev
+
+- shell: apt-mark showmanual | grep libcaca-dev
+  ignore_errors: yes
+  register: parent_output
+
+- name: Check that parent package is marked as installed manually
+  assert:
+    that:
+      - "'libcaca-dev' in parent_output.stdout"
+
+- shell: apt-mark showmanual | grep libslang2-dev
+  ignore_errors: yes
+  register: child_output
+
+- name: Check that child package is marked as installed manually
+  assert:
+    that:
+      - "'libslang2-dev' in child_output.stdout"
+
+- name: Clean up
+  apt:
+    name: "{{ item }}"
+    state: absent
+  with_items:
+    - libcaca-dev
+    - libslang2-dev
+
 - name: autoclean during install
   apt: pkg=hello state=present autoclean=yes
 


Code review comments:
diff_hunk: @@ -545,9 +547,22 @@ def install(m, pkgspec, cache, upgrade=False, default_release=None,
         if rc:
             status = False
             data = dict(msg="'%s' failed: %s" % (cmd, err), stdout=out, stderr=err, rc=rc)
-        return (status, data)
     else:
-        return (True, dict(changed=False))
+        status = True
+        data = dict(changed=False)
+
+    if not build_dep:
+        mark_installed_manually(m, package_names)
+
+    return (status, data)
+
+
+def mark_installed_manually(m, packages):
+    apt_mark_cmd_path = m.get_bin_path("apt-mark", required=True)
+    cmd = "%s manual %s" % (apt_mark_cmd_path, ' '.join(packages))
path: lib/ansible/modules/packaging/os/apt.py
text: @user1:
"manual" is a recent occurrence, for older versions it was 'unmarkauto' iirc

@author:
Thanks, added a fallback for that case.