vshn / espejo

OpenShift and Kubernetes Object Syncer
BSD 3-Clause "New" or "Revised" License
11 stars 2 forks source link

Limit scope of namespace-triggered reconciles #71

Closed zugao closed 3 years ago

zugao commented 3 years ago

Summary

As cluster-admin\ I want to limit the scope of a namespace-triggered reconcile of SyncConfigs to the source namespace\ So that I can reduce the load on the Kubernetes API on large clusters.

Context

The namespace watcher feature of Espejo has a flaw. On every namespace update, the SyncConfigs are reconciled for all other namespaces as well. This situation is expensive on big clusters (1000+ dynamic namespaces), especially combined with a low --recycling-interval. It should be possible to limit SyncConfig reconcile to only the namespace where the trigger came from. Increasing the interval on clusters with highly dynamic namespaces doesn't matter anymore, since espejo is reconciling all the time triggered by namespace event.

Out of Scope

Further links

Acceptance criteria

Implementation Ideas

ccremer commented 3 years ago

I've allowed myself to edit your comment a bit.

I've had an implementation idea how to limit reconciles to a single namespace:

  1. Remove the namespace event trigger for SyncConfigReconciler and make a dedicated namespace_controller instead. (so that a namespace event will not directly trigger a SyncConfig reconcile)
  2. Add a new field to the SyncConfigReconciler struct with the name of the namespace that it should be limited. For normal, or interval-triggered reconciles, this property is empty, meaning that all namespaces should be targeted. That is the current behaviour. (Don't ignore namespace selector though, might end up in a no-op)
  3. On a namespace event, the namespace controller will dynamically create a new SyncConfigReconciler and set the namespace property to the namespace name that was updated. Call Reconcile(...) on the SyncConfigReconciler for all SyncConfigs found in the espejo namespace (from within the namespace controller reconcile func).
  4. Modify filterNamespaces in https://github.com/vshn/espejo/blob/master/controllers/syncconfig_utils.go#L46, so that it only returns the namespace from the given property if it is non-empty.
  5. The SyncConfigReconciler will now only update the namespace where the event came from, leaving the other ones untouched. This allows us to preserve the namespace selector in the spec.

Thoughts?

srueg commented 3 years ago

I like this approach. Probably it doesn't even need a custom controller, just an adapted handler.EnqueueRequestsFromMapFunc function: https://github.com/vshn/espejo/blob/master/controllers/syncconfig_controller.go#L64

ccremer commented 3 years ago

I also thought of that, but I'm a bit afraid of side effects when modifying the same SyncConfigReconciler that is also used for normal reconciliations. The Map function basically translates the namespace event into a list of reconcile requests for the sync configs in the namespace. But then the controller-runtime calls Reconcile at later, unknown time in an unknown order in a queue (from the point of the Map function). How exactly would you modify the Map function so that the Reconcile function respects only a single namespace?

With a dedicated controller, we probably don't need the map function anymore, resp. much of that logic will be part of the controller anyway.

ccremer commented 3 years ago

I have edited the issue a bit