Skip to content

Commit a92bf98

Browse files
csviricsviri
authored
fix: retry finalizer removal on http 422 (#2776)
1 parent 2acb3f3 commit a92bf98

File tree

6 files changed

+166
-2
lines changed

6 files changed

+166
-2
lines changed

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

+12-2
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,13 @@ public P conflictRetryingPatch(
381381
} catch (KubernetesClientException e) {
382382
log.trace("Exception during patch for resource: {}", resource);
383383
retryIndex++;
384-
// only retry on conflict (HTTP 409), otherwise fail
385-
if (e.getCode() != 409) {
384+
// only retry on conflict (409) and unprocessable content (422) which
385+
// can happen if JSON Patch is not a valid request since there was
386+
// a concurrent request which already removed another finalizer:
387+
// List element removal from a list is by index in JSON Patch
388+
// so if addressing a second finalizer but first is meanwhile removed
389+
// it is a wrong request.
390+
if (e.getCode() != 409 && e.getCode() != 422) {
386391
throw e;
387392
}
388393
if (retryIndex >= MAX_UPDATE_RETRY) {
@@ -392,6 +397,11 @@ public P conflictRetryingPatch(
392397
+ ") retry attempts to patch resource: "
393398
+ ResourceID.fromResource(resource));
394399
}
400+
log.debug(
401+
"Retrying patch for resource name: {}, namespace: {}; HTTP code: {}",
402+
resource.getMetadata().getName(),
403+
resource.getMetadata().getNamespace(),
404+
e.getCode());
395405
resource =
396406
customResourceFacade.getResource(
397407
resource.getMetadata().getNamespace(), resource.getMetadata().getName());

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/concurrentfinalizerremoval/ConcurrentFinalizerRemovalCustomResource.java

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package io.javaoperatorsdk.operator.baseapi.concurrentfinalizerremoval;
2+
3+
import io.fabric8.kubernetes.api.model.Namespaced;
4+
import io.fabric8.kubernetes.client.CustomResource;
5+
import io.fabric8.kubernetes.model.annotation.Group;
6+
import io.fabric8.kubernetes.model.annotation.ShortNames;
7+
import io.fabric8.kubernetes.model.annotation.Version;
8+
9+
@Group("sample.javaoperatorsdk")
10+
@Version("v1")
11+
@ShortNames("cfr")
12+
public class ConcurrentFinalizerRemovalCustomResource
13+
extends CustomResource<ConcurrentFinalizerRemovalSpec, Void> implements Namespaced {}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/concurrentfinalizerremoval/ConcurrentFinalizerRemovalIT.java

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package io.javaoperatorsdk.operator.baseapi.concurrentfinalizerremoval;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.junit.jupiter.api.extension.RegisterExtension;
5+
import org.slf4j.Logger;
6+
import org.slf4j.LoggerFactory;
7+
8+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
9+
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
10+
import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
11+
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
import static org.awaitility.Awaitility.await;
14+
15+
class ConcurrentFinalizerRemovalIT {
16+
17+
private static final Logger log = LoggerFactory.getLogger(ConcurrentFinalizerRemovalIT.class);
18+
public static final String TEST_RESOURCE_NAME = "test";
19+
20+
@RegisterExtension
21+
LocallyRunOperatorExtension extension =
22+
LocallyRunOperatorExtension.builder()
23+
// should work without a retry, thus not retry the whole reconciliation but to retry
24+
// finalizer removal only.
25+
.withReconciler(
26+
new ConcurrentFinalizerRemovalReconciler1(),
27+
o ->
28+
o.withRetry(GenericRetry.noRetry()).withFinalizer("reconciler1.sample/finalizer"))
29+
.withReconciler(
30+
new ConcurrentFinalizerRemovalReconciler2(),
31+
o ->
32+
o.withRetry(GenericRetry.noRetry()).withFinalizer("reconciler2.sample/finalizer"))
33+
.build();
34+
35+
@Test
36+
void concurrentFinalizerRemoval() {
37+
for (int i = 0; i < 10; i++) {
38+
var resource = extension.create(createResource());
39+
await()
40+
.untilAsserted(
41+
() -> {
42+
var res =
43+
extension.get(
44+
ConcurrentFinalizerRemovalCustomResource.class, TEST_RESOURCE_NAME);
45+
assertThat(res.getMetadata().getFinalizers()).hasSize(2);
46+
});
47+
resource.getMetadata().setResourceVersion(null);
48+
extension.delete(resource);
49+
50+
await()
51+
.untilAsserted(
52+
() -> {
53+
var res =
54+
extension.get(
55+
ConcurrentFinalizerRemovalCustomResource.class, TEST_RESOURCE_NAME);
56+
assertThat(res).isNull();
57+
});
58+
}
59+
}
60+
61+
public ConcurrentFinalizerRemovalCustomResource createResource() {
62+
ConcurrentFinalizerRemovalCustomResource res = new ConcurrentFinalizerRemovalCustomResource();
63+
res.setMetadata(new ObjectMetaBuilder().withName(TEST_RESOURCE_NAME).build());
64+
res.setSpec(new ConcurrentFinalizerRemovalSpec());
65+
res.getSpec().setNumber(0);
66+
return res;
67+
}
68+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/concurrentfinalizerremoval/ConcurrentFinalizerRemovalReconciler1.java

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package io.javaoperatorsdk.operator.baseapi.concurrentfinalizerremoval;
2+
3+
import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
4+
import io.javaoperatorsdk.operator.api.reconciler.Context;
5+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
6+
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
7+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
8+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
9+
10+
@ControllerConfiguration
11+
public class ConcurrentFinalizerRemovalReconciler1
12+
implements Reconciler<ConcurrentFinalizerRemovalCustomResource>,
13+
Cleaner<ConcurrentFinalizerRemovalCustomResource> {
14+
15+
@Override
16+
public UpdateControl<ConcurrentFinalizerRemovalCustomResource> reconcile(
17+
ConcurrentFinalizerRemovalCustomResource resource,
18+
Context<ConcurrentFinalizerRemovalCustomResource> context) {
19+
return UpdateControl.noUpdate();
20+
}
21+
22+
@Override
23+
public DeleteControl cleanup(
24+
ConcurrentFinalizerRemovalCustomResource resource,
25+
Context<ConcurrentFinalizerRemovalCustomResource> context)
26+
throws Exception {
27+
return DeleteControl.defaultDelete();
28+
}
29+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/concurrentfinalizerremoval/ConcurrentFinalizerRemovalReconciler2.java

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package io.javaoperatorsdk.operator.baseapi.concurrentfinalizerremoval;
2+
3+
import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
4+
import io.javaoperatorsdk.operator.api.reconciler.Context;
5+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
6+
import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
7+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
8+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
9+
10+
@ControllerConfiguration
11+
public class ConcurrentFinalizerRemovalReconciler2
12+
implements Reconciler<ConcurrentFinalizerRemovalCustomResource>,
13+
Cleaner<ConcurrentFinalizerRemovalCustomResource> {
14+
15+
@Override
16+
public UpdateControl<ConcurrentFinalizerRemovalCustomResource> reconcile(
17+
ConcurrentFinalizerRemovalCustomResource resource,
18+
Context<ConcurrentFinalizerRemovalCustomResource> context) {
19+
return UpdateControl.noUpdate();
20+
}
21+
22+
@Override
23+
public DeleteControl cleanup(
24+
ConcurrentFinalizerRemovalCustomResource resource,
25+
Context<ConcurrentFinalizerRemovalCustomResource> context)
26+
throws Exception {
27+
return DeleteControl.defaultDelete();
28+
}
29+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/concurrentfinalizerremoval/ConcurrentFinalizerRemovalSpec.java

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package io.javaoperatorsdk.operator.baseapi.concurrentfinalizerremoval;
2+
3+
public class ConcurrentFinalizerRemovalSpec {
4+
5+
private int number;
6+
7+
public int getNumber() {
8+
return number;
9+
}
10+
11+
public ConcurrentFinalizerRemovalSpec setNumber(int number) {
12+
this.number = number;
13+
return this;
14+
}
15+
}

0 commit comments

Comments
 (0)