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

WIP: Add download_only option to the apt module #70889

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

Conversation

Taoquitok
Copy link
Contributor

@Taoquitok Taoquitok commented Jul 25, 2020

WIP to complete
  • Fix perpetual changed: true state. Once file is dowloaded, stop reporting changed.
  • Tests to validate above
SUMMARY

Extend apt module to add the apt/aptitude flag --download-only to allow for downloading packages without installing

Fixes #23220

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • apt.py
ADDITIONAL INFORMATION

#23220 is a long standing feature request for a useful edge case. Adding the --download-only flag will allow for users to download packages ahead of time.

Duplicate of closed #61500. I've refreshed pull request in hopes to correct labels and increase visibility

BEFORE
---
- name: Install hello
  apt: 
    name: hello
    state: present
TASK [Install hello] ******************************
changed: [testhost] => {"cache_update_time": 1590261291, "cache_updated": false, "changed": true, "stderr": "", "stderr_lines": [], "stdout": "Reading package lists...
Building dependency tree...
Reading state information...
The following NEW packages will be installed:
  hello
0 upgraded, 1 newly installed, 0 to remove and 76 not upgraded.
AFTER
- name: download hello without installing
  apt: 
    name: hello
    state: present
    download_only: true
TASK [download hello without installing] ******************************
changed: [testhost] => {"cache_update_time": 1590261291, "cache_updated": false, "changed": true, "stderr": "", "stderr_lines": [], "stdout": "Reading package lists...
Building dependency tree...
Reading state information...
The following NEW packages will be installed:
  hello
0 upgraded, 1 newly installed, 0 to remove and 76 not upgraded.
Need to get 27.4 kB of archives.
After this operation, 106 kB of additional disk space will be used.
Get:1 http://archive.ubuntu.com/ubuntu bionic-updates/main amd64 hello amd64 2.10-1build3 [27.4 kB]
Fetched 27.4 kB in 0s (108 kB/s)
Download complete and in download only mode

@Taoquitok Taoquitok force-pushed the Taoquitok-apt-download-only branch from 9551ef9 to 6534b96 Compare Jul 25, 2020
@ansibot ansibot added affects_2.11 core_review feature module needs_triage packaging support:community support:core labels Jul 25, 2020
@samdoran samdoran removed the needs_triage label Jul 28, 2020
@Taoquitok Taoquitok force-pushed the Taoquitok-apt-download-only branch from 6534b96 to 419a79b Compare Aug 1, 2020
@Taoquitok Taoquitok force-pushed the Taoquitok-apt-download-only branch from 419a79b to 151969a Compare Aug 8, 2020
@ansibot ansibot added the needs_ci label Aug 10, 2020
@Taoquitok Taoquitok closed this Aug 12, 2020
@Taoquitok Taoquitok reopened this Aug 12, 2020
@ansibot ansibot removed the needs_ci label Aug 12, 2020
@Taoquitok Taoquitok force-pushed the Taoquitok-apt-download-only branch from 151969a to e8bcc95 Compare Aug 16, 2020
@Taoquitok
Copy link
Contributor Author

Taoquitok commented Aug 16, 2020

ready_for_review

@ansibot ansibot added the stale_ci label Aug 24, 2020
@paltaa
Copy link

paltaa commented Aug 24, 2020

Is there any way to use this feature without having to wait for the PR to be merged?

@Taoquitok
Copy link
Contributor Author

Taoquitok commented Aug 24, 2020

Is there any way to use this feature without having to wait for the PR to be merged?

Hey @paltaa,
Yeah you can copy the apt.py file into your project and place it under a library folder in the top level of any play project that needs it
So something like: /your_play/Library/apt.py

Ansible-playbook will automatically pick up the apt_module under there.
fyi, the version linked is for the bleeding edge devel It should work, but If you need this for a specific version do say and I'll chuck up a copy for you

@paltaa
Copy link

paltaa commented Aug 24, 2020

Great @Taoquitok, thank you very much for the quick reply, will try this and report back

@paltaa
Copy link

paltaa commented Aug 24, 2020

Worked like a charm in /library/apt.py, thanks! Also, is there a way to easily specify where to download the .deb?

@Taoquitok
Copy link
Contributor Author

Taoquitok commented Aug 24, 2020

Worked like a charm in /library/apt.py, thanks! Also, is there a way to easily specify where to download the .deb?

It'll go to the default apt cache location /var/cache/apt/archives/.
I think you can override that via the apt properties, but I wouldn't suggest it as apt uses that for everything.
Rather it's best to move/copy the file after downloading

@Taoquitok Taoquitok force-pushed the Taoquitok-apt-download-only branch from e8bcc95 to c339ab5 Compare Aug 25, 2020
@ansibot ansibot removed the stale_ci label Aug 25, 2020
@Taoquitok Taoquitok force-pushed the Taoquitok-apt-download-only branch from c339ab5 to 9bb8e7d Compare Aug 31, 2020
@ansibot ansibot added needs_revision and removed core_review labels Aug 31, 2020
@Taoquitok Taoquitok closed this Aug 31, 2020
@Taoquitok Taoquitok reopened this Aug 31, 2020
@ansibot ansibot added core_review and removed needs_revision labels Aug 31, 2020
@Taoquitok
Copy link
Contributor Author

Taoquitok commented Aug 31, 2020

bot_status

@sivel
Copy link
Member

sivel commented Aug 31, 2020

This doesn't appear to properly handle change detection, in my tests it always reports changed: true

Change detection is handled on whether or not these strings appear in the output:

APT_GET_ZERO = "\n0 upgraded, 0 newly installed"
APTITUDE_ZERO = "\n0 packages upgraded, 0 newly installed"

With --download-only it is always reporting:

0 upgraded, 19 newly installed, 0 to remove and 8 not upgraded.

or

0 packages upgraded, 18 newly installed, 0 to remove and 8 not upgraded.

I tested this using vim, with both apt-get and aptitude.

As such, it looks like work will have to be made to validate whether this caused the module to actually need to download something so that it's not reporting a change regardless of what is happening.

@ansibot
Copy link
Contributor

ansibot commented Aug 31, 2020

Components

changelogs/fragments/61500-apt-download_only.yml
support: community
maintainers:

lib/ansible/modules/apt.py
support: core
maintainers: mgwilliams

test/integration/targets/apt/tasks/apt-builddep.yml
support: core
maintainers: mgwilliams

test/integration/targets/apt/tasks/apt-multiarch.yml
support: core
maintainers: mgwilliams

test/integration/targets/apt/tasks/apt.yml
support: core
maintainers: mgwilliams

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 0
shipit_actors (maintainer or core team member): []
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

@Taoquitok

This comment has been minimized.

@Taoquitok

This comment has been minimized.

@Taoquitok

This comment has been minimized.

@sivel
Copy link
Member

sivel commented Aug 31, 2020

Ignore check_mode for the time being.

If this option cannot properly report change detection during an actual live execution, why does it need to be part of the module?

I can only see this being accepted if the module can properly report whether something was actually downloaded or not. Otherwise, just use command: apt-get install --download-only whatever since it will always report changed: true.

Basically here is what I am talking about:

- name: download hello without installing
  apt: 
    name: hello
    state: present
    download_only: true

- name: download hello without installing, this should not produce a change
  apt: 
    name: hello
    state: present
    download_only: true
  register: second
    
- assert:
    that:
      - second is not changed

The first time these tasks are run, the first should set changed: true as it just downloaded the files. The 2nd time it should not, and should produce changed: false. As it stands, no matter how many times you run the task with this PR, it will report that the module made a change, regardless if it actually downloaded anything.

You will need to make adjustments to the module for the above to actually happen, otherwise there is no reason to accept this PR.

@sivel
Copy link
Member

sivel commented Aug 31, 2020

Also, what I was trying to indicate in my previous comment, was that no matter how many times you run apt-get with --download-only, it always reports N newly installed regardless of whether it needed to download packages or not.

That is the problem. The change detection expects it to say 0 newly installed. That needs to be accommodated in this pull request, otherwise the module always reports a change, no matter if packages were downloaded or now.

@Taoquitok
Copy link
Contributor Author

Taoquitok commented Aug 31, 2020

@sivel

Ah, I see what you mean. Never crossed my mind that that'd be an issue, but I do see why.

Thankfully the file on disk doesn't change even though apt-get/aptitude outputs that, so, I could add in some logic to compare lists of files (the list / hash / whatever behaviour I see pre-existing in the code base) matching the name before and after running the download and use that to dictate the changed state, plus tests to validate the behaviour

@Taoquitok Taoquitok changed the title RFR: Add download_only option to the apt module WIP: Add download_only option to the apt module Aug 31, 2020
@ansibot ansibot added WIP and removed core_review labels Aug 31, 2020
@ansibot ansibot added the stale_ci label Sep 9, 2020
@ansibot ansibot added pre_azp and removed stale_ci labels Dec 6, 2020
@ansibot ansibot added the needs_rebase label Jan 15, 2021
@ansibot ansibot removed the support:community label Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.11 feature module needs_rebase packaging pre_azp support:core WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants