-
Notifications
You must be signed in to change notification settings - Fork 991
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 #36747 - create new snippet for subscription-manager setup #9837
Fixes #36747 - create new snippet for subscription-manager setup #9837
Conversation
11b95da
to
a0fdcb8
Compare
app/views/unattended/provisioning_templates/snippet/redhat_register.erb
Outdated
Show resolved
Hide resolved
a0fdcb8
to
c43873a
Compare
server_hostname = "subscription.rhsm.redhat.com" | ||
server_port = "443" | ||
server_prefix = "/subscription" | ||
repo_ca_cert = "/etc/rhsm/ca/redhat-uep.pem" | ||
rhsm_baseurl = "https://cdn.redhat.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking out loud: is there a way to reset these to default values so we don't need to track these values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by 'reset to default'? From what I know, the default is empty values.
And where else will we use these vars, that we wouldn't need to track them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The registration process doesn't work for me, see:
curl -sS 'http://centos8-katello-devel-stable.example.com/register?activation_keys=default&location_id=2&organization_id=1&update_packages=false' -H 'Authorization: Bearer <token>'
returns:
echo "ERROR: standard_error";
echo "ERF42-7327 [Foreman::Exception]: The snippet 'subscription_manager_setup' threw an error: wrong number of arguments (given 2, expected 1)";
exit 1
app/views/unattended/provisioning_templates/registration/global_registration.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/redhat_register.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/redhat_register.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
Found the cause of the error: You need to pass the <%= snippet("subscription_manager_setup", variables: {existing_machine: true, subman_url: @rhsm_url }).strip %> Dont pass it as |
Actually, that didn't work for me.
|
c43873a
to
b2016ff
Compare
Oh yeah, looks like it's not allowed in the safe mode. |
I opened theforeman/safemode#46 almost a year ago. |
b2016ff
to
5740229
Compare
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
5740229
to
28d90cb
Compare
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
c678cb5
to
e5f2883
Compare
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
update-ca-trust | ||
fi | ||
|
||
# Restart yggdrasild if installed and running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be this part of the if registration_method == 'provisioning' && plugin_present?('katello')
block?
yggdrasild
is used for REX pull provider, I guess people can use it without Katello and also set it up during the registration, not just provisioning.
@adamruzicka just want to confirm, can you use pull provider without Katello?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use it without katello as long as you bring your own certs and set it up yourself, although I'm not sure if anyone does that. To be strictly correct, the restart should be outside the block.
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
e5f2883
to
9368c09
Compare
9368c09
to
d8d43a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments otherwise LGTM,
registration works as expected, and provisioning as well, I tried RHEL 8 & 9 without katello and hosts were successfully provisioned.
QAs will test provisioning with Katello.
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left you a couple of suggestions from the documentation perspective. (You don't have to incorporate all of them, unless you think they would be an improvement to the code.)
app/views/unattended/provisioning_templates/snippet/redhat_register.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/redhat_register.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/registration/global_registration.erb
Outdated
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Show resolved
Hide resolved
app/views/unattended/provisioning_templates/snippet/subscription_manager_setup.erb
Outdated
Show resolved
Hide resolved
d8d43a1
to
34635ad
Compare
chmod 644 $KATELLO_SERVER_CA_CERT | ||
<% end -%> | ||
|
||
# Prepare subscription-manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stejskalleos should we check if the machine is RHEL compatible?
For example, here we are calling the redhat_register
snipped without verifying the machine's RHEL compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need, kickstart is specifically for RHEL, if someone is going to use the snippet somewhere else, I expect that the person knows what is doing.
app/views/unattended/provisioning_templates/registration/global_registration.erb
Show resolved
Hide resolved
34635ad
to
de2f7f6
Compare
@stejskalleos @nofaralfasi I've tested this PR , It's not working as expected. |
9a9641c
to
ffa7a1b
Compare
@shubhamsg199: I have resolved the issue, and it should now function as expected. |
@nofaralfasi I will test this and update |
@nofaralfasi Things are working as expected now. |
fi | ||
fi | ||
# Select package manager for the OS (sets the $PKG_MANAGER* variables) | ||
<%= snippet 'pkg_manager' -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stejskalleos can we skip this line, since we are checking if PKG_MANAGER
variable is set in the new snippet lines 13-14?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if someone will use the snippet only in their templates? Then it won't work ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work, because we are checking it in the beginning of the new snippet - lines 13-14.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. Well, it needs to be rendered at least once, before the variable is used, right ... if that is what's happening, then it's fine and it can be removed
e6ffcbe
to
4832a14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM docs-wise. Ping me if you need another review.
This snippet replaces the use of the katello-ca-consumer RPM for host registration. It incorporates the necessary code from the Global Registration template to streamline subscription-manager configuration during provisioning.
4832a14
to
96a6bec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍏 LGTM,
Code looks sane, QAs confirmed functionality, so let's get this in!
Thanks @nofaralfasi @shubhamsg199 and others! |
This snippet replaces the use of the katello-ca-consumer RPM for host registration. It incorporates the necessary code from the Global Registration template to streamline subscription-manager configuration during provisioning.