Skip to content
Open
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 @@ -97,6 +97,7 @@ public void changeNamespaces(Set<String> namespaces) {
public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator<R> updateMethod) {
ResourceID id = ResourceID.fromResource(resourceToUpdate);
log.debug("Starting event filtering and caching update");
final boolean wasMarkedForDeletion = resourceToUpdate.isMarkedForDeletion();
R updatedResource = null;
try {
temporaryResourceCache.startEventFilteringModify(id);
Expand Down Expand Up @@ -141,7 +142,23 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator<
? ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown()
: null);
},
() -> log.debug("No new event present after the filtering update"));
() -> {
log.debug("No new event present after the filtering update");
// If the resource was marked for deletion concurrently with this update, the deletion
// event arrived at a lower RV than the update result and was deferred then dropped by
// doneEventFilterModify (deferred RV < lastOwnUpdatedRV). The API server merges the
// deletionTimestamp into the update response, so updatedForLambda already carries it,
// but no event was propagated to the reconciler. Trigger one now so the finalizer can
// be removed.
if (updatedForLambda != null
&& updatedForLambda.isMarkedForDeletion()
&& !wasMarkedForDeletion) {
log.debug(
"Resource {} was marked for deletion during update, triggering reconciliation",
id);
handleEvent(ResourceAction.UPDATED, updatedForLambda, resourceToUpdate, null);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be triggered with ResourceAction.DELETED instead?

}
Comment on lines +147 to +160
});
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright Java Operator SDK Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.javaoperatorsdk.operator.baseapi.deletionduringstatusupdate;

import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.model.annotation.Group;
import io.fabric8.kubernetes.model.annotation.ShortNames;
import io.fabric8.kubernetes.model.annotation.Version;

@Group("sample.javaoperatorsdk")
@Version("v1")
@ShortNames("ddsu")
public class DeletionDuringStatusUpdateCustomResource
extends CustomResource<Void, DeletionDuringStatusUpdateStatus> implements Namespaced {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Copyright Java Operator SDK Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.javaoperatorsdk.operator.baseapi.deletionduringstatusupdate;

import java.time.Duration;
import java.util.Collections;
import java.util.concurrent.TimeUnit;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;

import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

/**
* Regression test for: deletion event dropped when resource is deleted concurrently with a status
* update.
*/
class DeletionDuringStatusUpdateIT {

static final String RESOURCE_NAME = "test-resource";

@RegisterExtension
LocallyRunOperatorExtension extension =
LocallyRunOperatorExtension.builder()
.withReconciler(new DeletionDuringStatusUpdateReconciler())
.build();

@AfterEach
void forceCleanup() {
// If the test failed, remove the finalizer so the resource can be deleted
var res = extension.get(DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME);
if (res != null) {
res.getMetadata().setFinalizers(Collections.emptyList());
extension.replace(res);
extension.delete(res);
}
}

@Test
void deletionDuringStatusUpdateTriggersCleanup() throws InterruptedException {
var reconciler = extension.getReconcilerOfType(DeletionDuringStatusUpdateReconciler.class);

extension.create(testResource());

// Wait until the reconciler is inside the update operation (active-update window is open)
assertThat(reconciler.patchStartedLatch.await(30, TimeUnit.SECONDS))
.as("reconciler should enter the patch update operation")
.isTrue();

// Issue delete — K8s sets deletionTimestamp while the active-update window is open
extension.delete(testResource());

// Wait for deletionTimestamp to be confirmed on the resource in K8s
await()
.atMost(Duration.ofSeconds(30))
.until(
() -> {
var res =
extension.get(DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME);
return res != null && res.isMarkedForDeletion();
});

// Signal the reconciler to proceed with the actual PATCH. K8s will merge deletionTimestamp
// into the response - the deletion event (lower RV) is now deferred and will be dropped
// without the fix.
reconciler.deleteConfirmedLatch.countDown();

// cleanup() must be called — the deletion must not be silently lost
assertThat(reconciler.cleanupCalledLatch.await(30, TimeUnit.SECONDS))
.as("cleanup() must be called after the status update that races with the delete")
.isTrue();

// Resource must eventually disappear (finalizer removed)
await()
.atMost(Duration.ofSeconds(30))
.untilAsserted(
() ->
assertThat(
extension.get(
DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME))
.isNull());
}

DeletionDuringStatusUpdateCustomResource testResource() {
var resource = new DeletionDuringStatusUpdateCustomResource();
resource.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build());
return resource;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* Copyright Java Operator SDK Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.javaoperatorsdk.operator.baseapi.deletionduringstatusupdate;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;

@ControllerConfiguration
public class DeletionDuringStatusUpdateReconciler
implements Reconciler<DeletionDuringStatusUpdateCustomResource>,
Cleaner<DeletionDuringStatusUpdateCustomResource> {

final CountDownLatch patchStartedLatch = new CountDownLatch(1);
final CountDownLatch deleteConfirmedLatch = new CountDownLatch(1);
final CountDownLatch cleanupCalledLatch = new CountDownLatch(1);

@Override
public UpdateControl<DeletionDuringStatusUpdateCustomResource> reconcile(
DeletionDuringStatusUpdateCustomResource resource,
Context<DeletionDuringStatusUpdateCustomResource> context)
throws InterruptedException {
if (resource.isMarkedForDeletion()) {
return UpdateControl.noUpdate();
}

var status = new DeletionDuringStatusUpdateStatus();
status.setReady(true);
resource.setStatus(status);

context
.resourceOperations()
.resourcePatch(
resource,
r -> {
patchStartedLatch.countDown();
try {
if (!deleteConfirmedLatch.await(30, TimeUnit.SECONDS)) {
throw new RuntimeException("Timed out waiting for delete confirmation");
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
}
r.getMetadata().setResourceVersion(null);
return context.getClient().resource(r).patchStatus();
},
context.eventSourceRetriever().getControllerEventSource());

return UpdateControl.noUpdate();
}

@Override
public DeleteControl cleanup(
DeletionDuringStatusUpdateCustomResource resource,
Context<DeletionDuringStatusUpdateCustomResource> context) {
cleanupCalledLatch.countDown();
return DeleteControl.defaultDelete();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Java Operator SDK Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.javaoperatorsdk.operator.baseapi.deletionduringstatusupdate;

public class DeletionDuringStatusUpdateStatus {

private boolean ready;

public boolean isReady() {
return ready;
}

public void setReady(boolean ready) {
this.ready = ready;
}
}
Loading