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 #36747 - create new snippet for subscription-manager setup #9837

Merged

Conversation

nofaralfasi
Copy link
Contributor

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.

Comment on lines 82 to 86
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"
Copy link
Member

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?

Copy link
Contributor Author

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?

@nofaralfasi nofaralfasi marked this pull request as ready for review October 19, 2023 20:07
@nofaralfasi nofaralfasi changed the title [WIP] Fixes #36747 - create new snippet for subscription-manager setup Fixes #36747 - create new snippet for subscription-manager setup Oct 19, 2023
@stejskalleos stejskalleos self-assigned this Oct 23, 2023
Copy link
Contributor

@stejskalleos stejskalleos left a 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

@stejskalleos
Copy link
Contributor

Found the cause of the error: You need to pass the @rhsm_url to the snippet, something like this:

<%= snippet("subscription_manager_setup", variables: {existing_machine: true, subman_url: @rhsm_url }).strip %>

Dont pass it as rhsm_url: @rhsm_url, that won't work

@nofaralfasi
Copy link
Contributor Author

Found the cause of the error: You need to pass the @rhsm_url to the snippet, something like this:

<%= snippet("subscription_manager_setup", variables: {existing_machine: true, subman_url: @rhsm_url }).strip %>

Dont pass it as rhsm_url: @rhsm_url, that won't work

Actually, that didn't work for me.
I found that the issue was with the use of the '&' operator in the subscription_manager_setup snippet, as follows:

if plugin_present?('katello')
  server_hostname = @rhsm_url&.host
  server_port = @rhsm_url&.port
  server_prefix = @rhsm_url&.path

@stejskalleos
Copy link
Contributor

I found that the issue was with the use of the '&' operator

Oh yeah, looks like it's not allowed in the safe mode.

@ekohl
Copy link
Member

ekohl commented Oct 25, 2023

Oh yeah, looks like it's not allowed in the safe mode.

I opened theforeman/safemode#46 almost a year ago.

@nofaralfasi nofaralfasi force-pushed the bz_2153548_registration branch 2 times, most recently from c678cb5 to e5f2883 Compare October 30, 2023 20:45
update-ca-trust
fi

# Restart yggdrasild if installed and running
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

@stejskalleos stejskalleos left a 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.

Copy link

@Lennonka Lennonka left a 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.)

chmod 644 $KATELLO_SERVER_CA_CERT
<% end -%>

# Prepare subscription-manager
Copy link
Contributor Author

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.

Copy link
Contributor

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.

@shubhamsg199
Copy link

@stejskalleos @nofaralfasi I've tested this PR , It's not working as expected.
After the installation of the packages, it's not going to the registration section, It directly reboots and gives us the console. I also don't see the install.post.log file created to verify.

@nofaralfasi nofaralfasi force-pushed the bz_2153548_registration branch 2 times, most recently from 9a9641c to ffa7a1b Compare November 27, 2023 14:52
@nofaralfasi
Copy link
Contributor Author

@shubhamsg199: I have resolved the issue, and it should now function as expected.

@shubhamsg199
Copy link

@shubhamsg199: I have resolved the issue, and it should now function as expected.

@nofaralfasi I will test this and update

@shubhamsg199
Copy link

@nofaralfasi Things are working as expected now.
One thing to note is the subscription_manager_setup snippet which is rendered has unnecessary contents in the kickstart default template, which I think should not be present there

fi
fi
# Select package manager for the OS (sets the $PKG_MANAGER* variables)
<%= snippet 'pkg_manager' -%>
Copy link
Contributor Author

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?

Copy link
Contributor

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 ...

Copy link
Contributor Author

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.

Copy link
Contributor

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

@nofaralfasi nofaralfasi force-pushed the bz_2153548_registration branch 2 times, most recently from e6ffcbe to 4832a14 Compare November 29, 2023 16:51
Copy link

@Lennonka Lennonka left a 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.
Copy link
Contributor

@stejskalleos stejskalleos left a 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!

@stejskalleos stejskalleos merged commit dd26cc3 into theforeman:develop Nov 30, 2023
15 of 16 checks passed
@stejskalleos
Copy link
Contributor

Thanks @nofaralfasi @shubhamsg199 and others!

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.

6 participants