Skip to content

Commit 9d269a6

Browse files
metacosmmetacosm
authored
feat: matcher avoiding creating the desired state when possible (#992)
1 parent 4aa7b80 commit 9d269a6

File tree

7 files changed

+152
-56
lines changed

7 files changed

+152
-56
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,23 @@ public void reconcile(P primary, Context context) {
3636
final var updatable = isUpdatable(primary, context);
3737
if (creatable || updatable) {
3838
var maybeActual = getResource(primary);
39-
var desired = desired(primary, context);
4039
if (maybeActual.isEmpty()) {
4140
if (creatable) {
41+
var desired = desired(primary, context);
4242
log.debug("Creating dependent {} for primary {}", desired, primary);
4343
creator.create(desired, primary, context);
4444
}
4545
} else {
4646
final var actual = maybeActual.get();
47-
if (updatable && !updater.match(actual, desired, context)) {
48-
log.debug("Updating dependent {} for primary {}", desired, primary);
49-
updater.update(actual, desired, primary, context);
47+
if (updatable) {
48+
final var match = updater.match(actual, primary, context);
49+
if (!match.matched()) {
50+
final var desired = match.computedDesired().orElse(desired(primary, context));
51+
log.debug("Updating dependent {} for primary {}", desired, primary);
52+
updater.update(actual, desired, primary, context);
53+
}
5054
} else {
51-
log.debug("Update skipped for dependent {} as it matched the existing one", desired);
55+
log.debug("Update skipped for dependent {} as it matched the existing one", actual);
5256
}
5357
}
5458
} else {

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Matcher.java

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,36 @@
11
package io.javaoperatorsdk.operator.api.reconciler.dependent;
22

3+
import java.util.Optional;
4+
5+
import io.fabric8.kubernetes.api.model.HasMetadata;
36
import io.javaoperatorsdk.operator.api.reconciler.Context;
47

5-
public interface Matcher<R> {
6-
boolean match(R actualResource, R desiredResource, Context context);
8+
public interface Matcher<R, P extends HasMetadata> {
9+
interface Result<R> {
10+
boolean matched();
11+
12+
default Optional<R> computedDesired() {
13+
return Optional.empty();
14+
}
15+
16+
static <T> Result<T> nonComputed(boolean matched) {
17+
return () -> matched;
18+
}
19+
20+
static <T> Result<T> computed(boolean matched, T computedDesired) {
21+
return new Result<>() {
22+
@Override
23+
public boolean matched() {
24+
return matched;
25+
}
26+
27+
@Override
28+
public Optional<T> computedDesired() {
29+
return Optional.of(computedDesired);
30+
}
31+
};
32+
}
33+
}
34+
35+
Result<R> match(R actualResource, P primary, Context context);
736
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Updater.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
11
package io.javaoperatorsdk.operator.api.reconciler.dependent;
22

3-
import java.util.Objects;
4-
53
import io.fabric8.kubernetes.api.model.HasMetadata;
64
import io.javaoperatorsdk.operator.api.reconciler.Context;
5+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result;
76

87
@SuppressWarnings("rawtypes")
98
public interface Updater<R, P extends HasMetadata> {
10-
Updater NOOP = (actual, desired, primary, context) -> {
9+
Updater NOOP = new Updater() {
10+
@Override
11+
public void update(Object actual, Object desired, HasMetadata primary, Context context) {}
12+
13+
@Override
14+
public Result match(Object actualResource, HasMetadata primary, Context context) {
15+
return Result.nonComputed(true);
16+
}
1117
};
1218

1319
void update(R actual, R desired, P primary, Context context);
1420

15-
default boolean match(R actualResource, R desiredResource, Context context) {
16-
return Objects.equals(actualResource, desiredResource);
17-
}
21+
Result<R> match(R actualResource, P primary, Context context);
1822
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,37 +8,52 @@
88
import io.javaoperatorsdk.operator.api.reconciler.Context;
99
import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher;
1010

11-
public class GenericKubernetesResourceMatcher<R extends HasMetadata> implements Matcher<R> {
11+
public class GenericKubernetesResourceMatcher<R extends HasMetadata, P extends HasMetadata>
12+
implements Matcher<R, P> {
1213

13-
private GenericKubernetesResourceMatcher() {}
14+
private final KubernetesDependentResource<R, P> dependentResource;
1415

15-
@SuppressWarnings({"rawtypes", "unchecked"})
16-
public static <R extends HasMetadata> Matcher<R> matcherFor(Class<R> resourceType) {
16+
private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> dependentResource) {
17+
this.dependentResource = dependentResource;
18+
}
19+
20+
@SuppressWarnings({"unchecked", "rawtypes"})
21+
static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
22+
Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
1723
if (Secret.class.isAssignableFrom(resourceType)) {
18-
return (actual, desired, context) -> ResourceComparators.compareSecretData((Secret) desired,
19-
(Secret) actual);
24+
return (actual, primary, context) -> {
25+
final var desired = dependentResource.desired(primary, context);
26+
return Result.computed(
27+
ResourceComparators.compareSecretData((Secret) desired, (Secret) actual), desired);
28+
};
2029
} else if (ConfigMap.class.isAssignableFrom(resourceType)) {
21-
return (actual, desired, context) -> ResourceComparators
22-
.compareConfigMapData((ConfigMap) desired, (ConfigMap) actual);
30+
return (actual, primary, context) -> {
31+
final var desired = dependentResource.desired(primary, context);
32+
return Result.computed(
33+
ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actual),
34+
desired);
35+
};
2336
} else {
24-
return new GenericKubernetesResourceMatcher();
37+
return new GenericKubernetesResourceMatcher(dependentResource);
2538
}
2639
}
2740

2841
@Override
29-
public boolean match(R actualResource, R desiredResource, Context context) {
42+
public Result<R> match(R actualResource, P primary, Context context) {
3043
final var objectMapper = context.getConfigurationService().getObjectMapper();
44+
final var desired = dependentResource.desired(primary, context);
45+
3146
// reflection will be replaced by this:
3247
// https://github.com/fabric8io/kubernetes-client/issues/3816
33-
var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desiredResource));
48+
var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
3449
var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
3550
var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
3651
for (int i = 0; i < diffJsonPatch.size(); i++) {
3752
String operation = diffJsonPatch.get(i).get("op").asText();
3853
if (!operation.equals("add")) {
39-
return false;
54+
return Result.computed(false, desired);
4055
}
4156
}
42-
return true;
57+
return Result.computed(true, desired);
4358
}
4459
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider;
2222
import io.javaoperatorsdk.operator.api.reconciler.dependent.KubernetesClientAware;
2323
import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher;
24+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result;
2425
import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceUpdatePreProcessor;
26+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Updater;
2527
import io.javaoperatorsdk.operator.processing.event.ResourceID;
2628
import io.javaoperatorsdk.operator.processing.event.source.AssociatedSecondaryResourceIdentifier;
2729
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
@@ -39,14 +41,24 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
3941
protected KubernetesClient client;
4042
private InformerEventSource<R, P> informerEventSource;
4143
private boolean addOwnerReference;
42-
private final Matcher<R> matcher;
44+
private final Matcher<R, P> matcher;
4345
private final ResourceUpdatePreProcessor<R> processor;
4446

4547
@SuppressWarnings("unchecked")
4648
public KubernetesDependentResource() {
47-
init(this::create, this::update, this::delete);
48-
matcher = this instanceof Matcher ? (Matcher<R>) this
49-
: GenericKubernetesResourceMatcher.matcherFor(resourceType());
49+
init(this::create, new Updater<>() {
50+
@Override
51+
public void update(R actual, R desired, P primary, Context context) {
52+
KubernetesDependentResource.this.update(actual, desired, primary, context);
53+
}
54+
55+
@Override
56+
public Result<R> match(R actualResource, P primary, Context context) {
57+
return KubernetesDependentResource.this.match(actualResource, primary, context);
58+
}
59+
}, this::delete);
60+
matcher = this instanceof Matcher ? (Matcher<R, P>) this
61+
: GenericKubernetesResourceMatcher.matcherFor(resourceType(), this);
5062
processor = this instanceof ResourceUpdatePreProcessor
5163
? (ResourceUpdatePreProcessor<R>) this
5264
: GenericResourceUpdatePreProcessor.processorFor(resourceType());
@@ -117,8 +129,8 @@ public void update(R actual, R target, P primary, Context context) {
117129
}
118130
}
119131

120-
public boolean match(R actualResource, R desiredResource, Context context) {
121-
return matcher.match(actualResource, desiredResource, context);
132+
public Result<R> match(R actualResource, P primary, Context context) {
133+
return matcher.match(actualResource, primary, context);
122134
}
123135

124136
public void delete(P primary, Context context) {
@@ -173,4 +185,9 @@ public Optional<R> getResource(P primaryResource) {
173185
public void setKubernetesClient(KubernetesClient kubernetesClient) {
174186
this.client = kubernetesClient;
175187
}
188+
189+
@Override
190+
protected R desired(P primary, Context context) {
191+
return super.desired(primary, context);
192+
}
176193
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
22

3+
import java.util.Optional;
4+
35
import org.junit.jupiter.api.Test;
46

7+
import io.fabric8.kubernetes.api.model.HasMetadata;
58
import io.fabric8.kubernetes.api.model.apps.Deployment;
9+
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
610
import io.javaoperatorsdk.operator.ReconcilerUtils;
711
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
812
import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -24,29 +28,40 @@ class GenericKubernetesResourceMatcherTest {
2428

2529
@Test
2630
void checksIfDesiredValuesAreTheSame() {
27-
var target1 = createDeployment();
28-
var desired1 = createDeployment();
29-
final var matcher = GenericKubernetesResourceMatcher.matcherFor(Deployment.class);
30-
assertThat(matcher.match(target1, desired1, context)).isTrue();
31-
32-
var target2 = createDeployment();
33-
var desired2 = createDeployment();
34-
target2.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
35-
assertThat(matcher.match(target2, desired2, context))
31+
var actual = createDeployment();
32+
final var desired = createDeployment();
33+
final var matcher = GenericKubernetesResourceMatcher.matcherFor(Deployment.class,
34+
new KubernetesDependentResource<>() {
35+
@Override
36+
protected Deployment desired(HasMetadata primary, Context context) {
37+
final var currentCase = Optional.ofNullable(primary)
38+
.map(p -> p.getMetadata().getLabels().get("case"))
39+
.orElse(null);
40+
var d = desired;
41+
if ("removed".equals(currentCase)) {
42+
d = createDeployment();
43+
d.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
44+
}
45+
return d;
46+
}
47+
});
48+
assertThat(matcher.match(actual, null, context).matched()).isTrue();
49+
assertThat(matcher.match(actual, null, context).computedDesired().isPresent()).isTrue();
50+
assertThat(matcher.match(actual, null, context).computedDesired().get()).isEqualTo(desired);
51+
52+
actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
53+
assertThat(matcher.match(actual, null, context).matched())
3654
.withFailMessage("Additive changes should be ok")
3755
.isTrue();
3856

39-
var target3 = createDeployment();
40-
var desired3 = createDeployment();
41-
desired3.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
42-
assertThat(matcher.match(target3, desired3, context))
57+
actual = createDeployment();
58+
assertThat(matcher.match(actual, createPrimary("removed"), context).matched())
4359
.withFailMessage("Removed value should not be ok")
4460
.isFalse();
4561

46-
var target4 = createDeployment();
47-
var desired4 = createDeployment();
48-
target4.getSpec().setReplicas(2);
49-
assertThat(matcher.match(target4, desired4, context))
62+
actual = createDeployment();
63+
actual.getSpec().setReplicas(2);
64+
assertThat(matcher.match(actual, null, context).matched())
5065
.withFailMessage("Changed values are not ok")
5166
.isFalse();
5267
}
@@ -55,4 +70,12 @@ Deployment createDeployment() {
5570
return ReconcilerUtils.loadYaml(
5671
Deployment.class, GenericKubernetesResourceMatcherTest.class, "nginx-deployment.yaml");
5772
}
73+
74+
HasMetadata createPrimary(String caseName) {
75+
return new DeploymentBuilder()
76+
.editOrNewMetadata()
77+
.addToLabels("case", caseName)
78+
.endMetadata()
79+
.build();
80+
}
5881
}

‎sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.fabric8.kubernetes.api.model.SecretBuilder;
99
import io.javaoperatorsdk.operator.api.reconciler.Context;
1010
import io.javaoperatorsdk.operator.api.reconciler.dependent.Creator;
11+
import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result;
1112
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
1213
import io.javaoperatorsdk.operator.processing.event.ResourceID;
1314
import io.javaoperatorsdk.operator.processing.event.source.AssociatedSecondaryResourceIdentifier;
@@ -32,24 +33,27 @@ protected Secret desired(MySQLSchema schema, Context context) {
3233
final var password = RandomStringUtils
3334
.randomAlphanumeric(16); // NOSONAR: we don't need cryptographically-strong randomness here
3435
final var name = schema.getMetadata().getName();
35-
final var secretName = String.format(SECRET_FORMAT, name);
36+
final var secretName = getSecretName(name);
3637
final var userName = String.format(USERNAME_FORMAT, name);
3738

3839
return new SecretBuilder()
3940
.withNewMetadata()
4041
.withName(secretName)
4142
.withNamespace(schema.getMetadata().getNamespace())
4243
.endMetadata()
43-
.addToData(
44-
MYSQL_SECRET_USERNAME, encode(userName))
45-
.addToData(
46-
MYSQL_SECRET_PASSWORD, encode(password))
44+
.addToData(MYSQL_SECRET_USERNAME, encode(userName))
45+
.addToData(MYSQL_SECRET_PASSWORD, encode(password))
4746
.build();
4847
}
4948

49+
private String getSecretName(String name) {
50+
return String.format(SECRET_FORMAT, name);
51+
}
52+
5053
@Override
51-
public boolean match(Secret actual, Secret target, Context context) {
52-
return ResourceID.fromResource(actual).equals(ResourceID.fromResource(target));
54+
public Result<Secret> match(Secret actual, MySQLSchema primary, Context context) {
55+
final var desiredSecretName = getSecretName(primary.getMetadata().getName());
56+
return Result.nonComputed(actual.getMetadata().getName().equals(desiredSecretName));
5357
}
5458

5559
@Override

0 commit comments

Comments
 (0)