Skip to content

apt module, fix issue with packages not installing when pinned and cached #81364

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

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

Conversation

jhooda
Copy link

@jhooda jhooda commented Jul 28, 2023

SUMMARY

Currently when apt cache holds multiple versions of the same package, including newer versions and older versions, a specific installation of the package version (other than the latest version) fails with an error message:

no available installation candidate for pkgname=x.y.z

However, the command line such as

$ apt install pkgname=x.y.z

works just fine, the reason being some issue in apt_cache.Policy which fails to override the default priority.

When set_priority() is called before create_pin then it works as expected.

fixes #80813

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

modules/apt

@ansibot ansibot added feature This issue/PR relates to a feature request. test This PR relates to tests. needs_triage Needs a first human triage before being processed. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 28, 2023
@ansibot

This comment was marked as outdated.

@bcoca bcoca changed the title fix for issue 80813 apt module, fix issue with packages not installing when pinned and cached Jul 28, 2023
Copy link
Member

@bcoca bcoca left a comment

Choose a reason for hiding this comment

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

needs changelog and tests

@ansibot ansibot added the stale_review Updates were made after the last review and the last review is more than 7 days old. label Jul 28, 2023
@ansibot

This comment was marked as outdated.

@jhooda
Copy link
Author

jhooda commented Jul 29, 2023

@bcoca I need some help regarding the unit tests for this issue. For the unit test, I need multiple versions of a test debian package (e.g., https://ci-files.testing.ansible.com/test/integration/targets/dpkg_selections/hello_2.6-1_amd64.deb). Since, I don't have access to https://ci-files.testing.ansible.com I am not able to see what's available there. Can you please provide URLs for some test debian package that has at least three versions and which will not be wiped out in future. Thanks.

@bcoca
Copy link
Member

bcoca commented Jul 31, 2023

no, we have none, i would recommend adding them as part of the test .. also this is something i would expect in an integration test, not a unit one.

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Aug 1, 2023
@bcoca
Copy link
Member

bcoca commented Aug 1, 2023

cc @mattclay

@ansibot ansibot added the bug This issue/PR relates to a bug. label Aug 1, 2023
@bcoca
Copy link
Member

bcoca commented Aug 1, 2023

The apt tests use a setup_deb_repo which creates packages for foo and foobar and automatically sets up the repo so they can be installed from with these versions: foo-1.0.0 foo-1.0.1 foobar-1.0.0 foobar-1.0.1

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 4, 2023
@jhooda
Copy link
Author

jhooda commented Aug 4, 2023

@bcoca I've tested foobar pkg (even with one additional version foobar-1.0.2) and that worked okay even without this pull request. So not sure how to reproduce the issue using foobar. I've added some tests to this pull request. My guess is the issue has something to do with non-std pkg versions such as '1.9.0-71-g9c1898a'. I'll try some more tests later.

@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 4, 2023
@jhooda
Copy link
Author

jhooda commented Aug 4, 2023

Also to explain the underlying issue, see lines:265-268 in
https://github.com/Debian/apt/blob/aa56836331870d975c212a5df2f13db9ce3914bf/apt-pkg/policy.cc#L265-L268

	    for (pkgCache::VerIterator Ver = Pkg.VersionList(); Ver.end() != true; ++Ver)
	    {
	       if (Match.VersionMatches(Ver)) {
		  Pin *VP = VerPins + Ver->ID;
		  if (VP->Type == pkgVersionMatch::None) {
		     *VP = P;
		      matched = true;
		  }
	       }
	    }

The priority pins are only updated if VP-Type is None, which in this case is 'Version'. When we manually invoke setPriority, it sets the pin Type to None, and which is why it works (lines:363-369)

void pkgPolicy::SetPriority(pkgCache::VerIterator const &Ver, signed short Priority)
{
   Pin pin;
   pin.Data = "pkgPolicy::SetPriority";
   pin.Priority = Priority;
   VerPins[Ver->ID] = pin;
}

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 7, 2023
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 9, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 9, 2023
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 11, 2023
@jhooda jhooda requested a review from bcoca August 11, 2023 17:11
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 11, 2023
@webknjaz
Copy link
Member

@jhooda the CI failures look related to your patch: https://dev.azure.com/ansible/ansible/_build/results?buildId=86661&view=logs&j=26a6818c-7a55-5301-191d-87f9382f47e1&t=b498074a-badf-5058-76b3-ae0756883986&l=2586 — you need to fix them for the PR to be mergable.

@jhooda
Copy link
Author

jhooda commented Aug 14, 2023

@webknjaz I reviewed the logs. They seem unrelated to my changes: my tests are further down from the failure point and the failure is in an unrelated "apt cache update" feature. I can try rebasing the pull request with the latest from the devel branch. Please suggest the way forward. Just for records "ansible [core 2.12.10]" does not suffer from this issue.

@webknjaz
Copy link
Member

my tests are further down from the failure point and the failure is in an unrelated "apt cache update" feature.

This effectively means that your patch has side effects beyond what you expected, and it so happens that it breaks existing tests. Before this patch, policy.get_candidate_ver(pkg) was always called for = and now the control flow never reaches it in this case. This is a behavior change, and it does change how cache is used.
The failing test — https://github.com/ansible/ansible/blob/bd3ffbe/test/integration/targets/apt/tasks/apt.yml#L99-L104 — is a cache test, no wonder it got affected.

If you're sure the patch is correct, the changes necessary might be in the tests, not in the module.

Currently when apt cache holds multiple versions of the same package, including
newer versions and older versions, a specific installation of the package
version (other than the latest version) fails with an error message:

no available installation candidate for <pkgname>=x.y.z

However, the command line such as

$ apt install <pkgname>=x.y.z

works just fine, the reason being some issue in apt_cache.Policy which fails to override the default priority.

When set_priority() is called before create_pin then it works as expected.

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
@jhooda
Copy link
Author

jhooda commented Aug 21, 2023

@webknjaz thanks for the suggestions. I have slightly changed apt.py and squashed commits too. Let's see if this helps. Thanks.

@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 21, 2023
@jhooda
Copy link
Author

jhooda commented Aug 21, 2023

@webknjaz I ran the test on my local Ubuntu22.04 LTS (devel branch - with last commit: f10d11b) and it failed exactly at the same place, so something else is causing the failure. The test which I ran

$ cat /etc/issue
Ubuntu 22.04 LTS

$ python3 ./bin/ansible-test integration --allow-destructive apt
...
TASK [apt : verify update_cache] ***********************************************
fatal: [testhost]: FAILED! => {
    "assertion": "apt_update_cache_1 is changed",
    "changed": false,
    "evaluated_to": false,
    "msg": "Assertion failed"
}

@webknjaz
Copy link
Member

I ran the test on my local Ubuntu22.04 LTS

Try running with --docker to reproduce the exact CI env.

@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Aug 21, 2023
pkgver = next(v1 for v1 in pkg.version_list if v1.ver_str == version)
policy.set_priority(pkgver, 0)
except StopIteration:
pass
policy.create_pin('Version', pkgname, version, 1001)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this call modifies cache… Or maybe set_priority() does?

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 4, 2023
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Sep 9, 2023
@webknjaz webknjaz added the ci_verified Changes made in this PR are causing tests to fail. label Sep 11, 2023
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 18, 2023
@webknjaz
Copy link
Member

@jhooda could you rebase the PR branch, just in case the recent Launchpad API problems were related to the failure?

@Akasurde
Copy link
Member

@jhooda Could you please rebase this branch? Thanks in advance.

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jul 31, 2024
@timon-michel-scopevisio

@jhooda Any updates on the rebase? I just came across this issue while installing nodejs using nodesource, as it pins the repository origin:

Package: nodejs
Pin: origin deb.nodesource.com
Pin-Priority: 600

@ansibot ansibot added the stale_pr This PR has not been pushed to for more than one year. label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug. ci_verified Changes made in this PR are causing tests to fail. feature This issue/PR relates to a feature request. has_issue module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_pr This PR has not been pushed to for more than one year. stale_review Updates were made after the last review and the last review is more than 7 days old. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the apt module fails to install a package in some cases
6 participants