Skip to content

Commit de21ccd

Browse files
metacosmcsviri
metacosm
authored and
csviri
committed
refactor: isolate index handling to BulkDependentResource interface (#1517)
1 parent 736ef8e commit de21ccd

File tree

10 files changed

+119
-178
lines changed

10 files changed

+119
-178
lines changed

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

Lines changed: 38 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package io.javaoperatorsdk.operator.processing.dependent;
22

3-
import java.util.ArrayList;
4-
import java.util.List;
53
import java.util.Optional;
64

75
import org.slf4j.Logger;
@@ -20,15 +18,15 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
2018
implements DependentResource<R, P> {
2119
private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class);
2220

23-
protected final boolean creatable = this instanceof Creator;
24-
protected final boolean updatable = this instanceof Updater;
25-
protected final boolean bulk = this instanceof BulkDependentResource;
21+
private final boolean creatable = this instanceof Creator;
22+
private final boolean updatable = this instanceof Updater;
23+
private final boolean bulk = this instanceof BulkDependentResource;
2624

2725
protected Creator<R, P> creator;
2826
protected Updater<R, P> updater;
2927
protected BulkDependentResource<R, P> bulkDependentResource;
30-
31-
protected List<ResourceDiscriminator<R, P>> resourceDiscriminator = new ArrayList<>(1);
28+
private ResourceDiscriminator<R, P> resourceDiscriminator;
29+
private int currentCount;
3230

3331
@SuppressWarnings("unchecked")
3432
public AbstractDependentResource() {
@@ -42,48 +40,33 @@ public AbstractDependentResource() {
4240
public ReconcileResult<R> reconcile(P primary, Context<P> context) {
4341
if (bulk) {
4442
final var count = bulkDependentResource.count(primary, context);
45-
deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context);
46-
adjustDiscriminators(count);
43+
deleteBulkResourcesIfRequired(count, primary, context);
4744
@SuppressWarnings("unchecked")
4845
final ReconcileResult<R>[] results = new ReconcileResult[count];
4946
for (int i = 0; i < count; i++) {
5047
results[i] = reconcileIndexAware(primary, i, context);
5148
}
49+
currentCount = count;
5250
return ReconcileResult.aggregatedResult(results);
5351
} else {
5452
return reconcileIndexAware(primary, 0, context);
5553
}
5654
}
5755

58-
protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary,
59-
Context<P> context) {
60-
if (targetCount >= actualCount) {
56+
protected void deleteBulkResourcesIfRequired(int targetCount, P primary, Context<P> context) {
57+
if (targetCount >= currentCount) {
6158
return;
6259
}
63-
for (int i = targetCount; i < actualCount; i++) {
64-
var resource = getSecondaryResourceIndexAware(primary, i, context);
60+
for (int i = targetCount; i < currentCount; i++) {
61+
var resource = bulkDependentResource.getSecondaryResource(primary, i, context);
6562
var index = i;
6663
resource.ifPresent(
6764
r -> bulkDependentResource.deleteBulkResourceWithIndex(primary, r, index, context));
6865
}
6966
}
7067

71-
private void adjustDiscriminators(int count) {
72-
if (resourceDiscriminator.size() == count) {
73-
return;
74-
}
75-
if (resourceDiscriminator.size() < count) {
76-
for (int i = resourceDiscriminator.size(); i < count; i++) {
77-
resourceDiscriminator.add(bulkDependentResource.getResourceDiscriminator(i));
78-
}
79-
}
80-
if (resourceDiscriminator.size() > count) {
81-
resourceDiscriminator.subList(count, resourceDiscriminator.size()).clear();
82-
}
83-
}
84-
8568
protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> context) {
86-
Optional<R> maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context)
69+
Optional<R> maybeActual = bulk ? bulkDependentResource.getSecondaryResource(primary, i, context)
8770
: getSecondaryResource(primary, context);
8871
if (creatable || updatable) {
8972
if (maybeActual.isEmpty()) {
@@ -99,7 +82,7 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
9982
if (updatable) {
10083
final Matcher.Result<R> match;
10184
if (bulk) {
102-
match = updater.match(actual, primary, i, context);
85+
match = bulkDependentResource.match(actual, primary, i, context);
10386
} else {
10487
match = updater.match(actual, primary, context);
10588
}
@@ -124,17 +107,12 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
124107
}
125108

126109
private R desiredIndexAware(P primary, int i, Context<P> context) {
127-
return bulk ? desired(primary, i, context)
128-
: desired(primary, context);
110+
return bulk ? desired(primary, i, context) : desired(primary, context);
129111
}
130112

131113
public Optional<R> getSecondaryResource(P primary, Context<P> context) {
132-
return resourceDiscriminator.isEmpty() ? context.getSecondaryResource(resourceType())
133-
: resourceDiscriminator.get(0).distinguish(resourceType(), primary, context);
134-
}
135-
136-
protected Optional<R> getSecondaryResourceIndexAware(P primary, int index, Context<P> context) {
137-
return context.getSecondaryResource(resourceType(), resourceDiscriminator.get(index));
114+
return resourceDiscriminator == null ? context.getSecondaryResource(resourceType())
115+
: resourceDiscriminator.distinguish(resourceType(), primary, context);
138116
}
139117

140118
private void throwIfNull(R desired, P primary, String descriptor) {
@@ -195,28 +173,35 @@ protected R desired(P primary, Context<P> context) {
195173
}
196174

197175
protected R desired(P primary, int index, Context<P> context) {
198-
throw new IllegalStateException(
199-
"Must be implemented for bulk DependentResource creation");
176+
throw new IllegalStateException("Must be implemented for bulk DependentResource creation");
200177
}
201178

202-
public AbstractDependentResource<R, P> setResourceDiscriminator(
203-
ResourceDiscriminator<R, P> resourceDiscriminator) {
204-
if (resourceDiscriminator != null) {
205-
this.resourceDiscriminator.add(resourceDiscriminator);
179+
public void delete(P primary, Context<P> context) {
180+
if (bulk) {
181+
deleteBulkResourcesIfRequired(0, primary, context);
182+
} else {
183+
handleDelete(primary, context);
206184
}
207-
return this;
208185
}
209186

210-
public ResourceDiscriminator<R, P> getResourceDiscriminator() {
211-
if (this.resourceDiscriminator.isEmpty()) {
212-
return null;
213-
} else {
214-
return this.resourceDiscriminator.get(0);
215-
}
187+
protected void handleDelete(P primary, Context<P> context) {
188+
throw new IllegalStateException("delete method be implemented if Deleter trait is supported");
216189
}
217190

218-
protected int lastKnownBulkSize() {
219-
return resourceDiscriminator.size();
191+
public void setResourceDiscriminator(
192+
ResourceDiscriminator<R, P> resourceDiscriminator) {
193+
this.resourceDiscriminator = resourceDiscriminator;
220194
}
221195

196+
protected boolean isCreatable() {
197+
return creatable;
198+
}
199+
200+
protected boolean isUpdatable() {
201+
return updatable;
202+
}
203+
204+
protected boolean isBulk() {
205+
return bulk;
206+
}
222207
}

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package io.javaoperatorsdk.operator.processing.dependent;
22

3+
import java.util.Optional;
4+
35
import io.fabric8.kubernetes.api.model.HasMetadata;
46
import io.javaoperatorsdk.operator.api.reconciler.Context;
5-
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
67
import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
8+
import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
79

810
/**
911
* Manages dynamic number of resources created for a primary resource. Since the point of a bulk
@@ -30,6 +32,21 @@ public interface BulkDependentResource<R, P extends HasMetadata> extends Creator
3032
*/
3133
void deleteBulkResourceWithIndex(P primary, R resource, int i, Context<P> context);
3234

33-
ResourceDiscriminator<R, P> getResourceDiscriminator(int index);
35+
/**
36+
* Determines whether the specified secondary resource matches the desired state with target index
37+
* of a bulk resource as defined from the specified primary resource, given the specified
38+
* {@link Context}.
39+
*
40+
* @param actualResource the resource we want to determine whether it's matching the desired state
41+
* @param primary the primary resource from which the desired state is inferred
42+
* @param context the context in which the resource is being matched
43+
* @return a {@link Result} encapsulating whether the resource matched its desired state and this
44+
* associated state if it was computed as part of the matching process. Use the static
45+
* convenience methods ({@link Result#nonComputed(boolean)} and
46+
* {@link Result#computed(boolean, Object)})
47+
*/
48+
Result<R> match(R actualResource, P primary, int index, Context<P> context);
49+
50+
Optional<R> getSecondaryResource(P primary, int index, Context<P> context);
3451

3552
}

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@
1313
public interface BulkUpdater<R, P extends HasMetadata> extends Updater<R, P> {
1414

1515
default Matcher.Result<R> match(R actualResource, P primary, Context<P> context) {
16-
throw new IllegalStateException();
16+
if (!(this instanceof BulkDependentResource)) {
17+
throw new IllegalStateException(
18+
BulkUpdater.class.getSimpleName() + " interface should only be implemented by "
19+
+ BulkDependentResource.class.getSimpleName() + " implementations");
20+
}
21+
throw new IllegalStateException("This method should not be called from a "
22+
+ BulkDependentResource.class.getSimpleName() + " implementation");
1723
}
18-
19-
Matcher.Result<R> match(R actualResource, P primary, int index, Context<P> context);
2024
}

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,4 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
1616
var desired = abstractDependentResource.desired(primary, context);
1717
return Result.computed(actualResource.equals(desired), desired);
1818
}
19-
20-
@Override
21-
public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
22-
var desired = abstractDependentResource.desired(primary, index, context);
23-
return Result.computed(actualResource.equals(desired), desired);
24-
}
2519
}

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,19 +95,4 @@ public Optional<T> computedDesired() {
9595
* {@link Result#computed(boolean, Object)})
9696
*/
9797
Result<R> match(R actualResource, P primary, Context<P> context);
98-
99-
/**
100-
* Determines whether the specified secondary resource matches the desired state with target index
101-
* of a bulk resource as defined from the specified primary resource, given the specified
102-
* {@link Context}.
103-
*
104-
* @param actualResource the resource we want to determine whether it's matching the desired state
105-
* @param primary the primary resource from which the desired state is inferred
106-
* @param context the context in which the resource is being matched
107-
* @return a {@link Result} encapsulating whether the resource matched its desired state and this
108-
* associated state if it was computed as part of the matching process. Use the static
109-
* convenience methods ({@link Result#nonComputed(boolean)} and
110-
* {@link Result#computed(boolean, Object)})
111-
*/
112-
Result<R> match(R actualResource, P primary, int index, Context<P> context);
11398
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,4 @@ public interface Updater<R, P extends HasMetadata> {
88
R update(R actual, R desired, P primary, Context<P> context);
99

1010
Result<R> match(R actualResource, P primary, Context<P> context);
11-
12-
default Result<R> match(R actualResource, P primary, int index, Context<P> context) {
13-
throw new IllegalStateException("Implement this for bulk matching");
14-
}
1511
}

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

Lines changed: 24 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -23,47 +23,7 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> depen
2323
@SuppressWarnings({"unchecked", "rawtypes"})
2424
static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
2525
Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
26-
if (Secret.class.isAssignableFrom(resourceType)) {
27-
return new Matcher<>() {
28-
@Override
29-
public Result<R> match(R actualResource, P primary, Context<P> context) {
30-
final var desired = dependentResource.desired(primary, context);
31-
return Result.computed(
32-
ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
33-
desired);
34-
}
35-
36-
@Override
37-
public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
38-
final var desired = dependentResource.desired(primary, index, context);
39-
return Result.computed(
40-
ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
41-
desired);
42-
}
43-
};
44-
} else if (ConfigMap.class.isAssignableFrom(resourceType)) {
45-
return new Matcher<>() {
46-
@Override
47-
public Result<R> match(R actualResource, P primary, Context<P> context) {
48-
final var desired = dependentResource.desired(primary, context);
49-
return Result.computed(
50-
ResourceComparators.compareConfigMapData((ConfigMap) desired,
51-
(ConfigMap) actualResource),
52-
desired);
53-
}
54-
55-
@Override
56-
public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
57-
final var desired = dependentResource.desired(primary, index, context);
58-
return Result.computed(
59-
ResourceComparators.compareConfigMapData((ConfigMap) desired,
60-
(ConfigMap) actualResource),
61-
desired);
62-
}
63-
};
64-
} else {
65-
return new GenericKubernetesResourceMatcher(dependentResource);
66-
}
26+
return new GenericKubernetesResourceMatcher(dependentResource);
6727
}
6828

6929
@Override
@@ -72,14 +32,8 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
7232
return match(desired, actualResource, false);
7333
}
7434

75-
@Override
76-
public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
77-
var desired = dependentResource.desired(primary, index, context);
78-
return match(desired, actualResource, false);
79-
}
80-
81-
public static <R extends HasMetadata> Result<R> match(
82-
R desired, R actualResource, boolean considerMetadata) {
35+
public static <R extends HasMetadata> Result<R> match(R desired, R actualResource,
36+
boolean considerMetadata) {
8337
if (considerMetadata) {
8438
final var desiredMetadata = desired.getMetadata();
8539
final var actualMetadata = actualResource.getMetadata();
@@ -91,20 +45,30 @@ public static <R extends HasMetadata> Result<R> match(
9145
}
9246
}
9347

94-
final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper();
48+
if (desired instanceof ConfigMap) {
49+
return Result.computed(
50+
ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actualResource),
51+
desired);
52+
} else if (desired instanceof Secret) {
53+
return Result.computed(
54+
ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
55+
desired);
56+
} else {
57+
final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper();
9558

96-
// reflection will be replaced by this:
97-
// https://github.com/fabric8io/kubernetes-client/issues/3816
98-
var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
99-
var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
100-
var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
101-
for (int i = 0; i < diffJsonPatch.size(); i++) {
102-
String operation = diffJsonPatch.get(i).get("op").asText();
103-
if (!operation.equals("add")) {
104-
return Result.computed(false, desired);
59+
// reflection will be replaced by this:
60+
// https://github.com/fabric8io/kubernetes-client/issues/3816
61+
var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
62+
var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
63+
var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
64+
for (int i = 0; i < diffJsonPatch.size(); i++) {
65+
String operation = diffJsonPatch.get(i).get("op").asText();
66+
if (!operation.equals("add")) {
67+
return Result.computed(false, desired);
68+
}
10569
}
70+
return Result.computed(true, desired);
10671
}
107-
return Result.computed(true, desired);
10872
}
10973

11074
/**

0 commit comments

Comments
 (0)