Add loadbalancer support

[ehelms]

When deploying the container gateway behind a load-balancer, the client is given a redirect to the location of the container image as reported by Pulp. This location known by Pulp contains the URL of the smart-proxy and has no knowledge of the load-balancer to send a proper redirect.

This changes introduces a new setting to specification of a loadbalancer when one is in use. If this is provided then the redirect will use that value to calculate the right URL to send back to the client for it to fetch manifests and blobs.

Could we have just used the existing pulp_endpoint setting? No. That setting is used by the container gateway itself to make requests to Pulp and if we set it to the load-balancer than all "internal" requests would route all the way out to and back through the load-balancer unnecessarily.

There is some duplicated code between the manifest endpoint and blob handling I intend to look at re-factoring.

The puppet class handling the container gateway, https://github.com/theforeman/puppet-foreman_proxy/blob/master/manifests/plugin/container_gateway.pp, would need to set this using the foreman_proxy::registration_url that we currently have users set when using a loadbalancer.

[ekohl]

I'm trying to capture our offline discussion here.

Essentially the problem is that we have a control plane (Foreman <-> Foreman Proxy/Pulp) and a user facing endpoint (User -> Pulp). It was designed that there was just a single URL, which is a false assumption.

I'd like to have a more generic naming: load balancer implies a certain implementation, but the specific implementation actually wasn't behind a load balancer. Instead it was in an Amazon VPC so it was closer to multi homing.

There was also a thought to reuse settings from smart_proxy_pulp but that currently doesn't have the split either so it wouldn't help.

[ianballou]

Since the config option naming came up:

Suggested change

	**settings.slice(:pulp_endpoint, :pulp_client_ssl_ca, :pulp_client_ssl_cert, :pulp_client_ssl_key, :loadbalancer)
	**settings.slice(:pulp_endpoint, :pulp_client_ssl_ca, :pulp_client_ssl_cert, :pulp_client_ssl_key, :client_content_endpoint)

This configurable option determines where we redirect the client to to get their requested content. So, I propose that the generic name be client_content_endpoint.

[ianballou]

If that's too generic we could do something like container_content_endpoint. Or maybe container_delivery_endpoint?

[ianballou]

I'm trying to capture our offline discussion here.

Essentially the problem is that we have a control plane (Foreman <-> Foreman Proxy/Pulp) and a user facing endpoint (User -> Pulp). It was designed that there was just a single URL, which is a false assumption.

I'd like to have a more generic naming: load balancer implies a certain implementation, but the specific implementation actually wasn't behind a load balancer. Instead it was in an Amazon VPC so it was closer to multi homing.

There was also a thought to reuse settings from smart_proxy_pulp but that currently doesn't have the split either so it wouldn't help.

@ekohl this makes it sound like the current approach is generally accepted with some small modifications. If that's the case, I was thinking of creating a community post soon with the plan here for visibility and comments.

[ekohl]

@ekohl this makes it sound like the current approach is generally accepted with some small modifications. If that's the case, I was thinking of creating a community post soon with the plan here for visibility and comments.

The pattern exists in multiple places. In the Smart Proxy we have the templates URL, but also the registration URL. There it exists for similar reasons (control plane + client facing URL).

It would be nice if we could somehow standardize it so there's less configuration required but until then ...

[ianballou]

From some outside chats, it sounds like I'm going to take this PR over from here.

[ehelms]

The pattern exists in multiple places. In the Smart Proxy we have the templates URL, but also the registration URL. There it exists for similar reasons (control plane + client facing URL).

It would be nice if we could somehow standardize it so there's less configuration required but until then ...

Are you thinking a core smart_proxy settings value that represent the client facing URL that plugins can then use?

[ekohl]

I would be tempted to. This particular case feels a bit more tricky because it's the Pulp URL, not Smart Proxy

[ehelms]

The current state of the art is that we define a client_facing_servername built from the --foreman-proxy-registration-url parameter. We then use that value to define a URLs / settings:

(registration_url)[https://github.com/theforeman/puppet-foreman_proxy/blob/48fc520f2cd5a4e14908293d9e72a8154c79a9cd/manifests/module/registration.pp#L15]
(pulpcore_content_url)[https://github.com/theforeman/puppet-foreman_proxy_content/blob/master/manifests/init.pp#L304]
(rhsm_hostname)[https://github.com/theforeman/puppet-foreman_proxy_content/blob/master/manifests/init.pp#L311]

So we are saying currently, if you want to set a loadbalancer, or a VPC, set the registration URL and then everything else will inherit this indirectly to configure client facing settings to the loadbalancer.

That could be good enough reason to define a smart_proxy setting that is client_url or client_hostname. Or for now, we have this plugin define a client_endpoint and set the value the same way described above.

[ianballou]

The current state of the art is that we define a client_facing_servername built from the --foreman-proxy-registration-url parameter. We then use that value to define a URLs / settings:

(registration_url)[https://github.com/theforeman/puppet-foreman_proxy/blob/48fc520f2cd5a4e14908293d9e72a8154c79a9cd/manifests/module/registration.pp#L15] (pulpcore_content_url)[https://github.com/theforeman/puppet-foreman_proxy_content/blob/master/manifests/init.pp#L304] (rhsm_hostname)[https://github.com/theforeman/puppet-foreman_proxy_content/blob/master/manifests/init.pp#L311]

So we are saying currently, if you want to set a loadbalancer, or a VPC, set the registration URL and then everything else will inherit this indirectly to configure client facing settings to the loadbalancer.

That could be good enough reason to define a smart_proxy setting that is client_url or client_hostname. Or for now, we have this plugin define a client_endpoint and set the value the same way described above.

Is it safe to say then that this new loadbalancer setting in the current working implementation is exactly the same as $client_facing_servername in the Puppet modules?

If the new setting is something generic like client_endpoint, I think it wouldn't hurt to have it defined in the main smart_proxy project.

If that is the case, then we could:

1. Create the client_endpoint config option in smart_proxy
2. Use that config option in this PR
3. Update puppet-foreman_proxy (or puppet-foreman_proxy_content?) to take client_facing_servername and put it in the smart_proxy config file.

Hopefully it's okay to assume that users have --foreman-proxy-registration-url set to something sane, otherwise the container_gateway could be redirecting them somewhere strange. Hopefully it's only used for the loadbalancing scenario?

[ehelms]

@ekohl Based on these latest comments, what is your opinion on where the setting should live?

[ehelms]

@ianballou let's just go with a local setting for now, we can re-think it in the future if a core setting that is useful comes along