Skip to content

Fix generics #634

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

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5431289
chore(deps): bump micrometer-core from 1.7.4 to 1.7.5 (#606)
dependabot[bot] Oct 15, 2021
30dcf72
fix: re-schedule generics (#614)
csviri Oct 20, 2021
f9ee9b2
chore(deps): bump spring-boot.version from 2.5.5 to 2.5.6 (#617)
dependabot[bot] Oct 22, 2021
fb15a1e
chore(deps): bump awaitility from 4.1.0 to 4.1.1
dependabot[bot] Oct 26, 2021
f188473
fix: prevent double registration of same CR with different controllers
metacosm Oct 26, 2021
b437fef
Informer based CustomResourceEventSource and caching (#581)
csviri Oct 12, 2021
6a3503a
chore: update version to 2.0.0-SNAPSHOT
metacosm Oct 12, 2021
f4b6b7f
Reschedule delete (#600)
csviri Oct 13, 2021
1c9f257
Refined Interface of `EventSource` and `EventSourceManager` (#597)
csviri Oct 13, 2021
cb62f6c
Removing events from context (#596)
csviri Oct 13, 2021
7fc1c64
fix: cache handling on update (#604)
csviri Oct 20, 2021
60660ed
feat: Only one scheduled event (re-schedul / retry) at one time (#609)
csviri Oct 20, 2021
bea733d
feat: Cloner interface for Custom Resource instead of ObjectMapper (#…
csviri Oct 20, 2021
1a3558b
fix: EventSourceManager and ResourceController interface enhancements…
csviri Oct 26, 2021
8199a05
feat!: adapt monitoring code to new implementation
metacosm Oct 21, 2021
a5c481b
feat: removing Event using DefaultEvent instead, renamed DefaultEvent…
csviri Oct 27, 2021
d244948
refactor!: replace Closeable by explicit Stoppable interface
metacosm Oct 27, 2021
d857aca
refactor: fix generics
metacosm Oct 27, 2021
77b6556
chore: clean-up
metacosm Oct 27, 2021
ec53e03
fix: potential performance issue
metacosm Oct 27, 2021
fba211e
chore: clean-up
metacosm Oct 27, 2021
81412a6
fix: more generics fixes
metacosm Oct 27, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refined Interface of EventSource and EventSourceManager (#597)
* WIP

* Addressing Custom Resource by Name and Namespace refactor + Informer Cache WIP

* fix: DefaultEventHandler init from EventSourceManager

* fix: custom resource selector test improvement

* fix: wip test imrpovements

* fix: test improvements

* fix: further improvements

* fix: build

* feature: add mvn jar to gitignore

* Exposing CustomResourceEventSource and informers

* fix: cleanup

* fix: remove caching optimization since it not possible anymore with informer

* fix:  formatting

* refactor: make name/namespace final

* feature: Simple label selector support

* fix: formatting

* fix: code inspection reports

* fix: merge from v2

* fix: removed most deprecated apis

* chore: renaming vars named k8sClient to kubernetsClient

* chore(deps): bump jandex-maven-plugin from 1.1.1 to 1.2.1 (#592)

Bumps [jandex-maven-plugin](https://github.com/wildfly/jandex-maven-plugin) from 1.1.1 to 1.2.1.
- [Release notes](https://github.com/wildfly/jandex-maven-plugin/releases)
- [Commits](wildfly/jandex-maven-plugin@1.1.1...1.2.1)

---
updated-dependencies:
- dependency-name: org.jboss.jandex:jandex-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump mockito-core from 3.12.4 to 4.0.0 (#591)

Bumps [mockito-core](https://github.com/mockito/mockito) from 3.12.4 to 4.0.0.
- [Release notes](https://github.com/mockito/mockito/releases)
- [Commits](mockito/mockito@v3.12.4...v4.0.0)

---
updated-dependencies:
- dependency-name: org.mockito:mockito-core
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feature: Build PR on v2

* chore(ci): use Java 17

* chore(ci): use only Temurin distribution

* fix: Updated informer mapping to CustomResourceID

* chore: add generics to PostExecutionControl to reduce IDEs noise (#594)

* chore: polish the junit5 extension (#593)

* fix: EventSourceManager API wip

* fix: code review fixes

* fix: improvements of Event Source related APIs

* fix: remarks from code review

Co-authored-by: Chris Laprun <[email protected]>
Co-authored-by: Ioannis Canellos <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Luca Burgazzoli <[email protected]>
  • Loading branch information
5 people committed Oct 28, 2021
commit 1c9f257377a094def58e4493b8fae6782170adb5
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ private RetryExecution getOrInitRetryExecution(ExecutionScope<R> executionScope)
}

private void cleanupAfterDeletedEvent(CustomResourceID customResourceUid) {
eventSourceManager.cleanup(customResourceUid);
eventSourceManager.cleanupForCustomResource(customResourceUid);
eventBuffer.cleanup(customResourceUid);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
package io.javaoperatorsdk.operator.processing.event;

import java.io.IOException;
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.*;
import java.util.concurrent.locks.ReentrantLock;

import org.slf4j.Logger;
Expand All @@ -22,119 +17,81 @@
public class DefaultEventSourceManager<R extends CustomResource<?, ?>>
implements EventSourceManager {

public static final String RETRY_TIMER_EVENT_SOURCE_NAME = "retry-timer-event-source";
public static final String CUSTOM_RESOURCE_EVENT_SOURCE_NAME = "custom-resource-event-source";
private static final Logger log = LoggerFactory.getLogger(DefaultEventSourceManager.class);

private final ReentrantLock lock = new ReentrantLock();
private final Map<String, EventSource> eventSources = new ConcurrentHashMap<>();
private final Set<EventSource> eventSources = Collections.synchronizedSet(new HashSet<>());
private DefaultEventHandler<R> defaultEventHandler;
private TimerEventSource<R> retryTimerEventSource;
private CustomResourceEventSource customResourceEventSource;

DefaultEventSourceManager(DefaultEventHandler<R> defaultEventHandler) {
init(defaultEventHandler);
}

public DefaultEventSourceManager(ConfiguredController<R> controller) {
CustomResourceEventSource customResourceEventSource =
new CustomResourceEventSource<>(controller);
customResourceEventSource = new CustomResourceEventSource<>(controller);
init(new DefaultEventHandler<>(controller, customResourceEventSource));
registerEventSource(CUSTOM_RESOURCE_EVENT_SOURCE_NAME, customResourceEventSource);
registerEventSource(customResourceEventSource);
}

private void init(DefaultEventHandler<R> defaultEventHandler) {
this.defaultEventHandler = defaultEventHandler;
defaultEventHandler.setEventSourceManager(this);

this.retryTimerEventSource = new TimerEventSource<>();
registerEventSource(RETRY_TIMER_EVENT_SOURCE_NAME, retryTimerEventSource);
registerEventSource(retryTimerEventSource);
}

@Override
public void close() {
lock.lock();
try {
lock.lock();

try {
defaultEventHandler.close();
} catch (Exception e) {
log.warn("Error closing event handler", e);
}

for (var entry : eventSources.entrySet()) {
log.debug("Closing event sources.");
for (var eventSource : eventSources) {
try {
log.debug("Closing {} -> {}", entry.getKey(), entry.getValue());
entry.getValue().close();
eventSource.close();
} catch (Exception e) {
log.warn("Error closing {} -> {}", entry.getKey(), entry.getValue(), e);
log.warn("Error closing {} -> {}", eventSource);
}
}

eventSources.clear();
} finally {
lock.unlock();
}
}

@Override
public final void registerEventSource(String name, EventSource eventSource)
public final void registerEventSource(EventSource eventSource)
throws OperatorException {
Objects.requireNonNull(eventSource, "EventSource must not be null");

lock.lock();
try {
lock.lock();
if (eventSources.containsKey(name)) {
throw new IllegalStateException(
"Event source with name already registered. Event source name: " + name);
}
eventSources.put(name, eventSource);
eventSources.add(eventSource);
eventSource.setEventHandler(defaultEventHandler);
eventSource.start();
} catch (Throwable e) {
if (e instanceof IllegalStateException || e instanceof MissingCRDException) {
// leave untouched
throw e;
}
throw new OperatorException("Couldn't register event source named '" + name + "'", e);
} finally {
lock.unlock();
}
}

@Override
public Optional<EventSource> deRegisterEventSource(String name) {
try {
lock.lock();
EventSource currentEventSource = eventSources.remove(name);
if (currentEventSource != null) {
try {
currentEventSource.close();
} catch (IOException e) {
throw new RuntimeException(e);
}
}

return Optional.ofNullable(currentEventSource);
throw new OperatorException(
"Couldn't register event source: " + eventSource.getClass().getName(), e);
} finally {
lock.unlock();
}
}

@Override
public Optional<EventSource> deRegisterCustomResourceFromEventSource(
String eventSourceName, CustomResourceID customResourceUid) {
public void cleanupForCustomResource(CustomResourceID customResourceUid) {
lock.lock();
try {
lock.lock();
EventSource eventSource = this.eventSources.get(eventSourceName);
if (eventSource == null) {
log.warn(
"Event producer: {} not found for custom resource: {}",
eventSourceName,
customResourceUid);
return Optional.empty();
} else {
eventSource.eventSourceDeRegisteredForResource(customResourceUid);
return Optional.of(eventSource);
for (EventSource eventSource : this.eventSources) {
eventSource.cleanupForCustomResource(customResourceUid);
}
} finally {
lock.unlock();
Expand All @@ -146,19 +103,13 @@ public TimerEventSource getRetryTimerEventSource() {
}

@Override
public Map<String, EventSource> getRegisteredEventSources() {
return Collections.unmodifiableMap(eventSources);
public Set<EventSource> getRegisteredEventSources() {
return Collections.unmodifiableSet(eventSources);
}

@Override
public CustomResourceEventSource getCustomResourceEventSource() {
return (CustomResourceEventSource) getRegisteredEventSources()
.get(CUSTOM_RESOURCE_EVENT_SOURCE_NAME);
return customResourceEventSource;
}

public void cleanup(CustomResourceID customResourceUid) {
getRegisteredEventSources()
.keySet()
.forEach(k -> deRegisterCustomResourceFromEventSource(k, customResourceUid));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,10 @@ default void close() throws IOException {}

void setEventHandler(EventHandler eventHandler);

default void eventSourceDeRegisteredForResource(CustomResourceID customResourceUid) {}
/**
* Automatically called when a custom resource is deleted from the cluster.
*
* @param customResourceUid - id of custom resource
*/
default void cleanupForCustomResource(CustomResourceID customResourceUid) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

import java.io.Closeable;
import java.io.IOException;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.OperatorException;
Expand All @@ -14,29 +13,15 @@ public interface EventSourceManager<T extends CustomResource<?, ?>> extends Clos
/**
* Add the {@link EventSource} identified by the given <code>name</code> to the event manager.
*
* @param name the name of the {@link EventSource} to add
* @param eventSource the {@link EventSource} to register
* @throws IllegalStateException if an {@link EventSource} with the same name is already
* registered.
* @throws OperatorException if an error occurred during the registration process
*/
void registerEventSource(String name, EventSource eventSource)
void registerEventSource(EventSource eventSource)
throws IllegalStateException, OperatorException;

/**
* Remove the {@link EventSource} identified by the given <code>name</code> from the event
* manager.
*
* @param name the name of the {@link EventSource} to remove
* @return an optional {@link EventSource} which would be empty if no {@link EventSource} have
* been registered with the given name.
*/
Optional<EventSource> deRegisterEventSource(String name);

Optional<EventSource> deRegisterCustomResourceFromEventSource(
String name, CustomResourceID customResourceUid);

Map<String, EventSource> getRegisteredEventSources();
Set<EventSource> getRegisteredEventSources();

CustomResourceEventSource<T> getCustomResourceEventSource();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void scheduleOnce(R customResource, long delay) {
}

@Override
public void eventSourceDeRegisteredForResource(CustomResourceID customResourceUid) {
public void cleanupForCustomResource(CustomResourceID customResourceUid) {
cancelSchedule(customResourceUid);
cancelOnceSchedule(customResourceUid);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void cleanUpAfterDeleteEvent() {

waitMinimalTime();
verify(defaultEventSourceManagerMock, times(1))
.cleanup(CustomResourceID.fromResource(customResource));
.cleanupForCustomResource(CustomResourceID.fromResource(customResource));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.javaoperatorsdk.operator.processing.event;

import java.io.IOException;
import java.util.Map;
import java.util.Set;

import org.junit.jupiter.api.Test;

Expand All @@ -10,16 +10,13 @@
import io.javaoperatorsdk.operator.processing.DefaultEventHandler;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

class DefaultEventSourceManagerTest {

public static final String CUSTOM_EVENT_SOURCE_NAME = "CustomEventSource";

private DefaultEventHandler defaultEventHandlerMock = mock(DefaultEventHandler.class);
private DefaultEventSourceManager defaultEventSourceManager =
new DefaultEventSourceManager(defaultEventHandlerMock);
Expand All @@ -28,12 +25,12 @@ class DefaultEventSourceManagerTest {
public void registersEventSource() {
EventSource eventSource = mock(EventSource.class);

defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource);
defaultEventSourceManager.registerEventSource(eventSource);

Map<String, EventSource> registeredSources =
Set<EventSource> registeredSources =
defaultEventSourceManager.getRegisteredEventSources();
assertThat(registeredSources.entrySet()).hasSize(2);
assertThat(registeredSources.get(CUSTOM_EVENT_SOURCE_NAME)).isEqualTo(eventSource);
assertThat(registeredSources).hasSize(2);

verify(eventSource, times(1)).setEventHandler(eq(defaultEventHandlerMock));
verify(eventSource, times(1)).start();
}
Expand All @@ -42,37 +39,25 @@ public void registersEventSource() {
public void closeShouldCascadeToEventSources() throws IOException {
EventSource eventSource = mock(EventSource.class);
EventSource eventSource2 = mock(EventSource.class);
defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource);
defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME + "2", eventSource2);
defaultEventSourceManager.registerEventSource(eventSource);
defaultEventSourceManager.registerEventSource(eventSource2);

defaultEventSourceManager.close();

verify(eventSource, times(1)).close();
verify(eventSource2, times(1)).close();
}

@Test
public void throwExceptionIfRegisteringEventSourceWithSameName() {
EventSource eventSource = mock(EventSource.class);
EventSource eventSource2 = mock(EventSource.class);

defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource);
assertThatExceptionOfType(IllegalStateException.class)
.isThrownBy(
() -> defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME,
eventSource2));
}

@Test
public void deRegistersEventSources() {
CustomResource customResource = TestUtils.testCustomResource();
EventSource eventSource = mock(EventSource.class);
defaultEventSourceManager.registerEventSource(CUSTOM_EVENT_SOURCE_NAME, eventSource);
defaultEventSourceManager.registerEventSource(eventSource);

defaultEventSourceManager.deRegisterCustomResourceFromEventSource(
CUSTOM_EVENT_SOURCE_NAME, CustomResourceID.fromResource(customResource));
defaultEventSourceManager
.cleanupForCustomResource(CustomResourceID.fromResource(customResource));

verify(eventSource, times(1))
.eventSourceDeRegisteredForResource(eq(CustomResourceID.fromResource(customResource)));
.cleanupForCustomResource(eq(CustomResourceID.fromResource(customResource)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void deRegistersPeriodicalEventSources() {
untilAsserted(() -> assertThat(eventHandlerMock.events).hasSizeGreaterThan(1));

timerEventSource
.eventSourceDeRegisteredForResource(CustomResourceID.fromResource(customResource));
.cleanupForCustomResource(CustomResourceID.fromResource(customResource));

int size = eventHandlerMock.events.size();
untilAsserted(() -> assertThat(eventHandlerMock.events).hasSize(size));
Expand Down Expand Up @@ -103,7 +103,7 @@ public void deRegistersOnceEventSources() {

timerEventSource.scheduleOnce(customResource, PERIOD);
timerEventSource
.eventSourceDeRegisteredForResource(CustomResourceID.fromResource(customResource));
.cleanupForCustomResource(CustomResourceID.fromResource(customResource));

untilAsserted(() -> assertThat(eventHandlerMock.events).isEmpty());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class EventSourceTestCustomResourceController

@Override
public void init(EventSourceManager eventSourceManager) {
eventSourceManager.registerEventSource("Timer", timerEventSource);
eventSourceManager.registerEventSource(timerEventSource);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class InformerEventSourceTestCustomResourceController implements
public void init(EventSourceManager eventSourceManager) {
eventSource = new InformerEventSource<>(kubernetesClient, ConfigMap.class,
Mappers.fromAnnotation(RELATED_RESOURCE_UID));
eventSourceManager.registerEventSource("configmap", eventSource);
eventSourceManager.registerEventSource(eventSource);
}

@Override
Expand Down