SyncPeriod does not work if controller is created WithEventFilter(predicate.GenerationChangedPredicate{})

[patrickshan]

Basically this just reopens this old issue which hasn't been addressed yet: #1990

[sbueringer]

@patrickshan should we just reopen the other issue instead?

[troy0820]

/kind bug support

This bug was filed against v0.12.3, did this get fixed with v0.15.0?

[sbueringer]

The behavior described is expected and not a bug.

Every SyncPeriod the underlying informers will emit events to trigger reconciliation of all objects. But if there are predicates configured that filter out those events, accordingly the Reconcile func won't be executed.

Potentially we could improve our documentation there, but the behavior itself is intended

[jjchambl]

@sbueringer is there a way to filter for the sync trigger? The fact that it's undocumented makes it surprising for many relying on SyncPeriod. I don't think it's unreasonable that some would want their controller to reconcile on a regular schedule in addition to responding immediately to certain CRD changes.

[sbueringer]

I don't think it's unreasonable that some would want their controller to reconcile on a regular schedule in addition to responding immediately to certain CRD changes.

It's not unreasonable, but if the controller is configured to filter out events if the generation is not changed it will do exactly that.

As far as I'm aware there is no way to filter based on the "sync trigger". Basically this information is not propagated to where the predicate is called. I think this information is not available outside of the informer which is a component from client-go that we use.

[sbueringer]

I think without bigger refactorings across client-go and controller-runtime it is simply impossible to expose what triggered a reconcile. It also doesn't really fit with the current design e.g. because we de-duplicate objects in the reconcile queue. I.e. if you have multiple update events ~ at the same time on the same object it won't get reconciled for every single event.

If I would want to implement this I would implement a regular controller for the regular case and then just simply start a go routine with a ticker to periodically list and reconcile all objects.

[jjchambl]

@sbueringer you mentioned the documentation is sparse on this topic; is there any I can go to for an idea on how specifically controller-runtime handles this now? It would help us in our implementation as well as give us a point to add to them based on information gaps.

[sbueringer]

To be honest, not sure if we have any documentation about it apart from godocs across the code base

[jjchambl]

I can look through the code then. Any pointers to which section (I'm obviously new to it lol)

[sbueringer]

A good starting point is probably here:

controller-runtime/pkg/builder/controller.go

Line 227 in e644e50

func (blder *Builder) Build(r reconcile.Reconciler) (controller.Controller, error) {

There you can see how a controller is wired together.

Another interesting point is what happens on manager start:

controller-runtime/pkg/manager/internal.go

Line 318 in e59161e

func (cm *controllerManager) Start(ctx context.Context) (err error) {

From there it goes pretty deep down the rabbit hole until client-go primitives like the shared index informer.

It might be easies to step through there with a debugger to see how everything works at runtime.

[jjchambl]

Sounds good, thanks!

[jjchambl]

Okay, I'm back with a working solution. For full context, my team makes extensive use of the status property to set and manage the state of CRs. For that reason we implemented predicates to filter events in which the status property was modified, and as mentioned above this also filters out events triggered by the SyncPeriod.

One property we set in the status is LastSyncTime, a timestamp indicating when the CR was last reconciled. With this info we were able to implement a custom predicate function that compares the LastSyncTime with the current time of a triggering event. If the delta between the two is larger than the SyncPeriod, then we return true to enqueue the object for reconciliation. Example below. I'd be interested to know if this is a recommended alternative if developers need to support both filtered events and a global sync ticker.

// resync returns true if the specified resource has a status indicating that
// it has not been reconciled for at least the global constant SyncPeriod.
func resync(object client.Object) bool {
	objectType := reflect.TypeOf(object)
        // our custom CR with extended status property
	objectWithStatusInterface := reflect.TypeOf((*commonv1.ObjectWithStatus)(nil)).Elem()
	if !objectType.Implements(objectWithStatusInterface) {
		return false
	}
	objectWithStatus := object.(commonv1.ObjectWithStatus)
	lastSyncTime := test.ExtractLastSyncTime(objectWithStatus)
	syncPeriod := int64(SyncPeriod.Seconds())
	now := time.Now().Unix()
	return lastSyncTime >= 0 && now-lastSyncTime >= syncPeriod
}

[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