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

Fixes #37947 - install katello-host-tools by default #10358

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sbernhard
Copy link
Contributor

The default parameter if katello-host-tools should be installed is true according to [1].

[1] https://github.com/theforeman/foreman/blob/develop/app/views/unattended/provisioning_templates/snippet/redhat_register.erb#L81

@ianballou
Copy link
Contributor

I'm not certain it's necessary to install katello-host-tools by default anywhere anymore. It only provides tracer now, nothing else that's helpful that isn't already in dnf.

@sbernhard
Copy link
Contributor Author

I'm not certain it's necessary to install katello-host-tools by default anywhere anymore. It only provides tracer now, nothing else that's helpful that isn't already in dnf.

Still something for Suse, Debian, Ubuntu. And it would be the same as the register template

@ianballou
Copy link
Contributor

I'm not certain it's necessary to install katello-host-tools by default anywhere anymore. It only provides tracer now, nothing else that's helpful that isn't already in dnf.

Still something for Suse, Debian, Ubuntu. And it would be the same as the register template

Gotcha, out of curiosity, which bits still are useful for Suse, Debian, Ubuntu? Are their builds of subscription-manager using katello-host-tools?

@@ -73,7 +73,7 @@ if command -v subscription-manager &>/dev/null; then
subscription-manager refresh
fi

<% if host_param_true?('redhat_install_host_tools') -%>
<% if host_param_true?('redhat_install_host_tools', true) -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since only debian, suse, and ubuntu need this by default, what do you think about adding specific checks here for those OSs to install katello host tools by default?
It might also be worth adding a check for plugin_present?('katello') since katello-host-tools isn't useful without Katello.

This template seems to make different assumptions compared to redhat_register.erb about the Foreman configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, is it safe to assume that katello-host-tools will be available in the host's repository?
I'm not sure exactly where this template comes into play, so I'm not sure if that's a safe assumption.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check about is plugin Katello used, is already there some lines above.

I changed the default to the same as on redhat_register. Katello host tools should be delivered as this is done after the sub man registration.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully understand when this template gets used vs redhat_register. I see, in redhat_register, the katello host tools installation is guarded behind if host_param('kt_activation_keys'). Is the use of kt_activation_keys not some special workflow where a user is more likely to have the katello host tools package distributed?

I just want to make sure users aren't going to suddenly see the script start failing if they're not distributing katello host tools.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is global registration where redhat_register is provisioning.

Copy link
Contributor Author

@sbernhard sbernhard Oct 30, 2024

Choose a reason for hiding this comment

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

be on by default, users will need to know to set redhat_install_host_tools to false

=> same for the redhat_register snippet :-)

@maximiliankolb any thoughts regarding documentation? Can we add both parameter to the documentation (redhat_install_host_tools and redhat_install_host_tracer_tools)

Copy link
Contributor

Choose a reason for hiding this comment

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

From a user perspective, I expect that provisioning a host or registering a host to Foreman+Katello means that I have access to package lists in Foreman UI and to hints if services/the Kernel has to be restarted. With this in mind, I'd recommend, if at all, documenting how to disable this by changing a default parameter.

Regardless of the OS, if you provision a host and register a host to Foreman+Katello, the templates should install whatever is necessary to have the best user experience. I cannot speak much for Foreman on EL/Deb.

Current docs: Installing Tracer + Enabling Tracer

If we change the default from "is not installed" to "is installed automatically because of a new default host param", I can make the appropriate changes in foreman-documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Current docs: Installing Tracer + Enabling Tracer

the important part being the Prerequisite, which you have to do yourself :)

image

Copy link
Contributor

Choose a reason for hiding this comment

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

But there's a difference in doing this after your host is registered to F+K and you can provide content etc. vs registering a host using global host registration where you have to provide just the right repos. Any more packages other than subscription-manager have again more dependencies that users have to cater for.

Strictly speaking, the Client repo is not sufficient for any OS (I did not test RHEL):

$ podman run --rm -it almalinux:9.4

# rm -fr /etc/yum.repos.d/almalinux-*
# vi /etc/yum.repos.d/client.repo
# dnf makecache
...
# cat /etc/yum.repos.d/client.repo
[client]
baseurl=https://yum.theforeman.org/client/latest/el9/x86_64/
enabled=1
gpgcheck=0
name=client
sslverify=0

# dnf install katello-host-tools-tracer
Last metadata expiration check: 0:00:26 ago on Thu Oct 31 09:00:21 2024.
Error: 
 Problem: conflicting requests
  - nothing provides crontabs needed by katello-host-tools-tracer-4.4.0-1.el9.noarch from client
(try to add '--skip-broken' to skip uninstallable packages or '--nobest' to use not only best candidate packages)

I assume that most packages in Client repos have the same issue, e.g. OpenSCAP client requires Ruby ...

My conclusion: Limit pkg installation to a minimum when provisioning/registering a host. After the host is registered to F+K with an AK, have the user provide the "default" OS repos (e.g. BaseOS + AppStream on EL8+) and then install whatever from the combination of default OS repos + Client repo.

Disclaimer: tested with a container. I do not have access to VMs that are not provisioned in an automated way and therefore have automated pkg installations ... One could argue that crontab is installed by default in a EL9 VM compared to a container.

Copy link
Member

Choose a reason for hiding this comment

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

  • nothing provides crontabs needed by katello-host-tools-tracer-4.4.0-1.el9.noarch from client

This is in baseos

My conclusion: Limit pkg installation to a minimum when provisioning/registering a host. After the host is registered to F+K with an AK, have the user provide the "default" OS repos (e.g. BaseOS + AppStream on EL8+) and then install whatever from the combination of default OS repos + Client repo.

IMHO you always want the "default" OS repos and I can't think of a use case where you don't provide those. When you're subscribed to a content view, you always want to include those. The way I always interpreted the note as that you want base + Foreman Client, not "just Foreman Client". The AK should provide access to both.

@sbernhard
Copy link
Contributor Author

Katello host tools adds the hook scripts for zypper (Suse) and apt (Debian) so that package upload happens. And delivers the scripts to collect the tracer data .

@jeremylenz
Copy link
Contributor

Okay, I've done some testing.

With the Client repo synced and an activation key that enables those repositories, global registration works as expected.

However, with an activation key that does not enable the repositories, I get this:

#
# Running registration
#
subscription-manager is already installed!
The system has been registered with ID: d883bfda-8bf2-494b-bc3f-eb310997f050
The registered system name is: rhel9b.fedora.example.com
# Running [rhel9b.fedora.example.com] host initial configuration
/sbin/restorecon
Refreshing subscription data
All local data refreshed
#
# Installing packages
#
Updating Subscription Management repositories.
Red Hat Enterprise Linux 9 for x86_64 - AppStream (RPMs)                                                                                            165 kB/s | 4.5 kB     00:00    
Red Hat Enterprise Linux 9 for x86_64 - BaseOS (RPMs)                                                                                               156 kB/s | 4.1 kB     00:00    
No match for argument: katello-host-tools
Error: Unable to find a match: katello-host-tools
Host [rhel9b.fedora.example.com] initial configuration failed

After this I do see the host is registered and appears in All Hosts, but we get that "initial configuration failed" message because it can't find katello-host-tools.

Personally I don't think this is a great user experience. I would prefer to keep the default false.

cc @stejskalleos since global registration is yours, any thoughts?

Side note: I don't seem to have either of those parameters set by default. I suppose that's expected too.

@stejskalleos
Copy link
Contributor

Personally I don't think this is a great user experience. I would prefer to keep the default false.
cc @stejskalleos since global registration is yours, any thoughts?

I agree. We already have similar issues with enabling remote execution pull provider but not having required repos in the activation key.
Users should explicitly set the parameter to true; we should not do that secretly somewhere in the templates IMHO.

@sbernhard
Copy link
Contributor Author

Personally I don't think this is a great user experience. I would prefer to keep the default false.
cc @stejskalleos since global registration is yours, any thoughts?

I agree. We already have similar issues with enabling remote execution pull provider but not having required repos in the activation key. Users should explicitly set the parameter to true; we should not do that secretly somewhere in the templates IMHO.

Well, we do expect this already here: https://github.com/theforeman/foreman/blob/develop/app/views/unattended/provisioning_templates/snippet/redhat_register.erb#L81

So, it would just be to behave in the same way.

As a user, I would expect that there is no difference from a host how it is attached to a foreman/katello whether it is provisoned from foreman/katello or registered by Host Registration. Especially which foreman/katello tools are installed or not.

@stejskalleos
Copy link
Contributor

Well, we do expect this already here: https://github.com/theforeman/foreman/blob/develop/app/views/unattended/provisioning_templates/snippet/redhat_register.erb#L81

A few lines below, we have it without the true default, so I consider this example as a bug, IMHO.

@sbernhard
Copy link
Contributor Author

A few lines below, we have it without the true default, so I consider this example as a bug, IMHO.

The default is "true" if there is a activation key uses for this host. In case of host provisioning with katello, activation keys and subscription manager installation, the katello-host-tools are installed by default.

If kt_activation_key is not set, katello-host-tools is not installed by default -which make sense.

As tracer and host tools are "common" katello features, I would expect that both variables are default = true. You want to install the necessary tools so that the host works together with the katello features.

@stejskalleos
Copy link
Contributor

I must admit that I find the logic confusing; the activation keys are used in both cases:

    if host_param('kt_activation_keys')
      ...
      activation_key = host_param('kt_activation_keys')
      redhat_install_host_tools = host_param_true?('redhat_install_host_tools', true)
      ...
    else
      ...
      activation_key = host_param('activation_key')
      redhat_install_host_tools = host_param_true?('redhat_install_host_tools')
      ...
    end

So, the logic is to install host_tools only if Katello is present.
But this still won't work unless you specify additional repositories (for the registered host) from which you can install the host_tools, am I right?

If it's true, then it's a bug.

@sbernhard
Copy link
Contributor Author

If it's true, then it's a bug.

This section in redhat_register.rb exists since years - before community-templates were integrated in theforman core: theforeman/community-templates@8fb3721

Is it a bug or a feature? :-)

@ekohl
Copy link
Member

ekohl commented Nov 6, 2024

So, the logic is to install host_tools only if Katello is present.

I think there's logic to that: you don't install katello-host-tools if you're subscribing to the Red Hat CDN.

@sbernhard
Copy link
Contributor Author

sbernhard commented Nov 11, 2024

So, how should we proceed with this?
a) don't change anything
b) use this PR (and have default = true)
c) adapt the redhat_register.rb to use default = false and close this PR.

I guess you know that I vote for b)

@stejskalleos
Copy link
Contributor

Voting for: c

@stejskalleos
Copy link
Contributor

Or we could start a discussion on the community as well to see the opinions of others.

@sbernhard
Copy link
Contributor Author

Whats your opinion @ekohl / @jeremylenz / @ianballou ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants