Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix apt delete package by wildcard if it is not found #70333

Open
wants to merge 5 commits into
base: devel
Choose a base branch
from

Conversation

nickgryg
Copy link
Contributor

@nickgryg nickgryg commented Jun 26, 2020

SUMMARY

When we install/delete package(s) by wildcard using apt module, we always use function expand_pkgspec_from_fnmatches() to find the exact package(s) name(s). The thing is that we match our wildcard name only among packages in apt cache. That's totally correct when we install packages, because we need to be sure that we can install them in the target OS.
But it's not a case when we are trying to remove package(s) from the target system (state: 'absent'). We shouldn't care if we didn't find package(s) for removal in apt cache. The only explanation that there is(are) no package(s) installed in the target system which is(are) matched by wildcard. If so, we don't need to remove them. Ansible shouldn't throw an error in this case.

Fixes #62262

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

apt

ADDITIONAL INFORMATION

We have a simple playbook.yaml:

---
- hosts: localhost
  connection: local
  gather_facts: True
  tasks:
    - apt:
         name: "test-test-test*"
         state: absent
         autoremove: yes

Before this PR:

TASK [apt] *****************************************************************************************************************************************************************************************************
task path: /ansible/playbook.yaml:6
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: root
<127.0.0.1> EXEC /bin/sh -c 'echo ~root && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /root/.ansible/tmp `"&& mkdir /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949 && echo ansible-tmp-1593194564.1-9044-82833657605949="` echo /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949 `" ) && sleep 0'
Using module file /ansible/lib/ansible/modules/apt.py
<127.0.0.1> PUT /root/.ansible/tmp/ansible-local-8994OqAboD/tmpF1KRHS TO /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949/AnsiballZ_apt.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949/ /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949/AnsiballZ_apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949/AnsiballZ_apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /root/.ansible/tmp/ansible-tmp-1593194564.1-9044-82833657605949/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
WARNING: The below traceback may *not* be related to the actual failure.
  File "/tmp/ansible_apt_payload_FIkYeG/ansible_apt_payload.zip/ansible/modules/apt.py", line 547, in expand_pkgspec_from_fnmatches
fatal: [localhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "allow_unauthenticated": false,
            "autoclean": false,
            "autoremove": true,
            "cache_valid_time": 0,
            "deb": null,
            "default_release": null,
            "dpkg_options": "force-confdef,force-confold",
            "force": false,
            "force_apt_get": false,
            "install_recommends": null,
            "name": "test-test-test*",
            "only_upgrade": false,
            "package": [
                "test-test-test*"
            ],
            "policy_rc_d": null,
            "purge": false,
            "state": "absent",
            "update_cache": null,
            "update_cache_retries": 5,
            "update_cache_retry_max_delay": 12,
            "upgrade": null
        }
    },
    "msg": "No package(s) matching 'test-test-test*' available"
}

After this PR:

TASK [apt] *****************************************************************************************************************************************************************************************************
task path: /root/ansible/playbook.yaml:6
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: root
<127.0.0.1> EXEC /bin/sh -c 'echo ~root && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /root/.ansible/tmp `"&& mkdir /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749 && echo ansible-tmp-1593537430.2411246-5847-78438765107749="` echo /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749 `" ) && sleep 0'
Using module file /root/ansible/lib/ansible/modules/apt.py
<127.0.0.1> PUT /root/.ansible/tmp/ansible-local-5798y68sy6pa/tmp0v22nj5r TO /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749/AnsiballZ_apt.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749/ /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749/AnsiballZ_apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749/AnsiballZ_apt.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /root/.ansible/tmp/ansible-tmp-1593537430.2411246-5847-78438765107749/ > /dev/null 2>&1 && sleep 0'
[WARNING]: No package(s) matching 'test-test-test*' to delete. It might be the name(s) was(were) misspelled.
ok: [localhost] => {
    "changed": false,
    "invocation": {
        "module_args": {
            "allow_unauthenticated": false,
            "autoclean": false,
            "autoremove": true,
            "cache_valid_time": 0,
            "deb": null,
            "default_release": null,
            "dpkg_options": "force-confdef,force-confold",
            "force": false,
            "force_apt_get": false,
            "install_recommends": null,
            "name": "test-test-test*",
            "only_upgrade": false,
            "package": [
                "test-test-test*"
            ],
            "policy_rc_d": null,
            "purge": false,
            "state": "absent",
            "update_cache": null,
            "update_cache_retries": 5,
            "update_cache_retry_max_delay": 12,
            "upgrade": null
        }
    }
}
META: ran handlers
META: ran handlers

@ansibot ansibot added affects_2.11 bug core_review module needs_triage new_contributor packaging support:core traceback labels Jun 26, 2020
nickgryg added a commit to nickgryg/ansible that referenced this issue Jun 26, 2020
@ansibot ansibot added the support:community label Jun 26, 2020
@samdoran samdoran removed the needs_triage label Jun 30, 2020
lib/ansible/modules/apt.py Outdated Show resolved Hide resolved
@ansibot ansibot added needs_revision core_review and removed core_review new_contributor needs_revision labels Jun 30, 2020
@nickgryg nickgryg requested a review from bcoca Jun 30, 2020
bcoca
bcoca approved these changes Jul 1, 2020
lib/ansible/modules/apt.py Outdated Show resolved Hide resolved
@ansibot ansibot added core_review and removed needs_revision labels Jul 2, 2020
@nickgryg nickgryg requested a review from bcoca Jul 2, 2020
@ansibot ansibot added needs_revision and removed core_review labels Jul 2, 2020
@ansibot ansibot added stale_ci stale_review labels Jul 10, 2020
@ansibot ansibot added the needs_revision label Apr 25, 2021
@ansibot ansibot added stale_ci stale_review labels May 3, 2021
@ansibot ansibot added needs_rebase and removed stale_review labels Mar 2, 2022
Copy link
Contributor

@s-hertel s-hertel left a comment

Can you rebase this? Looks good to me besides the parameter name.

@@ -563,7 +563,7 @@ def expand_dpkg_options(dpkg_options_compressed):
return dpkg_options.strip()


def expand_pkgspec_from_fnmatches(m, pkgspec, cache):
def expand_pkgspec_from_fnmatches(m, pkgspec, cache, to_rmv=False):
Copy link
Contributor

@s-hertel s-hertel Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def expand_pkgspec_from_fnmatches(m, pkgspec, cache, to_rmv=False):
def expand_pkgspec_from_fnmatches(m, pkgspec, cache, remove=False):

Copy link
Contributor Author

@nickgryg nickgryg Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s-hertel Thank you for the review! I've changed the name of internal variable and rebased my branch.

@ansibot ansibot added core_review and removed needs_rebase needs_revision stale_ci labels Aug 3, 2022
@ansibot
Copy link
Contributor

ansibot commented Aug 3, 2022

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/apt.py:620:82: undefined-variable: Undefined variable 'to_text'
lib/ansible/modules/apt.py:623:33: undefined-variable: Undefined variable 'to_text'

click here for bot help

@ansibot ansibot added ci_verified needs_revision and removed core_review labels Aug 3, 2022
@ansibot ansibot added core_review and removed ci_verified needs_revision labels Aug 3, 2022
@nickgryg nickgryg requested a review from s-hertel Aug 3, 2022
@ansibot ansibot added needs_revision and removed core_review labels Aug 3, 2022
@ansibot ansibot added stale_ci stale_review labels Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.11 bug module needs_revision packaging stale_ci stale_review support:core traceback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants