Skip to content

Commit 6b1785b

Browse files
committed
fix: deletion not reconciled when merged into eventFilteringUpdate response
Signed-off-by: xstefank <xstefank122@gmail.com>
1 parent f51526f commit 6b1785b

5 files changed

Lines changed: 260 additions & 1 deletion

File tree

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,23 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator<
141141
? ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown()
142142
: null);
143143
},
144-
() -> log.debug("No new event present after the filtering update"));
144+
() -> {
145+
log.debug("No new event present after the filtering update");
146+
// If the resource was marked for deletion concurrently with this update, the deletion
147+
// event arrived at a lower RV than the update result and was deferred then dropped by
148+
// doneEventFilterModify (deferred RV < lastOwnUpdatedRV). The API server merges the
149+
// deletionTimestamp into the update response, so updatedForLambda already carries it,
150+
// but no event was propagated to the reconciler. Trigger one now so the finalizer can
151+
// be removed.
152+
if (updatedForLambda != null
153+
&& updatedForLambda.isMarkedForDeletion()
154+
&& !resourceToUpdate.isMarkedForDeletion()) {
155+
log.debug(
156+
"Resource {} was marked for deletion during update, triggering reconciliation",
157+
id);
158+
handleEvent(ResourceAction.UPDATED, updatedForLambda, resourceToUpdate, null);
159+
}
160+
});
145161
}
146162
}
147163

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.deletionduringstatusupdate;
17+
18+
import io.fabric8.kubernetes.api.model.Namespaced;
19+
import io.fabric8.kubernetes.client.CustomResource;
20+
import io.fabric8.kubernetes.model.annotation.Group;
21+
import io.fabric8.kubernetes.model.annotation.ShortNames;
22+
import io.fabric8.kubernetes.model.annotation.Version;
23+
24+
@Group("sample.javaoperatorsdk")
25+
@Version("v1")
26+
@ShortNames("ddsu")
27+
public class DeletionDuringStatusUpdateCustomResource
28+
extends CustomResource<Void, DeletionDuringStatusUpdateStatus> implements Namespaced {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
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.deletionduringstatusupdate;
17+
18+
import java.time.Duration;
19+
import java.util.Collections;
20+
import java.util.concurrent.TimeUnit;
21+
22+
import org.junit.jupiter.api.AfterEach;
23+
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.api.extension.RegisterExtension;
25+
26+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
27+
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
import static org.awaitility.Awaitility.await;
31+
32+
/**
33+
* Regression test for: deletion event dropped when resource is deleted concurrently with a status
34+
* update.
35+
*/
36+
class DeletionDuringStatusUpdateIT {
37+
38+
static final String RESOURCE_NAME = "test-resource";
39+
40+
@RegisterExtension
41+
LocallyRunOperatorExtension extension =
42+
LocallyRunOperatorExtension.builder()
43+
.withReconciler(new DeletionDuringStatusUpdateReconciler())
44+
.build();
45+
46+
@AfterEach
47+
void forceCleanup() {
48+
// If the test failed, remove the finalizer so the resource can be deleted
49+
var res = extension.get(DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME);
50+
if (res != null) {
51+
res.getMetadata().setFinalizers(Collections.emptyList());
52+
extension.replace(res);
53+
extension.delete(res);
54+
}
55+
}
56+
57+
@Test
58+
void deletionDuringStatusUpdateTriggersCleanup() throws InterruptedException {
59+
var reconciler = extension.getReconcilerOfType(DeletionDuringStatusUpdateReconciler.class);
60+
61+
extension.create(testResource());
62+
63+
// Wait until the reconciler is inside the update operation (active-update window is open)
64+
assertThat(reconciler.patchStartedLatch.await(30, TimeUnit.SECONDS))
65+
.as("reconciler should enter the patch update operation")
66+
.isTrue();
67+
68+
// Issue delete — K8s sets deletionTimestamp while the active-update window is open
69+
extension.delete(testResource());
70+
71+
// Wait for deletionTimestamp to be confirmed on the resource in K8s
72+
await()
73+
.atMost(Duration.ofSeconds(30))
74+
.until(
75+
() -> {
76+
var res =
77+
extension.get(DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME);
78+
return res != null && res.isMarkedForDeletion();
79+
});
80+
81+
// Signal the reconciler to proceed with the actual PATCH. K8s will merge deletionTimestamp
82+
// into the response - the deletion event (lower RV) is now deferred and will be dropped
83+
// without the fix.
84+
reconciler.deleteConfirmedLatch.countDown();
85+
86+
// cleanup() must be called — the deletion must not be silently lost
87+
assertThat(reconciler.cleanupCalledLatch.await(30, TimeUnit.SECONDS))
88+
.as("cleanup() must be called after the status update that races with the delete")
89+
.isTrue();
90+
91+
// Resource must eventually disappear (finalizer removed)
92+
await()
93+
.atMost(Duration.ofSeconds(30))
94+
.untilAsserted(
95+
() ->
96+
assertThat(
97+
extension.get(
98+
DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME))
99+
.isNull());
100+
}
101+
102+
DeletionDuringStatusUpdateCustomResource testResource() {
103+
var resource = new DeletionDuringStatusUpdateCustomResource();
104+
resource.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build());
105+
return resource;
106+
}
107+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
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.deletionduringstatusupdate;
17+
18+
import java.util.concurrent.CountDownLatch;
19+
import java.util.concurrent.TimeUnit;
20+
21+
import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
22+
import io.javaoperatorsdk.operator.api.reconciler.Context;
23+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
24+
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
25+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
26+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
27+
28+
@ControllerConfiguration
29+
public class DeletionDuringStatusUpdateReconciler
30+
implements Reconciler<DeletionDuringStatusUpdateCustomResource>,
31+
Cleaner<DeletionDuringStatusUpdateCustomResource> {
32+
33+
final CountDownLatch patchStartedLatch = new CountDownLatch(1);
34+
final CountDownLatch deleteConfirmedLatch = new CountDownLatch(1);
35+
final CountDownLatch cleanupCalledLatch = new CountDownLatch(1);
36+
37+
@Override
38+
public UpdateControl<DeletionDuringStatusUpdateCustomResource> reconcile(
39+
DeletionDuringStatusUpdateCustomResource resource,
40+
Context<DeletionDuringStatusUpdateCustomResource> context)
41+
throws InterruptedException {
42+
if (resource.isMarkedForDeletion()) {
43+
return UpdateControl.noUpdate();
44+
}
45+
46+
var status = new DeletionDuringStatusUpdateStatus();
47+
status.setReady(true);
48+
resource.setStatus(status);
49+
50+
context
51+
.resourceOperations()
52+
.resourcePatch(
53+
resource,
54+
r -> {
55+
patchStartedLatch.countDown();
56+
try {
57+
if (!deleteConfirmedLatch.await(30, TimeUnit.SECONDS)) {
58+
throw new RuntimeException("Timed out waiting for delete confirmation");
59+
}
60+
} catch (InterruptedException e) {
61+
Thread.currentThread().interrupt();
62+
throw new RuntimeException(e);
63+
}
64+
r.getMetadata().setResourceVersion(null);
65+
return context.getClient().resource(r).patchStatus();
66+
},
67+
context.eventSourceRetriever().getControllerEventSource());
68+
69+
return UpdateControl.noUpdate();
70+
}
71+
72+
@Override
73+
public DeleteControl cleanup(
74+
DeletionDuringStatusUpdateCustomResource resource,
75+
Context<DeletionDuringStatusUpdateCustomResource> context) {
76+
cleanupCalledLatch.countDown();
77+
return DeleteControl.defaultDelete();
78+
}
79+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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.deletionduringstatusupdate;
17+
18+
public class DeletionDuringStatusUpdateStatus {
19+
20+
private boolean ready;
21+
22+
public boolean isReady() {
23+
return ready;
24+
}
25+
26+
public void setReady(boolean ready) {
27+
this.ready = ready;
28+
}
29+
}

0 commit comments

Comments
 (0)