Skip to content

Commit 0701cc5

Browse files
committed
fix(micrometer): fix NPE when reconciling already marked events
Refs: #836
1 parent 220bf85 commit 0701cc5

File tree

3 files changed

+29
-5
lines changed

3 files changed

+29
-5
lines changed

micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.LinkedList;
55
import java.util.List;
66
import java.util.Map;
7+
import java.util.Optional;
78

89
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
910
import io.javaoperatorsdk.operator.api.reconciler.RetryInfo;
@@ -57,11 +58,13 @@ public void cleanupDoneFor(ResourceID customResourceUid) {
5758
incrementCounter(customResourceUid, "events.delete");
5859
}
5960

60-
public void reconcileCustomResource(ResourceID resourceID,
61-
RetryInfo retryInfo) {
61+
public void reconcileCustomResource(ResourceID resourceID, RetryInfo retryInfoNullable) {
62+
Optional<RetryInfo> retryInfo = Optional.ofNullable(retryInfoNullable);
6263
incrementCounter(resourceID, RECONCILIATIONS + "started",
63-
RECONCILIATIONS + "retries.number", "" + retryInfo.getAttemptCount(),
64-
RECONCILIATIONS + "retries.last", "" + retryInfo.isLastAttempt());
64+
RECONCILIATIONS + "retries.number",
65+
"" + retryInfo.map(RetryInfo::getAttemptCount).orElse(0),
66+
RECONCILIATIONS + "retries.last",
67+
"" + retryInfo.map(RetryInfo::isLastAttempt).orElse(true));
6568
}
6669

6770
@Override

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,22 @@ class EventProcessor<R extends HasMetadata> implements EventHandler, LifecycleAw
8181
eventSourceManager);
8282
}
8383

84+
EventProcessor(
85+
ReconciliationDispatcher<R> reconciliationDispatcher,
86+
EventSourceManager<R> eventSourceManager,
87+
String relatedControllerName,
88+
Retry retry,
89+
Metrics metrics) {
90+
this(
91+
eventSourceManager.getControllerResourceEventSource().getResourceCache(),
92+
null,
93+
relatedControllerName,
94+
reconciliationDispatcher,
95+
retry,
96+
metrics,
97+
eventSourceManager);
98+
}
99+
84100
private EventProcessor(
85101
Cache<R> cache,
86102
ExecutorService executor,

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.slf4j.LoggerFactory;
1313

1414
import io.fabric8.kubernetes.api.model.HasMetadata;
15+
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
1516
import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceCache;
1617
import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource;
1718
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction;
@@ -23,6 +24,7 @@
2324
import static io.javaoperatorsdk.operator.TestUtils.testCustomResource;
2425
import static org.assertj.core.api.Assertions.assertThat;
2526
import static org.mockito.ArgumentMatchers.eq;
27+
import static org.mockito.ArgumentMatchers.isNull;
2628
import static org.mockito.Mockito.any;
2729
import static org.mockito.Mockito.mock;
2830
import static org.mockito.Mockito.never;
@@ -47,6 +49,7 @@ class EventProcessorTest {
4749
private TimerEventSource retryTimerEventSourceMock = mock(TimerEventSource.class);
4850
private ControllerResourceEventSource controllerResourceEventSourceMock =
4951
mock(ControllerResourceEventSource.class);
52+
private Metrics metricsMock = mock(Metrics.class);
5053
private EventProcessor eventProcessor;
5154
private EventProcessor eventProcessorWithRetry;
5255

@@ -276,7 +279,8 @@ public void cancelScheduleOnceEventsOnSuccessfulExecution() {
276279
public void startProcessedMarkedEventReceivedBefore() {
277280
var crID = new ResourceID("test-cr", TEST_NAMESPACE);
278281
eventProcessor =
279-
spy(new EventProcessor(reconciliationDispatcherMock, eventSourceManagerMock, "Test", null));
282+
spy(new EventProcessor(reconciliationDispatcherMock, eventSourceManagerMock, "Test", null,
283+
metricsMock));
280284
when(resourceCacheMock.get(eq(crID))).thenReturn(Optional.of(testCustomResource()));
281285
eventProcessor.handleEvent(new Event(crID));
282286

@@ -285,6 +289,7 @@ public void startProcessedMarkedEventReceivedBefore() {
285289
eventProcessor.start();
286290

287291
verify(reconciliationDispatcherMock, timeout(100).times(1)).handleExecution(any());
292+
verify(metricsMock, times(1)).reconcileCustomResource(any(), isNull());;
288293
}
289294

290295
private ResourceID eventAlreadyUnderProcessing() {

0 commit comments

Comments
 (0)