Smart Proxy Feature classes

Development RFCs
1

The goal of this PR is to figure out how to model Smart Proxy Feature classes in Foreman. You may notice the proposal is not very concrete. That’s because this is a very early stage RFC which seeks to gather input from people. It may not even be a good idea at all.

Background

Starting with the background. The Smart Proxy has features (dhcp, dns, tftp, etc). When a Smart Proxy is registered to Foreman, it stores which features it has found. In Foreman :: Foreman Proxy Registration Protocol v2 explained I’ve explained this in much more detail.

I’ve observed that how we model the code on the Foreman side is inconsistent, especially when we take plugins into account. That’s what I’d like to see solved.

If we look at the model layer, we have a SmartProxy database model with a has_many to SmartProxyFeature. SmartProxyFeature has a belongs_to relation with Feature.

For pure API communication, Foreman has proxy_api but then this is used all over the Foreman code. Most notable in models and services.

If we look at Katello, there’s a large smart_proxy_extensions module.

Proposal

I would like it if a plugin could register a Smart Proxy Feature.

# It probably shouldn't inherit from the SmartProxyFeature model class
class SmartProxyPulpFeature < BaseSmartProxyFeature
  # Whatever code needed
end

Foreman::Plugin.register :katello do
  smart_proxy_feature 'Pulp', SmartProxyPulpFeature
end

This means we would end up with a registry of features. This could then be used in database seeding as well.

From the SmartProxy instance you have a method get_feature(feature) to get an instance for that feature class (or features[feature], but that’s implementation details). Note that the ProxyAPI classes can stay and would be called from these feature classes.

The built in features (DHCP, DNS, TFTP, etc) would also get a specific class to give a uniform API.

One result of this is that it is clear what a feature provides and have clear isolation.

Capabilities

Feature classes would also be the right place to perform checks on capabilities. It should be possible to declare that a Smart Proxy Feature must provide some capability, or registration fails. This would allow API changes to happen by introducing a new capability. At some point Foreman can drop support for Smart Proxies without the new API.

For example, let’s say the DNS API has a bad design and needs a rework. It introduces a V2 API and signals this via the V2_API capability.

At first the code will look roughly like this (simplified):

class SmartProxyDnsFeature < BaseSmartProxyFeature
  CAP_V2_API = 'V2_API'

  def self.required_capabilities
    []
  end

  def create_record(name, type, value)
    if has_capability?(CAP_V2_API)
      ProxyAPI::DNSV2
    else
      ProxyAPI::DNS
    end
  end
end

This will allow Foreman to use both new and old, making it easy to upgrade. You can run Foreman 2.5 with a Foreman Proxy 2.4. Then in a later maintenance window, you upgrade to Foreman Proxy 2.5.

At some point Foreman 3.0 comes around and drops support for the old API. The class is now reduced to:

class SmartProxyDnsFeature < BaseSmartProxyFeature
  CAP_V2_API = 'V2_API'

  def self.required_capabilities
    [CAP_V2_API]
  end

  def create_record(name, type, value)
    ProxyAPI::DNSV2
  end
end

Whenever a SmartProxyDnsFeature is initialized, it checks the SmartProxyFeature instance for capabilities. If any of the required_capabilities are not present, it raises an exception. Additionally, any other checks mechanism should also be used. For example, the ping API endpoint can detect the incompatibility. On the Smart Proxy detail page it should also show the problem. The notifications framework can also be used at instance start up. Registration should also fail.

Implementing those checks means that as a user, you get a lot of up front warnings that your setup is going to give problems at some point in the future. In an upgrade procedure you can add a hammer ping at the end to verify everything is running and compatible.

3 Likes
2

Thanks for bringing this up @ekohl! In general, I like the approach.
There is one point i’m wondering about, which is do we actually need the features table (or an in-memory feature registration) at all? We could store the features (and even capabilities) on the proxy instance itself as a json field (so it is easy to search by), and the code using it can just check if there is a proxy with feature/capability X connected to whatever resource, or get all proxies with a specific feature etc… That would remove the need to pre-seed features and make sure that they are properly connected to a proxy.

3

I think that’s a good point. I do think a feature table is needed but it doesn’t have to be in database.

Perhaps not everybody knows this, but the installer does verify if all features requested are recognized by Foreman. That means that if you install REX on a proxy without the corresponding Foreman plugin, the installer will tell you about it. That is IMHO a good feature (pun not intended).

What I can imagine is that Foreman registration itself gains this ability. If you register a proxy and some features are not recognized, that is potentially a problematic situation. The user requested functionality but that doesn’t work. If we can provide a warning about that, it might help some struggling user.

For this, an in-memory table is probably sufficient which would drop the Feature table. I also think that SmartProxyFeature could be inlined to SmartProxy as long as it’s easily searched. Today we often query for all Smart Proxies with a specific feature. That should remain possible.

I would also argue that we should make it possible to query for proxies with a specific capability. Today Katello exposes a Pulp 3 mirror via a setting, but perhaps a capability is better. Also the various content types are exposed as capabilities. Another example is the BMC module which recently gained the Redfish provider. I just noticed we didn’t actually expose that as a capability, but if we did, you may want to query for Redfish capable proxies.

4

I decided to fix that myself:
https://github.com/theforeman/smart-proxy/pull/782

While writing it up, I also thought a bit more about the Foreman side and how to consume it, giving it’s actually a dynamic list. I’d be interested in @katello experiences here with the dynamic side of Pulp content types.

5

How would that remove the need ot pre-seeding features? We still need to have some list of well-known features because ton of our code expect it to exist. Also JSON is much less convinient to search using ActiveRecord, I mean you need to load all records, parse them. Not probably issue for few dozens of records, but why not to keep the correct SQL way (normal-form)?

I think there are two areas: to seed or not to seed (everything is dynamic) and to store as JSON or relation (which can also be dynamic or pre-seeded). I actually like the combination of simple SQL relation + dynamic (no seeding) the most.

6

I’m not sure what replacing SQL table of Features with JSON improves, so I’m neutral to that.

For the proposal in first post, I think that introducing a plugin DSL for defining a Feature and Capabilities is a good thing. Also having a proper object encapsulating the functionality. One really needs to open the code to see how the lack of this causes the code being mixed in a single module.

At the same time, I think Smart Proxy should still be able to announce features it has enabled and Foreman should display that even without Feature subclasses being defined by a plugin. So generic string representation of what’s present on a proxy is still required I think.

7

My proposal perhaps didn’t mention it explicitly, but this would end up in a Registry. That’s effectively the same as having a list in the database from a code point, but you don’t have to seed it.

1 Like