Support predicate based filtering of events

[shrinandj]

Based on current experiments, I see that the handler gets called for all the operations on the CR (add, update and delete). However, there is no information about which of the operations resulted in the handler getting invoked.

Are there plans to add more context to the types.Event object (or some other mechanism) to distinguish between various operations?

[hasbro17]

@shrinandj Nothing as of yet. In fact with Finalizers we're hoping to remove the Delete events altogether. See #270

Did you have a use case in mind for handling Add/Update events separately? Normally an operator's handler should be able to handle both without distinguishing between the two.

[shawn-hurley]

Should this be defined by the resources status field?

i.e https://github.com/operator-framework/operator-sdk-samples/blob/master/vault-operator/pkg/vault/reconcile.go#L23

[blaggacao]

@shawn-hurley I understand, that not necessarily and if so, with care

[hasbro17]

@shawn-hurley Possibly yes. It depends on how you want to handle the setup/initialization for a newly added CR.

So for e.g you initially do the setup for a new CR and switch the phase from Initializing==>Running so that upon future reconciles you skip the setup phase. But if something from the initial setup goes out of sync how do you handle that? If it's not even possible to reconcile at that point you could switch to the phase Failed and stop.

Or you can do away with the Phases and reconcile everything from scratch and not distinguish between a newly created CR vs an existing one.

In either case you won't need to distinguish between Add vs Update events.

[blaggacao]

@hasbro17 I wonder though how to query previous (asserted) state during an update? Or are my update funtions supposed not to be depending on state (which is expensive in code and maybe resources if I need to query actual state in downstream application logic)?

Use case: I want to instanciate a postgres namespace in a given postgres cluster, which basically consists of a user name and some quota limits. Now I want to update quota....

Maybe I'm just missing something trivial, so sorry for that possibility.

[hasbro17]

@blaggacao The operator should act upon the current state of the world and not rely on any previously seen state.
This refers to the level driven vs edge driven convention used in k8s controllers. See point number 3 https://github.com/kubernetes/community/blob/master/contributors/devel/controllers.md#guidelines

[shrinandj]

It's good to learn that operators should act upon current state (which implies that everytime the operator is invoked, it should query for the state) and not rely upon who/why it was invoked. I guess this also takes care of edge cases when a single update might invoke the operator more than once.

I don't have a specific use-case in mind yet.

I can close the bug unless others who've commented here have specific needs for distinguishing between add and update.

[killwing]

JobController will set expectations when a pod is added while not updated:
https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/job/job_controller.go#L214
How could I handle similar case like this if I can't distinguish between add and update?

[michalrabinowitch]

What about stateful application where you might need to know the last “good” configuration to determine if the desired conf is valid?

[orainxiong]

I think I could contribute a concrete user case.

What I need is similar to UpdateFunc of statefulsetController.

	setInformer.Informer().AddEventHandlerWithResyncPeriod(
		cache.ResourceEventHandlerFuncs{
			AddFunc: ssc.enqueueStatefulSet,
			UpdateFunc: func(old, cur interface{}) {
				oldPS := old.(*apps.StatefulSet)
				curPS := cur.(*apps.StatefulSet)
				if oldPS.Status.Replicas != curPS.Status.Replicas {
					glog.V(4).Infof("Observed updated replica count for StatefulSet: %v, %d->%d", curPS.Name, oldPS.Status.Replicas, curPS.Status.Replicas)
				}
				ssc.enqueueStatefulSet(cur)
			},
			DeleteFunc: ssc.enqueueStatefulSet,
		},
		statefulSetResyncPeriod,
	)

Sometimes, I need to compare the difference between newObj and oldObj at the very beginning to make debugging much easier.

Based on current operator-sdk, I add the callback interface to do the work.

type ResourceEventHandlerCallback interface {
	OnAdd(obj interface{}) error
	OnUpdate(oldObj, newObj interface{}) error
	OnDelete(obj interface{}) error
}

Meanwhile, kubebuilder also provides predicates which is equal to callback to allow developers to add their own logic(eg. filter).

I hope it could be helpful.

[hasbro17]

@orainxiong You're right. After taking a second look there seem to be quite a few use cases for comparing the old and new objects.
I actually like the predicates approach which is more general. In fact we're currently evaluating the controller-runtime library (which is based on kubebuilder's internals) to redesign the sdk controller components.
And we'll most likely end up using predicates to let users write their own filters for events.

[orainxiong]

@hasbro17

Agree with you. The predicates approach will be more general and give the user more flexibility.