rpm_ostree_pkg: fails to set needs_reboot correctly

[millerthegorilla]

Summary

It is expected that ansible tasks are idempotent. However, when running rpm_ostree_package to install already installed, but pending, packages, the needs_reboot is returned as false, when if there are pending packages, as listed by rpm-ostree db diff the needs_reboot should be true.

Issue Type

Bug Report

Component Name

rpm_ostree_pkg

Ansible Version

ansible [core 2.18.4]
  config file = None
  configured module search path = ['/var/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /var/home/user/src/ansible-collection-picoreos/millerthegorilla/picoreos/.venv/lib64/python3.13/site-packages/ansible
  ansible collection location = /var/home/user/.ansible/collections:/usr/share/ansible/collections
  executable location = /var/home/user/src/ansible-collection-picoreos/millerthegorilla/picoreos/.venv/bin/ansible
  python version = 3.13.2 (main, Feb  4 2025, 00:00:00) [GCC 14.2.1 20250110 (Red Hat 14.2.1-7)] (/var/home/user/src/ansible-collection-picoreos/millerthegorilla/picoreos/.venv/bin/python)
  jinja version = 3.1.6
  libyaml = True

Community.general Version

Collection        Version
----------------- -------
community.general 10.5.0 

Configuration

config...

CONFIG_FILE() = None
EDITOR(env: EDITOR) = /usr/bin/nano

GALAXY_SERVERS:

OS / Environment

fedora silverblue 41 host addressing a stable coreos remote, on a raspberry pi4b.

Steps to Reproduce

on second run, without reboot of remote. rpm-ostree db diff on remote reveals a pending deployment.

- name: Ensure ClamAV packages are installed.
  package:
    name: "{{ clamav_packages }}"
  register: clamav_packages_install

- name: Reboot if necessary (when rpm-ostree is package manager)
  ansible.builtin.reboot:
    msg: "Reboot initiated by Ansible"
  when:
    - ansible_package_use is defined
    - '"rpm_ostree_pkg" in ansible_package_use'
    - clamav_packages_install.needs_reboot

needs_reboot is false, when it should be true.

Expected Results

clamav_packages_install.needs_reboot should be true

Actual Results

needs_reboot is false

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/rpm_ostree_pkg.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @Akasurde @dustymabe
click here for bot help

[millerthegorilla]

The module uses the return message from the rpm-ostree command.
But this shows no change if the command is run again without an intervening reboot.

If needs_reboot is set to ('pending' in rpm-ostree db diff), then this would work as expected.

[russoz]

hi @millerthegorilla thanks for reporting. Would you be willing to try and fix the module code? We'd be happy to assist you in the journey.

[felixfontein]

One question is whether the module actually needs fixing. I understand "Determine if machine needs a reboot to apply current changes." that it will inform you whether the changes made by the module require a reboot. If the module didn't do any changes (due to idempotency), there's no need to reboot.

The return value was added in #9167 by @shios86, maybe they can say more about this?

[shios86]

I did a simple check on the output of the command of rpm-ostree to set the flag for needs_reboot, it will not be able to detect pending changes on subsequent run of the same package install.

One question that should be asked, is whether the module itself should run 2 different command for 1 task.

What I could do, is implement another state called reboot that would just run the command mentioned above.

[felixfontein]

reboot is not a valid state name (it would be an action, but that's not what you mean). Having an _info module for querying whether a reboot is needed is probably the best way to solve this.

[shios86]

In that case, should needs_reboot flag be removed from the current pkg module?

[felixfontein]

No, why? It works as documented (though maybe the documentation can be improved to make clearer what exactly it does).

[shios86]

Okay, I can probably make this module later, and further clarify what needs_reboot does.
Refer to #10009 (comment)

[millerthegorilla]

Hi, I would have been happy to make the changes, but I would have suggested the need to use the needs_reboot flag.

I would have made a pending in rpm-ostree db diff and set the needs reboot flag.

The reason is because there are already codebases out there, including mine, that rely on the needs_reboot to trigger a reboot in a task that follows the rpm_ostree_package task.

Idempotency in the case of package installation is a special case for rpm-ostree systems. There is no point in installing a package without a reboot.

So if I run the task twice without reaching a reboot, for example in the case of some package installation failure, I would expect the code to reflect the continued need to reboot the system, even if no changes are made.

[millerthegorilla]

In fact, thinking about it, the need to reboot is such an integral part of the process of installing a package that it could be suggested that the rpm_ostree_package module could handle the reboot itself, if a flag were set.

In most of the use cases I have, it is the ansible.builtin.package module that is running the code, with rpm_ostree_package set by the variable ansible_package_use'. I am having to push changes to mature code bases to add the ability to detect a need reboot in the case of ansible_package_usebeing set tocommunity.general.rpm_ostree_package. It would be far better if I could define a 'please_reboot' (or similar) flag, when defining ansible_package_use` that would then allow me to use those codebases without the need to change them.

A further issue exists in that some packages are necessary to reboot immediately, in order to configure them or access the newly available paths, and some packages can be left to install and are used later.

If a reboot is initiated in every situation, this can lead to much longer running times.

So, in order to use existing code bases that use the builtin package module, I would, perhaps, define a 'reboot_immediately' flag. In the cases where a reboot can be delayed, I would rely on the availability of a correct reflection of the state, by the needs_reboot output.

[millerthegorilla]

I am guessing we are in different timezones, but if you would like me to make a proposed change then I can make a pull request, just let me know and I will make time.

[shios86]

I did make a parameters for it, but when providing the flag for reboot, it cut the SSH connection (shutting down would close all connections), which would fail the playbook run. As I am not really familiar with ansible internals, I removed the parameters, and made it a simple needs_reboot return value.

As said above, the needs_reboot flag just check the output rpm-ostree, it does not go further. My use cases were very simple so I did not dig further, if you wish to make the change, feel free to do so.

[millerthegorilla]

Ok, I will have a look. There is an existing reboot module which pauses the play until the reboot has completed, so I will take a look at using that or similar.

[felixfontein]

This module should not reboot the system. That's what the reboot module is for.

[millerthegorilla]

I disagree. I am currently having to make a pull request against the devsec.hardening codebase, simply because I cannot call into their code and reboot the machine when various hardening packages are installed.

The ansible.builtin.package module functions correctly by anticipating that when it has finished, the package will have been installed and will be available for further tasks to process.

Currently the ansible.builtin.package module incorrectly selects the atomic_container module when an rpm-ostree system is detected, which is both incorrect and deprecated. Hopefully this will be updated at some point so that community.general.rpm_ostree_package is chosen instead. In the meantime, the instructions to make the builtin package module work with rpm-ostree systems is to define the ansible_package_use fact and set it to community.general.rpm_ostree_package.

So it is expected that the community.general.rpm_ostree_package module will function like any other package manager, and for it to do that it must initiate a reboot.

The change I propose is to set a fact that will allow the rpm_ostree_package module to reboot when it is set, which will ensure that no breaking changes are made, but will also allow rpm_ostree_package to function as expected with the ansible.builtin.package module.

One cannot simply insert a reboot module into existing codebases that use the builtin package module without considerable effort as I have discovered whilst working with devsec.hardening (dev-sec/ansible-collection-hardening#864).

[millerthegorilla]

An alternative would be to include in the rpm_ostree_pkg module a notify handler that could the user could then define a reboot handler for. However, to get it to work, one would also have to add a meta: flush_handlers after the reboot was notified, and this might cause unwanted artefacts, as other handlers may run at that point.

[felixfontein]

I'm still not really convinced that a package installation module should automatically reboot the machine. I don't have experience with such systems though, and don't know about the challenges. I'm pretty sure though that automatically rebooting is wrong (doing so optionally could be OK, maybe).

My suggestion would be to start a discussion on the behavior for rpm_ostree_pkg on the Ansible forum. I'm also curious how similar package managers work. (Is nix similar here, for example? How is it done there? And what exactly are the challenges - is it impossible to modify config files once packages are installed without rebooting first? Or is it just that you can't start the service without rebooting first?)

[felixfontein]

An alternative would be to include in the rpm_ostree_pkg module a notify handler that could the user could then define a reboot handler for. However, to get it to work, one would also have to add a meta: flush_handlers after the reboot was notified, and this might cause unwanted artefacts, as other handlers may run at that point.

This would require every task that calls the package manager to be modified. Modules and action plugins cannot notify anything in playbooks/roles. There is nothing this collection can do about that (except amending the documentation of the module).

[millerthegorilla]

I took the opportunity to learn how action plugins work and wrote an action plugin that calls the rpm_ostree_pkg module and the builtin.reboot, depending on parameters passed to the plugin from the task.

https://gist.github.com/millerthegorilla/acedbc8c5a4eeea4beee9de42c0f29ea

An example playbook:

- name: Test action plugin
  hosts: all
  become: true
  vars:
   # ostree_reboot:
   #   always_reboot: True
   #   msg: "going down!"
   #   pre_reboot_delay: 10
   # ansible_package_use: community.general.rpm_ostree_pkg
  tasks:
    - name: Install_package
      community.general.rpm_ostree_pkg:
        name: nmap
        state: absent
        reboot: true
      register: output
    - debug: var=output

Both the ostree_reboot parameter, which works with ansible.builtin.package and reboot in the task, which works with rpm_ostree_pkg can either be the boolean value true or can be a dict containing arguments to pass to the ansible.builtin.reboot module.

This plugin works by dropping into the directory [your virtual environment]/lib/python3.13/site-packages/ansible_collections/community/general/plugins/action/rpm_ostree_pkg.py

I think it solves the problem reasonably elegantly, and with separation of concerns, decoupling etc...

Whether it should be pushed to the ansible_collections.community.general codebase is another question entirely, although, I think it would allow an rpm-ostree system addressed by ansible to function as expected.

I haven't yet tested it against the devsec codebase, ie an existing codebase that uses ansible.builtin.package. I will do that later today.

[millerthegorilla]

Unfortunately it doesn't work, the package module uses module_loader, and so it only picks up the rpm_ostree_pkg module, not the action plugin. Normally, the module code would be removed to the action plugin, and so the action plugin would work.
So, for the ansible.builtin.package to reboot after installing would take a redesign of the module or the inclusion of the module code or its analog in the action plugin, and the code to be stripped from the module.
Only in an action plugin can the reboot module be called, and it seems rather pointless to rewrite its code.
I guess I will leave this as an exercise in futility, although I think it should be done, I don't suppose it will be accepted, and I am already experiencing that with devsec.

[millerthegorilla]

Ok, I think that perhaps the ansible.builtin.package module perhaps should be able to call custom action plugins, which would resolve my difficulties as I could drop the code above into an action_plugins folder next to my playbook, and then define the ansible_package_use variable.

Given the point of custom action plugins, it would seem to be a sensible idea.

[millerthegorilla]

So I have opened a feature request at ansible/ansible#85021