-
Notifications
You must be signed in to change notification settings - Fork 220
Event Notification and Polling, Inbound Event Sources #720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 21 commits
2351e3f
058475c
64e27d0
766df4f
36844c4
5c2d87e
ff140d0
ddc0b37
5b3dcdb
9e2e741
193e119
09e69fc
3b97b6a
a26842d
814c707
629c12f
b4ddaae
89272b4
31224ee
c008dc0
e5b32f6
20ebbb5
aab7d04
fca3661
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
package io.javaoperatorsdk.operator.processing.event.source; | ||
|
||
import java.util.Optional; | ||
|
||
import javax.cache.Cache; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import io.javaoperatorsdk.operator.OperatorException; | ||
import io.javaoperatorsdk.operator.processing.event.Event; | ||
import io.javaoperatorsdk.operator.processing.event.ResourceID; | ||
|
||
/** | ||
* Base class for event sources with filtering and caching capabilities. | ||
* <p> | ||
* {@link #handleDelete(ResourceID)} - if the related resource is present in the cache it is | ||
* removed. and event propagated. There is no event propagated if the resource is not in the cache. | ||
* <p> | ||
* {@link #handleEvent(Object, ResourceID)} - caches the resource if changed (or nto present | ||
* before). Propagates an event if the resource is new or not equals to the one in the cache, and if | ||
* accepted by the filter if one is present. | ||
* | ||
* @param <T> represents the resource (usually external non-kubernetes one) handled. | ||
*/ | ||
public abstract class CachingFilteringEventSource<T> extends LifecycleAwareEventSource { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(CachingFilteringEventSource.class); | ||
|
||
protected Cache<ResourceID, T> cache; | ||
protected EventFilter<T> eventFilter; | ||
|
||
public CachingFilteringEventSource(Cache<ResourceID, T> cache, EventFilter<T> eventFilter) { | ||
this.cache = cache; | ||
this.eventFilter = eventFilter; | ||
} | ||
|
||
protected void handleDelete(ResourceID relatedResourceID) { | ||
if (!isRunning()) { | ||
log.debug("Received event but event for {} source is not running", relatedResourceID); | ||
return; | ||
} | ||
var cachedValue = cache.get(relatedResourceID); | ||
cache.remove(relatedResourceID); | ||
// we only propagate event if the resource was previously in cache | ||
if (cachedValue != null | ||
&& (eventFilter == null || eventFilter.acceptDelete(cachedValue, relatedResourceID))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would you not want to propagate a delete event? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question. Wanted to make a generic filter, but this might be over engineered. We can remove the filters for now and see if someone is asking for it. |
||
eventHandler.handleEvent(new Event(relatedResourceID)); | ||
} | ||
} | ||
|
||
protected void handleEvent(T value, ResourceID relatedResourceID) { | ||
if (!isRunning()) { | ||
log.debug("Received event but event for {} source is not running", relatedResourceID); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logging doesn't make sense There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same :) |
||
return; | ||
} | ||
var cachedValue = cache.get(relatedResourceID); | ||
if (cachedValue == null || !cachedValue.equals(value)) { | ||
cache.put(relatedResourceID, value); | ||
if (eventFilter == null || eventFilter.accept(value, cachedValue, relatedResourceID)) { | ||
eventHandler.handleEvent(new Event(relatedResourceID)); | ||
} | ||
} | ||
} | ||
|
||
public Cache<ResourceID, T> getCache() { | ||
return cache; | ||
} | ||
|
||
public Optional<T> getCachedValue(ResourceID resourceID) { | ||
return Optional.ofNullable(cache.get(resourceID)); | ||
} | ||
|
||
@Override | ||
public void stop() throws OperatorException { | ||
super.stop(); | ||
cache.close(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package io.javaoperatorsdk.operator.processing.event.source; | ||
|
||
import io.javaoperatorsdk.operator.processing.event.ResourceID; | ||
|
||
public interface EventFilter<T> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The concept of event filter actually doesn't really make much sense to me… the events come from event sources which you write, so presumably, the event source decide of which events trigger the event handler so I'm not sure why we need to filter them again… There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, the idea was to separate that two thing, maybe to reuse some filters. But agree, I will remove filtering |
||
|
||
boolean accept(T newValue, T oldValue, ResourceID relatedResourceID); | ||
|
||
default boolean acceptDelete(T value, ResourceID relatedResourceID) { | ||
return true; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package io.javaoperatorsdk.operator.processing.event.source; | ||
|
||
import io.javaoperatorsdk.operator.OperatorException; | ||
|
||
public abstract class LifecycleAwareEventSource extends AbstractEventSource { | ||
|
||
private volatile boolean running = false; | ||
|
||
public boolean isRunning() { | ||
return running; | ||
} | ||
|
||
@Override | ||
public void start() throws OperatorException { | ||
running = true; | ||
} | ||
|
||
@Override | ||
public void stop() throws OperatorException { | ||
running = false; | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package io.javaoperatorsdk.operator.processing.event.source; | ||
|
||
import io.fabric8.kubernetes.api.model.HasMetadata; | ||
|
||
public interface ResourceEventAware<T extends HasMetadata> { | ||
|
||
default void onResourceCreated(T resource) {} | ||
|
||
default void onResourceUpdated(T newResource, T oldResource) {} | ||
|
||
default void onResourceDeleted(T resource) {} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logging doesn't make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The though was to make is easier on debug level to find out the life cycle of an event. If you prefer I can remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the message that's output… I'm not sure what it means :)