Skip to content

Commit b4f7a15

Browse files
SamBarkerSamBarker
committed
Restructure reproducer as two independent controllers
Replace the single-controller model (primary patches secondary status through shared InformerEventSource) with two independent controllers matching the Kroxylicious pattern: a SecondaryReconciler that patches its own status via UpdateControl.patchStatus(), and a PrimaryReconciler that watches secondaries via InformerEventSource and tracks readiness. The test uses a stall mechanism to create a timing window where the secondary controller's status patch races with the primary's own reconciliation. Currently passes — the event is delivered correctly. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
1 parent 2d2d622 commit b4f7a15

6 files changed

Lines changed: 189 additions & 69 deletions

File tree

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuseventfiltering/StatusEventFilteringIT.java‎

Lines changed: 52 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717

1818
import java.time.Duration;
1919
import java.util.Map;
20+
import java.util.Objects;
2021

21-
import org.junit.jupiter.api.Disabled;
2222
import org.junit.jupiter.api.Test;
2323
import org.junit.jupiter.api.extension.RegisterExtension;
2424

@@ -31,29 +31,36 @@
3131
import static org.awaitility.Awaitility.await;
3232

3333
@Sample(
34-
tldr = "Shared InformerEventSource status event filtering reproducer",
34+
tldr = "Cross-controller status event filtering reproducer",
3535
description =
3636
"""
37-
Reproduces an issue where a controller patches a secondary resource's status \
38-
through its own InformerEventSource. The EventFilterWindow suppresses the resulting \
39-
event, so the mapper (which guards on generation == observedGeneration) never fires \
40-
and the controller is never re-triggered by the secondary becoming ready.\
37+
Reproduces an issue where two independent controllers run in the same operator. \
38+
A secondary controller reconciles its primary resource and patches its status \
39+
(observedGeneration = generation) while the primary controller patches its own \
40+
status. The primary controller watches secondaries via InformerEventSource. \
41+
The concurrent status patches may cause the secondary's status-change event to \
42+
be lost — leaving the primary controller unaware the secondary is ready.\
4143
""")
4244
class StatusEventFilteringIT {
4345

4446
@RegisterExtension
45-
LocallyRunOperatorExtension extension =
47+
static LocallyRunOperatorExtension extension =
4648
LocallyRunOperatorExtension.builder()
47-
.withAdditionalCustomResourceDefinition(StatusEventSecondaryResource.class)
49+
.withReconciler(StatusEventSecondaryReconciler.class)
4850
.withReconciler(StatusEventPrimaryReconciler.class)
4951
.build();
5052

5153
@Test
52-
@Disabled("https://github.com/operator-framework/java-operator-sdk/issues/3445")
53-
void mapperShouldFireAfterStatusPatchThroughSharedEventSource() {
54-
var primaryReconciler = extension.getReconcilerOfType(StatusEventPrimaryReconciler.class);
54+
void mapperShouldFireAfterIndependentControllerPatchesStatus() {
55+
// Given: primary exists, secondary created and reaches steady state
56+
var primary = new StatusEventPrimaryResource();
57+
primary.setMetadata(
58+
new ObjectMetaBuilder()
59+
.withName("test-primary")
60+
.withNamespace(extension.getNamespace())
61+
.build());
62+
extension.create(primary);
5563

56-
// Given: a secondary resource exists with generation=1 and no observedGeneration
5764
var secondary = new StatusEventSecondaryResource();
5865
secondary.setMetadata(
5966
new ObjectMetaBuilder()
@@ -65,37 +72,50 @@ void mapperShouldFireAfterStatusPatchThroughSharedEventSource() {
6572
secondary.getSpec().setValue("initial");
6673
extension.create(secondary);
6774

68-
var primary = new StatusEventPrimaryResource();
69-
primary.setMetadata(
70-
new ObjectMetaBuilder()
71-
.withName("test-primary")
72-
.withNamespace(extension.getNamespace())
73-
.build());
75+
await()
76+
.atMost(Duration.ofSeconds(30))
77+
.until(
78+
() -> {
79+
var p = extension.get(StatusEventPrimaryResource.class, "test-primary");
80+
return Boolean.TRUE.equals(p.getStatus().getSecondaryReady());
81+
});
7482

75-
// When: the primary resource is created, triggering reconciliation which patches
76-
// the secondary's observedGeneration=1 through the shared InformerEventSource
77-
extension.create(primary);
83+
// Arm the stall so the next primary reconciliation blocks before returning patchStatus
84+
var primaryReconciler = extension.getReconcilerOfType(StatusEventPrimaryReconciler.class);
85+
primaryReconciler.stallReconcile();
86+
87+
// When: secondary spec changes, triggering the secondary controller to reconcile
88+
// and patch its status via UpdateControl.patchStatus() (through
89+
// eventFilteringUpdateAndCacheResource)
90+
var current = extension.get(StatusEventSecondaryResource.class, "test-secondary");
91+
current.getSpec().setValue("updated");
92+
extension.replace(current);
7893

79-
// Then: the mapper should see gen==obsGen and trigger a second reconciliation.
80-
// With the bug, the EventFilterWindow suppresses the status-change event
81-
// so the mapper never fires — the controller is stuck at 1 execution.
94+
// Then: wait for secondary controller to patch its status (pure status-only WATCH event),
95+
// then release the stalled primary.
8296
await()
8397
.atMost(Duration.ofSeconds(30))
84-
.untilAsserted(
98+
.until(
8599
() -> {
86100
var sec = extension.get(StatusEventSecondaryResource.class, "test-secondary");
87-
assertThat(sec.getStatus().getObservedGeneration())
88-
.as("reconciler should patch observedGeneration to match generation")
89-
.isEqualTo(sec.getMetadata().getGeneration());
101+
return Objects.equals(
102+
sec.getStatus().getObservedGeneration(), sec.getMetadata().getGeneration());
90103
});
104+
primaryReconciler.releaseReconcile();
91105

106+
// The primary was stalled during the spec-change event (secondaryReady=false).
107+
// After release it patches secondaryReady=false. The status-change event from
108+
// the secondary controller's patchStatus should trigger a re-reconciliation
109+
// where the primary sees secondaryReady=true.
92110
await()
93111
.atMost(Duration.ofSeconds(15))
94112
.pollInterval(Duration.ofMillis(500))
95113
.untilAsserted(
96-
() ->
97-
assertThat(primaryReconciler.getNumberOfExecutions())
98-
.as("mapper should fire for gen==obsGen and trigger re-reconciliation")
99-
.isGreaterThanOrEqualTo(2));
114+
() -> {
115+
var p = extension.get(StatusEventPrimaryResource.class, "test-primary");
116+
assertThat(p.getStatus().getSecondaryReady())
117+
.as("primary should see secondary as ready after controller status patch")
118+
.isTrue();
119+
});
100120
}
101121
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuseventfiltering/StatusEventPrimaryReconciler.java‎

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import java.util.List;
1919
import java.util.Objects;
2020
import java.util.Set;
21+
import java.util.concurrent.CountDownLatch;
22+
import java.util.concurrent.TimeUnit;
2123
import java.util.concurrent.atomic.AtomicInteger;
2224

2325
import org.slf4j.Logger;
@@ -41,8 +43,19 @@ public class StatusEventPrimaryReconciler implements Reconciler<StatusEventPrima
4143
public static final String PRIMARY_NAME_LABEL = "primary-name";
4244

4345
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
44-
private InformerEventSource<StatusEventSecondaryResource, StatusEventPrimaryResource>
45-
secondaryEventSource;
46+
private volatile CountDownLatch stallLatch;
47+
48+
public void stallReconcile() {
49+
stallLatch = new CountDownLatch(1);
50+
}
51+
52+
public void releaseReconcile() {
53+
var latch = stallLatch;
54+
if (latch != null) {
55+
latch.countDown();
56+
}
57+
stallLatch = null;
58+
}
4659

4760
@Override
4861
public UpdateControl<StatusEventPrimaryResource> reconcile(
@@ -62,28 +75,29 @@ public UpdateControl<StatusEventPrimaryResource> reconcile(
6275
.withLabel(PRIMARY_NAME_LABEL, resource.getMetadata().getName())
6376
.list()
6477
.getItems();
65-
66-
for (var secondary : secondaries) {
67-
var generation = secondary.getMetadata().getGeneration();
68-
var observedGeneration = secondary.getStatus().getObservedGeneration();
69-
70-
if (!Objects.equals(generation, observedGeneration)) {
71-
log.info(
72-
"Patching secondary '{}' status: obsGen {} -> {}",
73-
secondary.getMetadata().getName(),
74-
observedGeneration,
75-
generation);
76-
77-
secondaryEventSource.eventFilteringUpdateAndCacheResource(
78-
secondary,
79-
s -> {
80-
s.getStatus().setObservedGeneration(s.getMetadata().getGeneration());
81-
return context.getClient().resource(s).patchStatus();
82-
});
78+
var latch = stallLatch;
79+
if (latch != null) {
80+
try {
81+
log.info("PrimaryReconciler stalled before returning patchStatus...");
82+
latch.await(60, TimeUnit.SECONDS);
83+
log.info("PrimaryReconciler released, returning patchStatus");
84+
} catch (InterruptedException e) {
85+
Thread.currentThread().interrupt();
8386
}
8487
}
85-
86-
return UpdateControl.noUpdate();
88+
boolean allReady =
89+
!secondaries.isEmpty()
90+
&& secondaries.stream()
91+
.allMatch(
92+
s ->
93+
Objects.equals(
94+
s.getMetadata().getGeneration(),
95+
s.getStatus().getObservedGeneration()));
96+
97+
log.info("PrimaryReconciler: secondaryReady={}, secondaries={}", allReady, secondaries.size());
98+
99+
resource.getStatus().setSecondaryReady(allReady);
100+
return UpdateControl.patchStatus(resource);
87101
}
88102

89103
@Override
@@ -95,30 +109,20 @@ public List<EventSource<?, StatusEventPrimaryResource>> prepareEventSources(
95109
StatusEventSecondaryResource.class, StatusEventPrimaryResource.class)
96110
.withSecondaryToPrimaryMapper(
97111
secondary -> {
98-
var generation = secondary.getMetadata().getGeneration();
99-
var observedGeneration = secondary.getStatus().getObservedGeneration();
100-
101-
if (!Objects.equals(generation, observedGeneration)) {
102-
log.info(
103-
"Mapper: secondary '{}' gen {} != obsGen {} -> skipping",
104-
secondary.getMetadata().getName(),
105-
generation,
106-
observedGeneration);
112+
var primaryName = secondary.getMetadata().getLabels().get(PRIMARY_NAME_LABEL);
113+
if (primaryName == null) {
107114
return Set.of();
108115
}
109-
110-
var primaryName = secondary.getMetadata().getLabels().get(PRIMARY_NAME_LABEL);
111116
log.info(
112-
"Mapper: secondary '{}' gen matches -> mapping to primary '{}'",
117+
"Mapper: secondary '{}' -> primary '{}'",
113118
secondary.getMetadata().getName(),
114119
primaryName);
115120
return Set.of(
116121
new ResourceID(primaryName, secondary.getMetadata().getNamespace()));
117122
})
118123
.build();
119124

120-
secondaryEventSource = new InformerEventSource<>(config, context);
121-
return List.of(secondaryEventSource);
125+
return List.of(new InformerEventSource<>(config, context));
122126
}
123127

124128
public int getNumberOfExecutions() {

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuseventfiltering/StatusEventPrimaryResource.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,11 @@
2424
@Group("sample.javaoperatorsdk")
2525
@Version("v1")
2626
@ShortNames("sepr")
27-
public class StatusEventPrimaryResource extends CustomResource<Void, Void> implements Namespaced {}
27+
public class StatusEventPrimaryResource
28+
extends CustomResource<Void, StatusEventPrimaryResourceStatus> implements Namespaced {
29+
30+
@Override
31+
protected StatusEventPrimaryResourceStatus initStatus() {
32+
return new StatusEventPrimaryResourceStatus();
33+
}
34+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuseventfiltering/StatusEventPrimaryResourceStatus.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.baseapi.statuseventfiltering;
17+
18+
public class StatusEventPrimaryResourceStatus {
19+
private Boolean secondaryReady;
20+
21+
public Boolean getSecondaryReady() {
22+
return secondaryReady;
23+
}
24+
25+
public void setSecondaryReady(Boolean secondaryReady) {
26+
this.secondaryReady = secondaryReady;
27+
}
28+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuseventfiltering/StatusEventSecondaryReconciler.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright Java Operator SDK Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.javaoperatorsdk.operator.baseapi.statuseventfiltering;
17+
18+
import java.util.Objects;
19+
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
22+
23+
import io.javaoperatorsdk.operator.api.reconciler.Context;
24+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
25+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
26+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
27+
28+
@ControllerConfiguration
29+
public class StatusEventSecondaryReconciler implements Reconciler<StatusEventSecondaryResource> {
30+
31+
private static final Logger log = LoggerFactory.getLogger(StatusEventSecondaryReconciler.class);
32+
33+
@Override
34+
public UpdateControl<StatusEventSecondaryResource> reconcile(
35+
StatusEventSecondaryResource resource, Context<StatusEventSecondaryResource> context) {
36+
37+
var generation = resource.getMetadata().getGeneration();
38+
var observedGeneration = resource.getStatus().getObservedGeneration();
39+
40+
if (!Objects.equals(generation, observedGeneration)) {
41+
log.info(
42+
"Patching secondary '{}' status: obsGen {} -> {}",
43+
resource.getMetadata().getName(),
44+
observedGeneration,
45+
generation);
46+
resource.getStatus().setObservedGeneration(generation);
47+
return UpdateControl.patchStatus(resource);
48+
}
49+
50+
return UpdateControl.noUpdate();
51+
}
52+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuseventfiltering/StatusEventSecondaryResourceStatus.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
public class StatusEventSecondaryResourceStatus {
1919
private Long observedGeneration;
20+
private String message;
2021

2122
public Long getObservedGeneration() {
2223
return observedGeneration;
@@ -25,4 +26,12 @@ public Long getObservedGeneration() {
2526
public void setObservedGeneration(Long observedGeneration) {
2627
this.observedGeneration = observedGeneration;
2728
}
29+
30+
public String getMessage() {
31+
return message;
32+
}
33+
34+
public void setMessage(String message) {
35+
this.message = message;
36+
}
2837
}

0 commit comments

Comments
 (0)