ansible
ansible copied to clipboard
dnf module doesn't honor state: present / state: installed
SUMMARY
Fixes #76463
When using state: present or state: installed the check on if the package is installed always returns False
if installed.filter(**package_spec):
return True
else:
return False
The dict unpacking of package_spec caused this. This fix removes the unpacking.
ISSUE TYPE
- Bugfix Pull Request
COMPONENT NAME
dnf
ADDITIONAL INFORMATION
Details in issue mentioned, but before change running the following play:
- hosts: all
tasks:
- dnf:
name: java-11-openjdk
state: installed
java-11-openjdk is already installed before the play:
[root@centos-s-1vcpu-1gb-nyc1-01 ~]# rpm -qa | grep java-11-openjdk
java-11-openjdk-11.0.13.0.8-1.el8_4.x86_64
java-11-openjdk-headless-11.0.13.0.8-1.el8_4.x86_64
Updates the packages which shouldn't happen:
"msg": "",
"rc": 0,
"results": [
"Installed: java-11-openjdk-headless-1:11.0.13.0.8-3.el8_5.x86_64",
"Installed: java-11-openjdk-1:11.0.13.0.8-3.el8_5.x86_64",
"Removed: java-11-openjdk-1:11.0.13.0.8-1.el8_4.x86_64",
"Removed: java-11-openjdk-headless-1:11.0.13.0.8-1.el8_4.x86_64"
]
After fix:
[root@centos-s-1vcpu-1gb-nyc1-01 ~]# rpm -qa | grep jdk
java-11-openjdk-11.0.13.0.8-1.el8_4.x86_64
java-11-openjdk-headless-11.0.13.0.8-1.el8_4.x86_64
Run play and returns:
}
},
"msg": "Nothing to do",
"rc": 0,
"results": []
}
packages still at same level:
[root@centos-s-1vcpu-1gb-nyc1-01 ~]# rpm -qa | grep jdk
java-11-openjdk-11.0.13.0.8-1.el8_4.x86_64
java-11-openjdk-headless-11.0.13.0.8-1.el8_4.x86_64
The test ansible-test sanity --test package-data [explain] failed with the error:
Command "/root/.ansible/test/venv/sanity.package-data/3.10/ed96082c/bin/python /root/ansible/test/sanity/code-smell/package-data.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
File "/root/ansible/test/sanity/code-smell/package-data.py", line 401, in <module>
main()
File "/root/ansible/test/sanity/code-smell/package-data.py", line 378, in main
sdist_path = create_sdist(tmp_dir)
File "/root/ansible/test/sanity/code-smell/package-data.py", line 185, in create_sdist
raise Exception('make snapshot failed:\n%s' % stderr)
Exception: make snapshot failed:
docs/man/man1/ansible-console.1.rst:46: (WARNING/2) Bullet list ends without a blank line; unexpected unindent.
make: *** [Makefile:143: changelog] Error 5
The test ansible-test sanity --test changelog [explain] failed with 1 error:
changelogs/fragments/76463-dnf-installed-present-bug.yml:0:0: yaml parsing error
The test ansible-test sanity --test yamllint [explain] failed with 2 errors:
changelogs/fragments/76463-dnf-installed-present-bug.yml:2:119: error: syntax error: mapping values are not allowed here (syntax)
changelogs/fragments/76463-dnf-installed-present-bug.yml:2:119: unparsable-with-libyaml: None - mapping values are not allowed in this context
Commenter does not have sufficient privileges for PR 76467 in repo ansible/ansible
/rebuild
The tests are failing due to the changes made in this PR. The packages needed to run the dnf test are not being installed since the dnf module was changed.
The tests are failing due to the changes made in this PR. The packages needed to run the
dnftest are not being installed since thednfmodule was changed.
yes thanks, figured out the exact test and reason and pushing shortly
ok apologies for all the commits, still getting used to the test flow, think I got it now :-)
This takes care of current issue mentioned, I do see some areas in this code I may open another pr at a later point.
I can see the issue clearly now.
Here's Fedora35:
[root@fedora-s-1vcpu-1gb-nyc1-01 ~]# dnf install java-11-openjdk
Last metadata expiration check: 0:00:10 ago on Tue 07 Dec 2021 04:25:41 PM UTC.
Package java-11-openjdk-1:11.0.12.0.7-4.fc35.x86_64 is already installed.
Dependencies resolved.
Nothing to do.
Complete!
On CentOS the behavior is different..
[root@centos-s-1vcpu-1gb-nyc1-01 ~]# dnf install java-11-openjdk
Last metadata expiration check: 0:01:42 ago on Tue 07 Dec 2021 04:25:57 PM UTC.
Package java-11-openjdk-1:11.0.13.0.8-1.el8_4.x86_64 is already installed.
Dependencies resolved.
==========================================================================================================================================================================
Package Architecture Version Repository Size
==========================================================================================================================================================================
Upgrading:
java-11-openjdk x86_64 1:11.0.13.0.8-3.el8_5 appstream 266 k
java-11-openjdk-headless x86_64 1:11.0.13.0.8-3.el8_5 appstream 39 M
Transaction Summary
==========================================================================================================================================================================
Upgrade 2 Packages
Total size: 40 M
Is this ok [y/N]:
This is due to /etc/dnf/dnf.conf defaults:
Fedora35:
best=False
CentOS 8:
best=True
Changing CentOS node to use best=False and I get what I would expect.
[root@centos-s-1vcpu-1gb-nyc1-01 ~]# dnf install java-11-openjdk
Last metadata expiration check: 0:01:19 ago on Tue 07 Dec 2021 04:41:11 PM UTC.
Package java-11-openjdk-1:11.0.13.0.8-1.el8_4.x86_64 is already installed.
Dependencies resolved.
Nothing to do.
Complete!
Discussion here: https://github.com/rpm-software-management/dnf/pull/1484
Should we change this in ansible? I guess if state == installed || present we could force the config to nobest but not sure if that's a correct way forward.
Adding more to above after further testing. I think the bug is here:
# Set best
if self.nobest:
conf.best = 0
and should be:
# Set best
if not self.nobest:
conf.best = 0
By default self.nobest is False so ,so we should set dnf config best to False.
Ran some unit tests locally, so far so good:
Downgraded java-11-openjdk on CentOS 8 and Fedora 35 nodes:
Test on my branch
Play:
- hosts: all
tasks:
- dnf:
name:
- java-11-openjdk
state: installed
java-11-openjdk did not get upgrade ✔️
(venv) ➜ ansible git:(dnfpresentfix) ✗ ansible-playbook dnf.yml -i hosts -u root
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features under development. This is a rapidly changing source of code and can become unstable at any point.
PLAY [all] ***********************************************************************************************************************************************************************************************************************************************************************************
TASK [Gathering Facts] ***********************************************************************************************************************************************************************************************************************************************************************
ok: [159.223.127.187]
ok: [167.172.139.237]
TASK [dnf] ***********************************************************************************************************************************************************************************************************************************************************************************
ok: [159.223.127.187]
ok: [167.172.139.237]
PLAY RECAP ***********************************************************************************************************************************************************************************************************************************************************************************
159.223.127.187 : ok=2 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0
167.172.139.237 : ok=2 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0
Test on devel Same play of above..
(venv) ➜ ansible git:(devel) ✗ ansible-playbook dnf.yml -i hosts -u root
[WARNING]: You are running the development version of Ansible. You should only run Ansible from "devel" if you are modifying the Ansible engine, or trying out features
under development. This is a rapidly changing source of code and can become unstable at any point.
PLAY [all] ***************************************************************************************************************************************************************
TASK [Gathering Facts] ***************************************************************************************************************************************************
ok: [159.223.127.187]
ok: [167.172.139.237]
TASK [dnf] ***************************************************************************************************************************************************************
ok: [167.172.139.237]
changed: [159.223.127.187]
PLAY RECAP ***************************************************************************************************************************************************************
159.223.127.187 : ok=2 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0
167.172.139.237 : ok=2 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0
Would like to see what others think?
By default self.nobest is False so ,so we should set dnf config best to False.
It's inverted, though. self.nobest (emphasis on the no). So when nobest is true, best should be false. When nobest is false, best should be true.
See also https://github.com/ansible/ansible/pull/74224 which probably needs to be dusted off if someone wants to take it over. I went down the rabbit hole a fair bit there, and added some comments and a bunch of test cases. Some parts of it touch the nobest logic.
Thanks, you cleared that up and made me also realize I went down that nobest rabbit hole.. SO here is the issue around using state:installed / state: present and back to my original thought around this code.
In _is_installed method the check below may wrongly determine if certain packages exist or not:
if installed.filter(**package_spec):
return True
else:
return False
on my Fedora and CentOS nodes... I have java-11-openjdk package installed but the check above would return False. This is due to _packagename_dict method.. if epoch of the package is not successfully grabbed when parsing it will set it 0:
e.g.: {'name': 'java', 'epoch': '0', 'release': 'openjdk', 'version': '11'}
However packages epoch is '1':
[root@centos-s-1vcpu-1gb-nyc1-01 ~]# dnf list java-11-openjdk
Last metadata expiration check: 0:00:18 ago on Thu 09 Dec 2021 10:07:24 PM UTC.
Installed Packages
java-11-openjdk.x86_64 1:11.0.13.0.8-1.el8_4 @appstream
Available Packages
java-11-openjdk.x86_64 1:11.0.13.0.8-3.el8_5
Again sorry for all commits to this.. but I try to be descriptive in each update so its less confusing :-)
Summary of the fix:
- updated
_packagename_dictmethod for better handling of getting package details if the package is currently installed on the system and if user is calling with state: present or state: installed. - Updated
_whatprovidesmethod and added an argumentavailablewhich by default (available=True) maintains prior behavior such as getting any available package that's given. Using available=False allows for checking on a package a user has given in the form of "java-11-openjdk" for only the currently installed packages and gets the NEVRA format if there is an installed packages that provides the given one. This addresses an issue I saw where in previous code the NEVRA could have incorrect values mentioned in prior comment - Add output to stdout when the package is indeed already installed as this is good for user and follows "Always return useful data, even when there is no change." Example below.
- Added test cases and few relating to the java-11-openjdk package specifically.
play:
tasks:
- dnf:
name:
- java-11-openjdk
- httpd
- vim
state: installed
output when all packages are already installed:
"msg": "Nothing to do",
"rc": 0,
"results": [
"Package vim is already installed.",
"Package java-11-openjdk is already installed.",
"Package httpd is already installed."
Output when java-11-openjdk and vim are already installed but not httpd:
"msg": "",
"rc": 0,
"results": [
"Package java-11-openjdk is already installed.",
"Package vim is already installed.",
"Installed: centos-logos-httpd-85.8-2.el8.noarch",
"Installed: mod_http2-1.15.7-3.module_el8.4.0+778+c970deab.x86_64",
"Installed: apr-1.6.3-12.el8.x86_64",
"Installed: apr-util-1.6.1-6.el8.x86_64",
"Installed: apr-util-bdb-1.6.1-6.el8.x86_64",
"Installed: httpd-2.4.37-43.module_el8.5.0+1022+b541f3b1.x86_64",
"Installed: httpd-filesystem-2.4.37-43.module_el8.5.0+1022+b541f3b1.noarch",
"Installed: mailcap-2.1.48-3.el8.noarch",
"Installed: apr-util-openssl-1.6.1-6.el8.x86_64",
"Installed: httpd-tools-2.4.37-43.module_el8.5.0+1022+b541f3b1.x86_64"
]
}
Why needs_revision? Because @s-hertel requested changes?
Is there any progress? This issue is very crucial for us to see if there are any changes on managed nodes. @s-hertel
Hello. I would like to ask if the issue is being addressed in any way. It has been open for almost 2 years now, and there have been no changes so far. It's somewhat inconvenient to constantly verify changes that essentially do not exist. Thank you.
rebased, ci passed.
/azp run
Azure Pipelines successfully started running 1 pipeline(s).
@webknjaz When this request will be merged?
@jameslivulpi this branch has conflicts. @s-hertel could you review the changes once the conflicts get resolved? This damages our experience with Ansble package module significantly.
Superseded by https://github.com/ansible/ansible/pull/82725