Add lifecycle hooks to EventSource

[lburgazzoli]

The class io.javaoperatorsdk.operator.processing.event.EventSource does not provide any lifecycle method, so there is a chance a watch is registered and an event is received before an EventHandler is registered.

As example, the memcached example register a watcher before registering the EventSource.

I think it would be nice to have some sort of start/stop hook the implementors can use to safely start watch.

[csviri]

Hi @lburgazzoli , yes this seems to be reasonable.

Not sure what you mean by " event is received before an EventHandler is registered ". You mean the controller?

But the current intention is that the EventSources are registered in the init method of the controller. At this point when an event would be produced it won't be missed by the system. Have to check , but you might be right that if the instantiation of the event source happens before (for some reason) and it would produce an event, this would lead to an undefined behavior (I guess a nullpointer expception :D) .

So this is a good point.

I was thinking also in other events for the event source, like when for example a custom resource is first time processed, an event source could be notified or things like that. But the plan is to add these events ad-hoc, based on some real use-cases.

(FYI see init call: https://github.com/java-operator-sdk/java-operator-sdk/blob/112a8268155a9f6262e1238b592e9813b09e1557/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java#L150 )

[lburgazzoli]

Not sure what you mean by " event is received before an EventHandler is registered ". You mean the controller?

So usually when an EventSource is created, you do something like:

    client.apps().deployments().inAnyNamespace().withLabels(labels).watch(this)

But if the resource changes (in this case a deployment) before the eventHandler is set to the event source, there is a change either to miss the event or trigger an NPE (at least the memcached example would trigger an NPE). To solve this, in my code I do override AbstractEventSource::setEventHandler and I start watching only after the event handler has been set, like:

    @Override
    public void setEventHandler(EventHandler eventHandler) {
        super.setEventHandler(eventHandler);
        watch();
    }

Having some lifecycle method would make it clear when it is possible to start watching.

[csviri]

Ahh I see how do you mean it, yes this seems to be a valid point, to avoid doing such workarounds with event handlers. Also the stop would be useful to be able to do graceful shutdown.

[lburgazzoli]

I started having a look at this but I found that it is quite easy to add a start method but adding a stop one does not seems very simple as I do not see a clear shutdown process (i.e. there is a Operator.start but not Operator.stop/close). Also the EventSourceManager has a register method, but lacks the deRegister one.

@csviri how does the operator handle shutdown logic ?