Update dnf.py

[bartlomiejkida]

SUMMARY

Fix ansible-lint error: "Package installs should not use latest."

ISSUE TYPE

• Docs Pull Request

COMPONENT NAME

• Docs examples

ADDITIONAL INFORMATION

Example

[localhost:: ~ ]:$ cat test.yml 
- name: test playbook
  hosts: all
  tasks:
  - name: Upgrade all packages
    ansible.builtin.dnf:
      name: "*"
      state: latest
[localhost:: ~ ]:$ ansible-lint --version|grep using
ansible-lint 6.10.0 using ansible 2.14.1

[localhost:: ~ ]:$ ansible-lint test.yml 
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: test.yml
WARNING  Listing 2 violation(s) that are fatal
name[casing]: All names should start with an uppercase letter. (warning)
test.yml:1

package-latest: Package installs should not use latest.
test.yml:4 Task/Handler: Upgrade all packages

You can skip specific rules or tags by adding them to your configuration file:
# .config/ansible-lint.yml
warn_list:  # or 'skip_list' to silence them completely
  - package-latest  # Package installs should not use latest.

               Rule Violation Summary               
 count tag            profile  rule associated tags 
     1 name[casing]   moderate idiom (warning)      
     1 package-latest safety   idempotency          

Failed after basic profile, 1/5 star rating: 1 failure(s), 1 warning(s) on 1 files.
A new release of ansible-lint is available: 6.10.0 → 6.10.2

[localhost:: ~ ]:$ echo -e "      update_only: true" >> test.yml

[localhost:: ~ ]:$ ansible-lint test.yml 
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: test.yml
WARNING  Listing 1 violation(s) that are fatal
name[casing]: All names should start with an uppercase letter. (warning)
test.yml:1


              Rule Violation Summary              
 count tag          profile  rule associated tags 
     1 name[casing] moderate idiom (warning)      

Passed with basic profile, 1/5 star rating: 0 failure(s), 1 warning(s) on 1 files.
A new release of ansible-lint is available: 6.10.0 → 6.10.2

[ansibot]

Thanks for your Ansible docs contribution! We talk about Ansible documentation on matrix at #docs:ansible.im and on libera IRC at #ansible-docs if you ever want to join us and chat about the docs! We meet there on Tuesdays (see the Ansible calendar) and welcome additions to our weekly agenda items - scroll down to find the upcoming agenda and add a comment to put something new on that agenda.

click here for bot help

[mkrizek]

This would prevent any dependencies introduced in new versions of packages not being installed and ultimately failing the upgrade. I believe wanting to upgrade all packages regardless of whether they pull in new packages as their dependencies is a valid use case that is worth documenting.

[samccann]

We have two examples here that use 'latest'. Both would cause an ansible-lint error. IMO one of the examples should be changed to show update_only in action. @mkrizek - would it be more appropriate to move this change to the other http example at

ansible/lib/ansible/modules/dnf.py

Line 288 in 49ed148

- name: Install the latest version of Apache

?
And change the name to "Update to latest version of Apache but do not install new dependencies" Or something like that to make clear what it is doing?

[mkrizek]

IMHO I would ignore this lint error. Using latest for installs is perfectly valid use case. Otherwise for cases where users do not wish to update the package (if available) on every playbook run (for example when reproducible environment is desired), state: present can be used instead.

Also, the lint error can be suppressed like so:

state: latest # noqa package-latest

Having said that I agree that the use of update_only should be documented.

I would add a new example that would better describe the intent of update_only, how about the following:

- name: Update the webserver, depending on which is installed on the system, do not install the other one
  ansible.builtin.dnf:
    name:
      - httpd
      - nginx
    state: latest
    update_only: yes

(the description in name could probably use better wording)

What do you think @samccann?

[samccann]

@ssbarnea - do you have an opinion here since we're talking about ansible-lint errors ?

[ssbarnea]

Thanks for pinging me. Yep, I think that it is a good idea to update the docs and use an example that is closer to what people are expected to use, and avoid use of examples that would not pass our linting.

[samccann]

@ssbarnea That's where I think we have problems (using a real-world example w/o lint errors). If I understand correctly, @mkrizek is saying the existing example is already a real-world example, but will also throw ansible-lint errors using the default ansible-lint rules.

[s-hertel]

@samccann In case you haven't seen it already, here's ansible-lint's rationale https://github.com/ansible/ansible-lint/blob/dd29bc01d8643d7c016281158bde9eb4743d2346/src/ansiblelint/rules/package_latest.md. This PR wouldn't hide all examples that break the rule (at a glance, I see it in the yum and package examples too). Modules with more states than present/absent are a pet peeve so I can understand where the rule is coming from, but it's a valid use-case (and a common use case I'd guess) and there are no plans to deprecate it. I am in favor of adding a new example instead of replacing the existing one, as suggested by @mkrizek.

[samccann]

Thanks @wireboot for the PR and initiating the conversation around this fix! And welcome to the Ansible project.