Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts #10804

jeremylenz · 2023-11-16T18:04:48Z

What are the changes introduced in this pull request?

As we know, currently

full load balanced setups are not supported
Smart proxy load balancing is only supported for Pulp content, not other smart proxy services
When registering a host, you pass the --serverurl parameter to subscription-manager, which contacts the server you specify for registration.
In such a case, the host's registered_through value is set to the hostname of the load balancer, which is not a smart proxy.
Therefore, the registered_through value won't match the name of any known Smart Proxy.
Previously, when calculating remote execution smart proxies available to the host, that caused the host's registered_through Smart Proxy to be listed as [], breaking Remote Execution on certain setups depending on your settings.

With this change, some methods are added to attempt to surmise

when you're registering to a load-balanced smart proxy;
when a certain smart proxy is behind a load balancer; and
which smart proxies are behind a given load balancer.

In the case of smart proxies, this is determined by comparing the smart proxy's registration_url hostname with its url hostname. If they're different, we assume it's a load balanced setup.

In the case of host registration, this is still determined (for now) by comparing the request headers to known smart proxy names.

Additionally, we add the following behavior changes when a host is registered through a load balancer:

Set the host's remote execution registered_through to all known smart proxies behind the load balancer
Set the host's content source to the first found smart proxy behind the load balancer. This should be safe because (theoretically) all smart proxies behind the load balancer should be syncing the same content. It's also better than setting content source to nil, because nil assumes the host has access to all library content, when actually it only has access to content synced by its smart proxy(ies).

Considerations taken when implementing this change?

Ideally a load balancer would be a Foreman entity which holds a list of smart proxies, and the user would explicitly tell us which smart proxies go with which load balancer, and we could just refer to that. But that would require a rather large effort, so this is being offered as somewhat of a stopgap to fix the immediate bug. None of the changes here preclude a more proper feature design from happening later.

What are the testing steps for this pull request?

I actually haven't tested this with a load balancer yet. You may leave my hard-codings in, or ideally set up an actual load balancer and take them out.

app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb

ehelms · 2023-11-16T21:34:26Z

app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb

+
+    def registering_thru_load_balancer?(hostname)
+      # If the request doesn't come from any known smart proxy, it came from a load balancer.
+      get_content_source_id(hostname).nil?


Is this a safe assumption? Could you instead check all smart-proxies registration URLs and see if they match the hostname?

Yeah, that would be better. At this point I should probably just change it not to use the header, too..

Updated to check if hostname is a known load balancer. 👍

app/models/katello/concerns/smart_proxy_extensions.rb

ehelms · 2023-11-16T21:38:35Z

@ekohl given you had a lot of opinion around the implementation where we used registration_url to indicate a load-balancer rather than introducing a dedicated parameter to indicate load-balancing I think you should review this.

…ering hosts

jeremylenz · 2023-11-21T21:20:49Z

Added some tests; marking ready for review.

adamruzicka

I don't have a setup with load balanced capsules so I can't really test this, but in general this reads like it should do the trick, assuming my mental model of the whole thing is correct.

sayan3296 · 2023-12-08T08:33:55Z

@adamruzicka @jeremylenz I have done some testings on a Sat\Cap 6.15 + Haproxy setup with two RHEL clients .. The results are looking great.

Details can be found in the private comment 11 of the main Bugzilla https://bugzilla.redhat.com/show_bug.cgi?id=2240956

ianballou

After poking around the rails console on a load-balanced set up, this is looking good to me! Credit to @sayan3296 for the setup.

app/models/katello/concerns/host_managed_extensions.rb

missed the logs

ianballou

Just the log statements need cleaning then this should be good to merge.

evgeni · 2023-12-11T16:55:44Z

lib/katello.rb

@@ -6,6 +6,7 @@
 require "oauth"
 require "gettext_i18n_rails"
 require "foreman-tasks"
+require "foreman_remote_execution"


Given

katello/katello.gemspec

Line 37 in 6d8d3ca

gem.add_dependency "foreman_remote_execution", ">= 7.1.0"

I am rather confident we never have a Katello install w/o REX.

But at the same time it indicates that the way we load stuff differs so heavily in prod/dev/test, that things go weird.

https://community.theforeman.org/t/requiring-rex-in-katello-4-0/19582 indicates that dep is here to stay :)

ianballou

Tests are good, so ACK!

…ering hosts (Katello#10804) * Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts (cherry picked from commit 27e9e68)

…ering hosts (#10804) * Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts (cherry picked from commit 27e9e68)

pr-processor bot added the Not yet reviewed label Nov 16, 2023

jeremylenz requested review from adamruzicka, ehelms, parthaa and wbclark November 16, 2023 18:10

ehelms reviewed Nov 16, 2023

View reviewed changes

app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb Outdated Show resolved Hide resolved

ehelms reviewed Nov 16, 2023

View reviewed changes

app/models/katello/concerns/smart_proxy_extensions.rb Outdated Show resolved Hide resolved

jeremylenz force-pushed the 36928-load-balancers-whoa branch from e4139f2 to 4245df0 Compare November 17, 2023 14:49

jeremylenz added 2 commits November 21, 2023 15:25

Fixes #36928 - (sorta) handle load balanced smart proxies when regist…

8851de5

…ering hosts

Refs #36928 - remove hard-coded values

46a24de

jeremylenz force-pushed the 36928-load-balancers-whoa branch from 4245df0 to 46a24de Compare November 21, 2023 20:28

jeremylenz marked this pull request as ready for review November 21, 2023 20:28

Refs #36928 - add remote_execution_proxies tests

168cb30

adamruzicka reviewed Nov 24, 2023

View reviewed changes

ehelms approved these changes Nov 28, 2023

View reviewed changes

pr-processor bot removed the Not yet reviewed label Nov 28, 2023

pr-processor bot added the Waiting on contributor label Dec 7, 2023

jeremylenz force-pushed the 36928-load-balancers-whoa branch from 53f81be to a865133 Compare December 7, 2023 21:15

pr-processor bot added the Needs re-review label Dec 7, 2023

ianballou previously approved these changes Dec 8, 2023

View reviewed changes

app/models/katello/concerns/host_managed_extensions.rb Outdated Show resolved Hide resolved

pr-processor bot removed the Needs re-review label Dec 8, 2023

ianballou requested changes Dec 8, 2023

View reviewed changes

jeremylenz force-pushed the 36928-load-balancers-whoa branch from a865133 to ed31c3b Compare December 8, 2023 21:09

pr-processor bot added Needs re-review and removed Waiting on contributor labels Dec 8, 2023

jeremylenz force-pushed the 36928-load-balancers-whoa branch from ed31c3b to 4254bde Compare December 11, 2023 14:55

Refs #36928 - fix tests

a0ddb54

jeremylenz force-pushed the 36928-load-balancers-whoa branch from 4254bde to a0ddb54 Compare December 11, 2023 16:48

evgeni reviewed Dec 11, 2023

View reviewed changes

ianballou approved these changes Dec 11, 2023

View reviewed changes

pr-processor bot removed the Needs re-review label Dec 11, 2023

jeremylenz merged commit 27e9e68 into Katello:master Dec 11, 2023
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts #10804

Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts #10804

jeremylenz commented Nov 16, 2023 •

edited

Loading

ehelms Nov 16, 2023

jeremylenz Nov 17, 2023

jeremylenz Nov 17, 2023

ehelms commented Nov 16, 2023

jeremylenz commented Nov 21, 2023

adamruzicka left a comment

sayan3296 commented Dec 8, 2023

ianballou left a comment

ianballou left a comment

evgeni Dec 11, 2023

evgeni Dec 11, 2023

ianballou left a comment

Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts #10804

Fixes #36928 - (sorta) handle load balanced smart proxies when registering hosts #10804

Conversation

jeremylenz commented Nov 16, 2023 • edited Loading

What are the changes introduced in this pull request?

Considerations taken when implementing this change?

What are the testing steps for this pull request?

ehelms Nov 16, 2023

Choose a reason for hiding this comment

jeremylenz Nov 17, 2023

Choose a reason for hiding this comment

jeremylenz Nov 17, 2023

Choose a reason for hiding this comment

ehelms commented Nov 16, 2023

jeremylenz commented Nov 21, 2023

adamruzicka left a comment

Choose a reason for hiding this comment

sayan3296 commented Dec 8, 2023

ianballou left a comment

Choose a reason for hiding this comment

ianballou left a comment

Choose a reason for hiding this comment

evgeni Dec 11, 2023

Choose a reason for hiding this comment

evgeni Dec 11, 2023

Choose a reason for hiding this comment

ianballou left a comment

Choose a reason for hiding this comment

jeremylenz commented Nov 16, 2023 •

edited

Loading