Skip to content

fix!: remove periodic schedule from timer event source #653

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

Merged
merged 2 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,7 @@ public class TimerEventSource<R extends CustomResource<?, ?>> extends AbstractEv
private final Timer timer = new Timer();
private final AtomicBoolean running = new AtomicBoolean();
private final Map<CustomResourceID, EventProducerTimeTask> onceTasks = new ConcurrentHashMap<>();
private final Map<CustomResourceID, EventProducerTimeTask> timerTasks = new ConcurrentHashMap<>();

public void schedule(R customResource, long delay, long period) {
if (!running.get()) {
throw new IllegalStateException("The TimerEventSource is not running");
}

CustomResourceID resourceUid = CustomResourceID.fromResource(customResource);
if (timerTasks.containsKey(resourceUid)) {
return;
}
EventProducerTimeTask task = new EventProducerTimeTask(resourceUid);
timerTasks.put(resourceUid, task);
timer.schedule(task, delay, period);
}

public void scheduleOnce(R customResource, long delay) {
if (!running.get()) {
Expand All @@ -51,17 +37,9 @@ public void scheduleOnce(R customResource, long delay) {

@Override
public void cleanupForCustomResource(CustomResourceID customResourceUid) {
cancelSchedule(customResourceUid);
cancelOnceSchedule(customResourceUid);
}

public void cancelSchedule(CustomResourceID customResourceID) {
TimerTask timerTask = timerTasks.remove(customResourceID);
if (timerTask != null) {
timerTask.cancel();
}
}

public void cancelOnceSchedule(CustomResourceID customResourceUid) {
TimerTask timerTask = onceTasks.remove(customResourceUid);
if (timerTask != null) {
Expand All @@ -78,7 +56,6 @@ public void start() {
public void stop() {
running.set(false);
onceTasks.keySet().forEach(this::cancelOnceSchedule);
timerTasks.keySet().forEach(this::cancelSchedule);
timer.cancel();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class TimerEventSourceTest {

public static final int INITIAL_DELAY = 50;
public static final int PERIOD = 50;
public static final int TESTING_TIME_SLACK = 40;

private TimerEventSource<TestCustomResource> timerEventSource;
private CapturingEventHandler eventHandlerMock;
Expand All @@ -38,35 +37,6 @@ public void setup() {
timerEventSource.start();
}

@Test
public void producesEventsPeriodically() {
TestCustomResource customResource = TestUtils.testCustomResource();
timerEventSource.schedule(customResource, INITIAL_DELAY, PERIOD);

untilAsserted(() -> {
assertThat(eventHandlerMock.events)
.hasSizeGreaterThan(2);
assertThat(eventHandlerMock.events)
.allMatch(e -> e.getRelatedCustomResourceID()
.equals(CustomResourceID.fromResource(customResource)));

});
}

@Test
public void deRegistersPeriodicalEventSources() {
TestCustomResource customResource = TestUtils.testCustomResource();

timerEventSource.schedule(customResource, INITIAL_DELAY, PERIOD);
untilAsserted(() -> assertThat(eventHandlerMock.events).hasSizeGreaterThan(1));

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

int size = eventHandlerMock.events.size();
untilAsserted(() -> assertThat(eventHandlerMock.events).hasSize(size));
}

@Test
public void schedulesOnce() {
TestCustomResource customResource = TestUtils.testCustomResource();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

import java.util.concurrent.atomic.AtomicInteger;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.ControllerUtils;
import io.javaoperatorsdk.operator.api.Context;
Expand All @@ -24,9 +21,6 @@ public class EventSourceTestCustomResourceController
public static final String FINALIZER_NAME =
ControllerUtils.getDefaultFinalizerName(
CustomResource.getCRDName(EventSourceTestCustomResource.class));
private static final Logger log =
LoggerFactory.getLogger(EventSourceTestCustomResourceController.class);
public static final int TIMER_DELAY = 300;
public static final int TIMER_PERIOD = 500;
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
private final TimerEventSource<EventSourceTestCustomResource> timerEventSource =
Expand All @@ -41,13 +35,11 @@ public void prepareEventSources(EventSourceManager eventSourceManager) {
public UpdateControl<EventSourceTestCustomResource> createOrUpdateResource(
EventSourceTestCustomResource resource, Context context) {

timerEventSource.schedule(resource, TIMER_DELAY, TIMER_PERIOD);

numberOfExecutions.addAndGet(1);
ensureStatusExists(resource);
resource.getStatus().setState(EventSourceTestCustomResourceStatus.State.SUCCESS);

return UpdateControl.updateStatusSubResource(resource);
return UpdateControl.updateStatusSubResource(resource).rescheduleAfter(TIMER_PERIOD);
}

private void ensureStatusExists(EventSourceTestCustomResource resource) {
Expand Down