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

modules/dnf: Substitute variables in DNF cache path #80094

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

BenoitKnecht
Copy link
Contributor

SUMMARY

The cache directory can be specified with variables that are expanded by DNF, e.g.

cachedir=/var/cache/yum/$basearch/$releasever

But the dnf module would use that path literally, instead of replacing $basearch and $releasever with their values.

This commit ensures that variables in cachedir are properly substituted.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

dnf

ADDITIONAL INFORMATION

To reproduce the issue, set

[main]
cachedir=/var/cache/yum/$basearch/$releasever

in /etc/dnf/dnf.conf and use the dnf module to install a package. On the target system, Ansible will create a directory called /var/cache/yum/$basearch/$releasever, instead of /var/cache/yum/x86_64/8 for instance.


@ansibot ansibot added affects_2.15 bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. small_patch labels Feb 27, 2023
@ansibot

This comment was marked as outdated.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Feb 27, 2023
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. small_patch labels Feb 27, 2023
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 27, 2023
@mattclay mattclay added ci_verified Changes made in this PR are causing tests to fail. and removed needs_triage Needs a first human triage before being processed. labels Feb 27, 2023
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 28, 2023
@BenoitKnecht

This comment was marked as outdated.

@ansibot ansibot added test This PR relates to tests. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 28, 2023
@BenoitKnecht

This comment was marked as outdated.

@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 Mar 8, 2023
@BenoitKnecht BenoitKnecht requested a review from sivel March 15, 2023 08:29
Copy link
Contributor

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

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

For the changes here we will need a changelog fragment for this PR which is documented here https://docs.ansible.com/ansible/latest/community/development_process.html#creating-changelog-fragments. We would also look at adding some integration tests for this scenario under https://github.com/ansible/ansible/tree/devel/test/integration/targets/dnf. The integration tests might be a bit harder to setup but we can certainly work through them if you have any questions.

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 26, 2023
The cache directory can be specified with variables that are expanded by DNF,
e.g.

```
cachedir=/var/cache/yum/$basearch/$releasever
```

But the `dnf` module would use that path literally, instead of replacing
`$basearch` and `$releasever` with their values.

This commit ensures that variables in `cachedir` are properly substituted.

Signed-off-by: Benoît Knecht <bknecht@protonmail.ch>
Now that the `dnf` module uses `libdnf`, `mypy` returns the following error on
`lib/ansible/modules/dnf.py`:

```
import: Cannot find implementation or library stub for module named "libdnf"
```

This commit makes `mypy` ignore missing imports for `libdnf`.

Signed-off-by: Benoît Knecht <bknecht@protonmail.ch>
Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

LGTM

@ansibot ansibot removed 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 Apr 9, 2024
@Akasurde Akasurde merged commit d304fd8 into ansible:devel Apr 9, 2024
66 checks passed
mkrizek added a commit to mkrizek/ansible that referenced this pull request Apr 10, 2024
In ansible#80094 support for var substitution for cachedir was added but there
are more options that should be supported. Using an API for
prepend_installroot which should be done anyway provide that feature
so use that. In addition, perform the operation once all substitutes
are in place (releasever as well).
mkrizek added a commit that referenced this pull request Apr 12, 2024
In #80094 support for var substitution for cachedir was added but there
are more options that should be supported. Using an API for
prepend_installroot which should be done anyway provide that feature
so use that. In addition, perform the operation once all substitutes
are in place (releasever as well).
@ansible ansible locked and limited conversation to collaborators May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.15 bug This issue/PR relates to a bug. 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. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants