Registration - use registration_url for RHSM configuration #10370
Conversation
|
Issues: #35811 |
There was a problem hiding this comment.
This is not doing what you think it's doing. You're changing a dead code path and won't work without changes to the installer to STOP setting the rhsm_url.
Then comes the point: when do you set rhsm_url and when don't you. I'd say this leads to more problems than it solves so 👎 from my point of view.
| elsif pulp_primary? | ||
| URI("https://#{URI.parse(url).host}/rhsm") | ||
| URI("https://#{URI.parse(proxy_url).host}/rhsm") | ||
| elsif pulp_mirror? | ||
| URI("https://#{URI.parse(url).host}:8443/rhsm") | ||
| URI("https://#{URI.parse(proxy_url).host}:8443/rhsm") |
There was a problem hiding this comment.
These code paths should be unused since Foreman 3.1.
There was a problem hiding this comment.
I grok from that only the initial if statement is ever triggered now, and is really not needed anymore. That, with this idea this function could be shortened to:
def rhsm_url
url = setting('Registration', 'registration_url') || setting(SmartProxy::PULP3_FEATURE, 'rhsm_url')
URI(url)
end
There was a problem hiding this comment.
Correct that only the initial if is triggered. Still, big 👎 from my side on this.
There was a problem hiding this comment.
@ekohl so what would be the proper fix / update?
Can you expand on this? Sounds like the devil is in the details, but we need the details to understand why this won't work. |
|
The short answer is that we have a mechanism to declare what the RHSM URL is. I think we should really use that end-to-end. This PR effectively gets rid of the mechanism, which I think is a step back. It's something we should solve in our configuration layer (Puppet/installer in this case), not on the application layer. There we already have a registration URL setting - we could derive the value there and put it statically in the right Smart Proxy's YAML file. |
|
We discussed this a bit today -- this solution only covers half the use case and has the potential to leave some functional gaps (e.g. changing the content source). We are instead, considering this simpler approach: |
|
Okay then, I'm closing the PR and let's continue in theforeman/puppet-foreman_proxy_content#432 |
What are the changes introduced in this pull request?
Follow up to #10315, if Smart Proxy Registration module
is configured to load balancing (
registration_urlis set),use this value for RHSM configuration when registering host, see global registration
Considerations taken when implementing this change?
What are the testing steps for this pull request?