Align on how to configure mandatory plugins (and whether they need to be modeled as plugins in the first place)

[ahg-g]

See full discussion in #981

My thinking is to continue to model mandatory features as plugins and have a "framework" for configuring them via code (i.e., not via the config api and so they would not be configured by users).

An initial cut would be as simple as a list of filters that get prepended to all profiles. Since this in code, we can evolve this slowly and by reacting to new needs, it doesn't need to be fully fleshed out from the beginning since it is not use facing.

[nirrozenbaum]

I have a strong preference not to implement mandatory features as plugins.

This may introduce a lot of complexity to the code. while it was claimed in the original discussion that it will make the core code lighter, I strongly disagree on this point. I think this will require to add the mechanics to do it (add core features as plugins after parsing the configuration), and will have to include the handling of multiple edge cases. some of those can be:

• a feature that is implemented as plugin has a name/type, but whoever configures GIE is not aware of it so he might choose to use the same type/name. so we may have collisions in names. is it solvable? sure. we may return an error that this name is preserved for system use. is it expected from a UX perspective? I think not.
• it does "magic tricks". while the user who configures GIE is sure he understands exactly which plugins are running and when, this adds behind the scenes additional plugins. this may affect debugging (if I want to grep the log for all filters for example) and other developer experience.
• it may introduce duplications and major inefficiencies when running multiple profiles which in turn may affect serving time.
  GIE upstream currently has only one profile, but it may not be the case in any other scheduler or in GIE in the near future. as I stated on the original issue:
  in case subset is implemented via filter and we have multiple scheduling profiles - this filter has to run in each.
  so if for example we have 4 profiles, 10000 candidate pods, and we specify in the subset key a list of 10 pods to choose from - we will need to filter the 10 pods out of the 10000 candidates 4 times.
• it also splits core request related functionality into multiple places making it harder to read and follow the code and to understand why things happen.

I would like to use subset filter as a representative example:
implementing it using a filter plugin includes all the disadvantages I pointed out above.
on the other hand, implementing it as a simple function in the director keeps it clean, readable and maintainable with none of the issues mentioned earlier. see example here:
https://github.com/nirrozenbaum/gateway-api-inference-extension/blob/subset-alternative/pkg/epp/requestcontrol/director.go#L145

This code filters the pods once before calling scheduler in a simple 30 LoC function. These are the same 30 LoC that were used to implement subset as a filter. just that now none of the mechanics around it is needed.

to summarize:
is it possible to implement core feature using plugins? yes.
having said that, I think this is not the right choice and will add a lot of extra code and complexity that is completely unnecessary. I don't see any clear advantage for implementing core features as plugins.
I think the recommendation for when to use plugins should be either for implementing optional features or for customized logic.

[ahg-g]

a feature that is implemented as plugin has a name/type

This is not a real issue, we need to generally establish a common practice for plugins names/types since this can happen regardless; perhaps using a "domain" (e.g., mandatory plugins always start with system/ while the configurable default ones start with default/.

it does "magic tricks"

I don't think so. The code has to be executed regardless, so no surprises on behavior; those plugins are not hidden, they are just not configurable, and so they would be documented; In fact, as plugins, it will be easier to document the EPP behavior since we would be having a common structure for documentation. Debugging may also improve, again because the user would use the same approach to debug any plugin (filtering logs based on plugin name for example).

it may introduce duplications and major inefficiencies when running multiple profiles

Right, a plugin would need to expect that it may run multiple times on different profiles and utilize CycleState to cache any state they don't need to compute on each call. This adds complexity to the plugin implementation.

it also splits core request related functionality into multiple places making it harder to read and follow the code and to understand why things happen.

The way I think about it is that a reader of the code should be aware of the concept of mandatory plugins, they should be easy to lookup.

Having said that, I don't really hold a very strong opinion on this at the moment, while I do see benefits of establishing a pattern of implementing most things as plugins since in my view it will help standardize aspects of code structure and logging, but I wont push back if we don't want to pursue this now.

[nirrozenbaum]

If there is no objection, I’d like to convert the subset filter to a helper function in the request control layer (in director) as described above.

we can continue the discussion and if we eventually decide to have system plugins, we should implement the mechanics around it.

@ahg-g wdyt?

[ahg-g]

sounds good

[elevran]

Whether we hard code a function implementation for a mandatory behavior or use plugins/callbacks seems to be an internal architecture design choice. If we expect the behavior to be extended in implementations - then a ("system") plugin might be a good choice. For example, we can have a fixed Subset function (based on the header) run upon creating snapshots of data layer endpoints or we can have a list of endpoint subset plugins that are registered and called, with the subset based on header always registered and run first.

These should be clearly decoupled from user's Plugin's so they're not accidently manipulated.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale