Fixes #37051 - Add Katello Ansible default job templates

[jeremylenz]

What are the changes introduced in this pull request?

In the new host details UI, remote execution jobs use, by default, the 'xxx by search - Katello Script Default' job templates for package install / remove / upgrade, and errata apply. For those that use remote execution via Ansible, previously there was no way to use the web UI because there were no corresponding 'by search' Ansible job templates. This PR adds those.

Considerations taken when implementing this change?

For the package actions, I was originally looking at inheriting from "Package Action - Ansible Default." But that template only takes a single package name, and I needed multiple. So I'm just rendering the Ansible directly in Katello's template, instead of raising a PR to foreman_ansible. You can yell at me if you must.

What are the testing steps for this pull request?

1. Set up REX with Ansible (enable foreman_ansible and make sure your smart proxy shows the "Ansible" feature)
2. Check out the PR
3. bundle exec rails runner "ForemanInternal.all.first.destroy" to trigger seeds, then restart your Foreman server
4. Go to the Remote Execution Features page and click on each feature
5. Select the '<feature name> by search - Katello Ansible Default' as the default template for each feature. Example:
   

From the new host details Packages and Errata tabs, try

• package remove
• package install
• errata apply
• package upgrade
• package upgrade but the one where you have the little dropdown and you select which version to upgrade to (I didn't test this yet)

Make sure that templates render the Ansible correctly (check the 'Preview templates' tab on the REX job details). There's no need to actually communicate with the host, though this is nice to have.

[jeremylenz]

Marking draft for now because this also needs #10845. When that's merged I'll rebase this without it. But feel free to review any time.

[Et7f3]

Sorry for my suggestion (I wanted to upstream it but the PR process of opening a issue refrained me to upstream them: however we tested those in prod and they works ok) The only culprit is no package = all package for update. I don't know if we should force user to specify '*'. One thing we did is run this job on machine with query applicable_rpms ~ yum and we use package query name = yum however it resolved to empty query and then a yum update was fired (hopefully no machine were broken).

I don't think if we use the UI we would see the warning.

[Et7f3]

Suggested change

	---
	- hosts: all
	tasks:
	- package:
	name:
	<% package_names.each do |package_name| -%>
	- <%= package_name %>
	<% end -%>
	state: present
	<%= render_template('Package Action - Ansible Default', :state => 'present', :name => package_names) %>

At least it will be on par with the Katello without Ansible

[Et7f3]

Rendered output:

# For Windows targets use the win_package module instead.
---
- hosts: all
  tasks:
    - package:
        name: ["libssh2", "libssh-devel"]
        state: present

query: name = libssh-devel or name = libssh2
We have a trailling line empty but that can be changed by adding minus

[Et7f3]

Suggested change

	---
	- hosts: all
	tasks:
	- package:
	name:
	<% package_names.each do |package_name| -%>
	- <%= package_name %>
	<% end -%>
	state: present
	<%= render_template('Package Action - Ansible Default', :state => 'latest', :name => package_names) %>

You have written state: present

[Et7f3]

There is really an error here: state: present -> state: latest or use Package Action - Ansible Default with latest
I don't put the rendered template because my test machine is outdated (long list) but we have the same layout than the others.

Scenario tested:

• update one package by clicking and it works correctly.
• update two packages by clicking and it works correctly.
• update with search query * and I see the full list and it works correctly
• update with blank search query => the lists is [] so a correct yaml. rc=0 and "Nothing to do"
• update with a query to a uptodate package name ~ acl same than blank query. In the proposed file: it would cause to trigger a yum update because the list is empty.

[jeremylenz]

The only culprit is no package = all package for update. I don't know if we should force user to specify '*'

The reason for the empty string is that if you run dnf update with no arguments, it updates all packages. I don't think it would understand dnf update *? (I could be wrong)

[Et7f3]

The reason for the empty string is that if you run dnf update with no arguments, it updates all packages. I don't think it would understand dnf update *? (I could be wrong)

if we use the ansible module yes: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/dnf_module.html other dnf command like provides,list accepts glob. I can test it but almost sure it supports it.

[jeremylenz]

Rebased per #10850 (comment), marked ready for review

[Et7f3]

I really think my suggestion is worthwhile at least for the last comment.

[Et7f3]

Rendered output:

# For Windows targets use the win_package module instead.
---
- hosts: all
  tasks:
    - package:
        name: ["libssh2", "libssh-devel"]
        state: present

query: name = libssh-devel or name = libssh2
We have a trailling line empty but that can be changed by adding minus

[Et7f3]

There is really an error here: state: present -> state: latest or use Package Action - Ansible Default with latest
I don't put the rendered template because my test machine is outdated (long list) but we have the same layout than the others.

Scenario tested:

• update one package by clicking and it works correctly.
• update two packages by clicking and it works correctly.
• update with search query * and I see the full list and it works correctly
• update with blank search query => the lists is [] so a correct yaml. rc=0 and "Nothing to do"
• update with a query to a uptodate package name ~ acl same than blank query. In the proposed file: it would cause to trigger a yum update because the list is empty.

[ianballou]

@ianballou 's testing tracker

Tested so far:
Single package remove, install, update
Single errata apply
Install numerous packages

[jeremylenz]

There is really an error here: state: present -> state: latest or use Package Action - Ansible Default with latest

@Et7f3 Sometimes, you need to be able to upgrade to a version that is not the latest. There will be multiple upgrade versions to choose from, and you could choose one that's not the latest. This feature was added in #10263. You can see the community demo of this feature here. My idea with using present instead of latest for package updates was so this would continue to work. The package name in the rendered template should be the full package with version number, not just the base name. If you or @ianballou can test this scenario, please let me know since I haven't tested it yet.

[ianballou]

Re: updating to an intermediate version:

---
- hosts: all
  tasks:
    - package:
        name: 
          - c-ares-1.17.1-5.el9_2.1.x86_64
          - crypto-policies-20230731-1.git94f0e2c.el9_3.1.noarch
        state: present

Here's an example. Neither of these are the latest.

[ianballou]

@jeremylenz should clicking the hamburger button on a package to update and selecting "Upgrade via remote execution" use the ansible script if it's the default? In my case it used bash.

[ianballou]

I'm hitting a weird bug that, so far, isn't reproducible with all RPMs:

The ansible script gets the latest version of the package, but the input is an earlier version to update.

Reproducible via a RHEL 9 host and RHEL 9 appstream/baseos

[jeremylenz]

should clicking the hamburger button on a package to update and selecting "Upgrade via remote execution" use the ansible script if it's the default?

In my testing it did! Keep in mind you do have to set the default script for all the features on the Features page (katello_package_install_by_search, katello_packages_update_by_search, katello_errata_install_by_search, etc.)

[jeremylenz]

The ansible script gets the latest version of the package, but the input is an earlier version to update.

Reproducible via a RHEL 9 host and RHEL 9 appstream/baseos

Can you also test the same with the defaults set back to Script? This def looks like a bug but I don't see how it could be ansible-related

[ianballou]

Can you also test the same with the defaults set back to Script? This def looks like a bug but I don't see how it could be ansible-related

Confirmed it fails with Script as well. redmine.

In my testing it did! Keep in mind you do have to set the default script for all the features on the Features page (katello_package_install_by_search, katello_packages_update_by_search, katello_errata_install_by_search, etc.)

All good, I got confused by the switch to Update Package - Katello Ansible Default and thought it was script since I was bash, oops.

[ianballou]

This is working well for me, tested via the host details page.

[Et7f3]

@Et7f3 Sometimes, you need to be able to upgrade to a version that is not the latest. There will be multiple upgrade versions to choose from, and you could choose one that's not the latest.

(I just discovered randomly while showcasing foreman to colleagues)

latest c-ares can vary but latest c-ares-1.17.1-5.el9_2.1.x86_64 (the one rendered should be unique). I will test it soon but think latest/present in Ansible will honour the version we specify.

I don't recall the script version take care of this (command yum update/yum install)

[Et7f3]

I will test it soon but think latest/present in Ansible will honour the version we specify.

Test "Update Packages by search query - Katello Ansible Default" with latest and selected version not latest

On a machine with libcurl-7.76.1-23.el9.x86_64 upgradable to libcurl-7.76.1-23.el9_2.4.x86_64 or libcurl-7.76.1-26.el9_3.2.x86_64
I triggered Update package(s) id ^ (18828)

# For Windows targets use the win_package module instead.
---
- hosts: all
  tasks:
    - package:
        name: ["libcurl-7.76.1-23.el9_2.4.x86_64"]
        state: latest

Packages search query id ^ (18828)
Selected update versions ["libcurl-7.76.1-23.el9_2.4.x86_64"]

I changed my template to present like this PR:

Test "Update Packages by search query - Katello Ansible Default" with present and selected version not latest

On a machine with open-vm-tools-12.1.5-1.el9_2.2.x86_64 upgradable to open-vm-tools-12.1.5-1.el9_2.3.x86_64, open-vm-tools-12.2.5-3.el9_3.x86_64 or open-vm-tools-12.2.5-3.el9_3.2.x86_64 I triggered `Update package(s) id ^ (19232)` ```yaml # For Windows targets use the win_package module instead. --- - hosts: all tasks: - package: name: ["open-vm-tools-12.1.5-1.el9_2.3.x86_64"] state: present ```

Packages search query id ^ (19232)
Selected update versions ["open-vm-tools-12.1.5-1.el9_2.3.x86_64"]

I don't recall the script version take care of this (command yum update/yum install)

Test "Update Packages - Katello Ansible Default" selected version not latest

On a machine with glibc-2.34-60.el9.x86_64 upgradadable to glibc-2.34-60.el9_2.7.x86_64 or glibc-2.34-83.el9_3.7.x86_64
I triggered Update package(s) glibc-2.34-60.el9_2.7.x86_64 that executed: yum -y update glibc-2.34-60.el9_2.7.x86_64

I am suprised that these template use raw ansible command when Package action - Ansible Default is present

Here is the result:

yum list installed glibc libcurl open-vm-tools | cat
Mise à jour des référentiels de gestion des abonnements.
Paquets installés
glibc.x86_64             2.34-60.el9_2.7       @rhel-9-for-x86_64-baseos-rpms   
libcurl.x86_64           7.76.1-23.el9_2.4     @rhel-9-for-x86_64-baseos-rpms   
open-vm-tools.x86_64     12.1.5-1.el9_2.3      @rhel-9-for-x86_64-appstream-rpms

Ok since package_names_for_job_template always fill the name+version present/latest does the same thing. I haven't tested on a debian host but think it also fill name+version. Ok so I have no strong argument about using latest instead of present (maybe just avoid future reader to think it is incorrect). And that is the reason the template Update Packages - Katello Ansible Default always works even if it begin with yum update -y.

One more argument about depending on Package Action - Ansible Default is that if one day we want to add a feature (like enable repos in pre_task, pass other options, ...). With the current implementation you have to repeat for each jobs instead of doing it once. So it is not DRY friendly.

[jeremylenz]

Ok so I have no strong argument about using latest instead of present (maybe just avoid future reader to think it is incorrect)

I think this is a good reason. I had originally used present instead of latest because I thought latest would not work with a non-latest version, but as your testing shows, present and latest are equivalent as long as the full package name with version is specified. So latest better signifies the intent. Since this is a package upgrade, I will change it to latest.

One more argument about depending on Package Action - Ansible Default is that if one day we want to add a feature (like enable repos in pre_task, pass other options, ...). With the current implementation you have to repeat for each jobs instead of doing it once. So it is not DRY friendly.

Using Package Action - Ansible Default would require the ['short', 'form'] of writing lists. I did not even know this was possible before now. To be honest, I just prefer the long form of YAML lists (one item per line, beginning with -) which I see everywhere and I believe is easier to read. So if you don't mind I would like to keep it this way. (also, I don't think DRY will be much benefit here, since these templates do not change often.)

[jeremylenz]

Changed the package update template to use latest.

[Et7f3]

So if you don't mind I would like to keep it this way

I don't mind short form/long form. Can you modify the Package Action - Ansible Default to print in long form then ?

You already have the code just put in the right file.

since these templates do not change often

We use the pre_script and thus we would need to add x4

[Et7f3]

Suggested change

	<% package_names.each do |package_name| -%>
	- <%= package_name %>
	<% end -%>
	<% if package_names.empty? -%>
	name: []
	<% else -%>
	name:
	<% package_names.each do |package_name| -%>
	- <%= package_name %>
	<% end -%>
	<% end -%>

Oh one thing about short form/long form for list: if list is empty (because we use the REX tab) you need to use the short form otherwise the yaml won't be valid and we will get an syntax error.

So either print only short for the empty case or render_error that forbid it.

[Et7f3]

This seems like common idioms there should be a macro:

Suggested change

	name:
	<% package_names.each do |package_name| -%>
	- <%= package_name %>
	<% end -%>
	name: <%= indent(10) { to_yaml(package_names).gsub(/---/, "") } -%>

['long', 'form'] render:

        name:           
          - long
          - form

while [] render

        name:            []

code already exists in foreman

[jeremylenz]

I don't mind short form/long form. Can you modify the Package Action - Ansible Default to print in long form then ?

That would require a code change in https://github.com/theforeman/foreman_ansible which I am trying to avoid.

[Et7f3]

That would require a code change in https://github.com/theforeman/foreman_ansible which I am trying to avoid.

Oh didn't see it was another repo. Nevertheless this change is appreciable: will open a PR for it.

I tried to add argument about consistency but some code in Katello use it:

katello/app/views/foreman/job_templates/remove_package_-_katello_ansible_default.erb

Line 16 in f281fc5

<%= render_template('Package Action - Ansible Default', :state => 'absent', :name => input('package')) %>

katello/app/views/foreman/job_templates/install_package_-_katello_ansible_default.erb

Line 16 in f281fc5

<%= render_template('Package Action - Ansible Default', :state => 'latest', :name => input('package')) %>

but then found upgrade used another mechanism:

katello/app/views/foreman/job_templates/update_package_-_katello_ansible_default.erb

Line 16 in f281fc5

<%= render_template('Run Command - Ansible Default', :command => "yum -y update #{input('package')}") %>

[jeremylenz]

---
- hosts: all
  tasks:
    - package:
        name: []

It seems this doesn't work. (dnf thinks there is nothing to do.) I will need to handle the "update all" case separately. Will update shortly.

[Et7f3]

---
- hosts: all
  tasks:
    - package:
        name: []

It seems this doesn't work. (dnf thinks there is nothing to do.) I will need to handle the "update all" case separately. Will update shortly.

It is in the scenario I tested: #10850 (comment) and I think it is correct behavior as it avoid a global yum upgrade

[jeremylenz]

It is in the scenario I tested: #10850 (comment) and I think it is correct behavior as it avoid a global yum upgrade

When you select all upgradable packages in the web UI, the resulting command should be dnf -y upgrade (with no more arguments). This is the way the select all feature works - an empty search query matches all packages. The Script templates, the web UI, and our scoped_search framework all work the same way as well.

[jeremylenz]

Rebased to current master and updated the package upgrade template to work properly with an empty search.

[jeremylenz]

@ianballou This should be ready for another look, if you wish!

[ianballou]

Working well!

Re: update script updates all vs none for an empty package list, I agree that an empty list should update all because it mimics the yum update command.

[jeremylenz]

Merging to ensure it gets into Katello 4.11.1.

Thanks for the feedback and reviews @ianballou @Et7f3 !