Repository: ansible/ansible
Pull request: Fix apt making changes in check mode
##### SUMMARY
Fixes https://github.com/ansible/ansible/issues/66413
Supersedes https://github.com/ansible/ansible/pull/45575

Don't update the cache or mark packages as manually installed in check mode

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
lib/ansible/modules/apt.py


Problem statement:
apt module touches package states when run in check mode
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
<!--- Explain the problem briefly below -->
When running the apt module in check mode it set packages to manual installed that where set on auto installed before. This shouldn't happen in check mode!

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below, use your best guess if unsure -->
apt module
##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
ansible 2.7.7
  config file = /home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg
  configured module search path =['/home/jo-jo/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.7.3 (default, Apr  3 2019, 05:39:12) [GCC 8.3.0]

```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```paste below

 ansible-config dump --only-changed
CACHE_PLUGIN(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= jsonfile
CACHE_PLUGIN_CONNECTION(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= facts
CACHE_PLUGIN_TIMEOUT(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= 300
DEFAULT_FORKS(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= 20
DEFAULT_GATHERING(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= smart
DEFAULT_JINJA2_EXTENSIONS(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= jinja2.ext.do
DEFAULT_VAULT_PASSWORD_FILE(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= /home/jo-jo/tmp/repos/ansible-checkout/trunk/secrets/ansible-vault.sh
DISPLAY_SKIPPED_HOSTS(/home/jo-jo/tmp/repos/ansible-checkout/trunk/ansible.cfg)
= False
```

##### OS / ENVIRONMENT
<!--- Provide all relevant information below, e.g. target OS versions, network device firmware, etc. -->
Source and target OS: Debian 10

##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->
Build a playbook with an apt install task. Install the packages in this task and mark them auto installed (by hand). Then run the playbook in check mode.

##### EXPECTED RESULTS
<!--- Describe what you expected to happen when running the steps above -->
The state of the packages don't get changed.

##### ACTUAL RESULTS
<!--- Describe what actually happened. If possible run with extra verbosity (-vvvv) -->
The module changes the package state.
<!--- Paste verbatim command output between quotes -->
```paste below

```

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/packaging/os/apt.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/packaging/os/apt.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @mgwilliams
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
I just searched the repo a bit and found this pull request which fixes my problem:
https://github.com/ansible/ansible/pull/45575
Could you please consider merging this?
Can you please show the commands you use to verify package state and how it indicates it is changed?

needs_info
I use apt show $package to verify the package state. The 'APT-Manual-Installed' Flag changes by running the described actions.
The flag switches from no to yes after the playbook run in check mode.
Files identified in the description:
* [`lib/ansible/modules/apt.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/apt.py)

If these files are incorrect, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->

Commit message: changelog

Patch to review:
diff --git a/changelogs/fragments/78496-fix-apt-check-mode.yml b/changelogs/fragments/78496-fix-apt-check-mode.yml
new file mode 100644
index 00000000000000..25d6ae84666f19
--- /dev/null
+++ b/changelogs/fragments/78496-fix-apt-check-mode.yml
@@ -0,0 +1,3 @@
+bugfixes:
+  - apt - don't actually update the cache in check mode with update_cache=true.
+  - apt - don't mark existing packages as manually installed in check mode (https://github.com/ansible/ansible/issues/66413).
diff --git a/lib/ansible/modules/apt.py b/lib/ansible/modules/apt.py
index 0d6c3352926b5f..b254fb031e1198 100644
--- a/lib/ansible/modules/apt.py
+++ b/lib/ansible/modules/apt.py
@@ -809,7 +809,7 @@ def install(m, pkgspec, cache, upgrade=False, default_release=None,
         status = True
         data = dict(changed=False)
 
-    if not build_dep:
+    if not build_dep and not m.check_mode:
         mark_installed_manually(m, package_names)
 
     return (status, data)
@@ -1332,7 +1332,8 @@ def main():
 
                     for retry in range(update_cache_retries):
                         try:
-                            cache.update()
+                            if not module.check_mode:
+                                cache.update()
                             break
                         except apt.cache.FetchFailedException as e:
                             err = to_native(e)
@@ -1347,7 +1348,7 @@ def main():
 
                     cache.open(progress=None)
                     mtimestamp, post_cache_update_time = get_updated_cache_time()
-                    if updated_cache_time != post_cache_update_time:
+                    if module.check_mode or updated_cache_time != post_cache_update_time:
                         updated_cache = True
                     updated_cache_time = post_cache_update_time
 
diff --git a/test/integration/targets/apt/tasks/apt.yml b/test/integration/targets/apt/tasks/apt.yml
index 5b1a24a3b4fc95..1e9fd7c3ae7045 100644
--- a/test/integration/targets/apt/tasks/apt.yml
+++ b/test/integration/targets/apt/tasks/apt.yml
@@ -83,12 +83,26 @@
       - apt_install_fnmatch is changed
       - apt_uninstall_fnmatch is changed
 
+- name: Test update_cache 1 (check mode)
+  apt:
+    update_cache: true
+    cache_valid_time: 10
+  register: apt_update_cache_1_check_mode
+  check_mode: true
+
 - name: Test update_cache 1
   apt:
     update_cache: true
     cache_valid_time: 10
   register: apt_update_cache_1
 
+- name: Test update_cache 2 (check mode)
+  apt:
+    update_cache: true
+    cache_valid_time: 10
+  register: apt_update_cache_2_check_mode
+  check_mode: true
+
 - name: Test update_cache 2
   apt:
     update_cache: true
@@ -98,7 +112,9 @@
 - name: verify update_cache
   assert:
     that:
+      - apt_update_cache_1_check_mode is change
       - apt_update_cache_1 is changed
+      - apt_update_cache_2_check_mode is not changed
       - apt_update_cache_2 is not changed
 
 - name: uninstall apt bindings with apt again
diff --git a/test/integration/targets/apt/tasks/repo.yml b/test/integration/targets/apt/tasks/repo.yml
index aa90109b69cf44..85f7a961dcdf94 100644
--- a/test/integration/targets/apt/tasks/repo.yml
+++ b/test/integration/targets/apt/tasks/repo.yml
@@ -196,6 +196,42 @@
       name: foobar=1.0.0
       allow_unauthenticated: yes
 
+  - name: mark foobar as auto for next test
+    shell: sudo apt-mark auto foobar
+
+  - name: Install foobar (marked as manual) (check mode)
+    apt:
+      name: foobar=1.0.1
+      allow_unauthenticated: yes
+    check_mode: yes
+    register: manual_foobar_install_check_mode
+
+  - name: check foobar was not marked as manually installed by check mode
+    shell: apt-mark showmanual | grep foobar
+    ignore_errors: yes
+    register: showmanual
+
+  - assert:
+      that:
+        - manual_foobar_install_check_mode.changed
+        - "'foobar' not in showmanual.stdout"
+
+  - name: Install foobar (marked as manual)
+    apt:
+      name: foobar=1.0.1
+      allow_unauthenticated: yes
+    register: manual_foobar_install
+
+  - name: check foobar was marked as manually installed
+    shell: apt-mark showmanual | grep foobar
+    ignore_errors: yes
+    register: showmanual
+
+  - assert:
+      that:
+        - manual_foobar_install.changed
+        - "'foobar' in showmanual.stdout"
+
   - name: Upgrade foobar to a version which does not depend on foo, autoremove should remove foo
     apt:
       upgrade: dist


Code review comments:
diff_hunk: @@ -98,7 +112,9 @@
 - name: verify update_cache
   assert:
     that:
+      - apt_update_cache_1_check_mode is change
path: test/integration/targets/apt/tasks/apt.yml
text: @author:
```suggestion
      - apt_update_cache_1_check_mode is changed
```