Skip to content

Commit a6fea70

Browse files
committed
refactor: isolate index handling to BulkDependentResource interface
1 parent 79c63fb commit a6fea70

File tree

10 files changed

+108
-159
lines changed

10 files changed

+108
-159
lines changed

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

Lines changed: 40 additions & 54 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;
@@ -22,16 +20,16 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
2220
implements DependentResource<R, P> {
2321
private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class);
2422

25-
protected final boolean creatable = this instanceof Creator;
26-
protected final boolean updatable = this instanceof Updater;
27-
protected final boolean bulk = this instanceof BulkDependentResource;
23+
private final boolean creatable = this instanceof Creator;
24+
private final boolean updatable = this instanceof Updater;
25+
private final boolean bulk = this instanceof BulkDependentResource;
2826

2927
protected Creator<R, P> creator;
3028
protected Updater<R, P> updater;
3129
protected BulkDependentResource<R, P> bulkDependentResource;
3230
private boolean returnEventSource = true;
33-
34-
protected List<ResourceDiscriminator<R, P>> resourceDiscriminator = new ArrayList<>(1);
31+
private ResourceDiscriminator<R, P> resourceDiscriminator;
32+
private int currentCount;
3533

3634
@SuppressWarnings("unchecked")
3735
public AbstractDependentResource() {
@@ -62,48 +60,33 @@ protected abstract ResourceEventSource<R, P> provideEventSource(
6260
public ReconcileResult<R> reconcile(P primary, Context<P> context) {
6361
if (bulk) {
6462
final var count = bulkDependentResource.count(primary, context);
65-
deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context);
66-
adjustDiscriminators(count);
63+
deleteBulkResourcesIfRequired(count, primary, context);
6764
@SuppressWarnings("unchecked")
6865
final ReconcileResult<R>[] results = new ReconcileResult[count];
6966
for (int i = 0; i < count; i++) {
7067
results[i] = reconcileIndexAware(primary, i, context);
7168
}
69+
currentCount = count;
7270
return ReconcileResult.aggregatedResult(results);
7371
} else {
7472
return reconcileIndexAware(primary, 0, context);
7573
}
7674
}
7775

78-
protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary,
79-
Context<P> context) {
80-
if (targetCount >= actualCount) {
76+
protected void deleteBulkResourcesIfRequired(int targetCount, P primary, Context<P> context) {
77+
if (targetCount >= currentCount) {
8178
return;
8279
}
83-
for (int i = targetCount; i < actualCount; i++) {
84-
var resource = getSecondaryResourceIndexAware(primary, i, context);
80+
for (int i = targetCount; i < currentCount; i++) {
81+
var resource = bulkDependentResource.getSecondaryResource(primary, i, context);
8582
var index = i;
8683
resource.ifPresent(
8784
r -> bulkDependentResource.deleteBulkResourceWithIndex(primary, r, index, context));
8885
}
8986
}
9087

91-
private void adjustDiscriminators(int count) {
92-
if (resourceDiscriminator.size() == count) {
93-
return;
94-
}
95-
if (resourceDiscriminator.size() < count) {
96-
for (int i = resourceDiscriminator.size(); i < count; i++) {
97-
resourceDiscriminator.add(bulkDependentResource.getResourceDiscriminator(i));
98-
}
99-
}
100-
if (resourceDiscriminator.size() > count) {
101-
resourceDiscriminator.subList(count, resourceDiscriminator.size()).clear();
102-
}
103-
}
104-
10588
protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> context) {
106-
Optional<R> maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context)
89+
Optional<R> maybeActual = bulk ? bulkDependentResource.getSecondaryResource(primary, i, context)
10790
: getSecondaryResource(primary, context);
10891
if (creatable || updatable) {
10992
if (maybeActual.isEmpty()) {
@@ -119,7 +102,7 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
119102
if (updatable) {
120103
final Matcher.Result<R> match;
121104
if (bulk) {
122-
match = updater.match(actual, primary, i, context);
105+
match = bulkDependentResource.match(actual, primary, i, context);
123106
} else {
124107
match = updater.match(actual, primary, context);
125108
}
@@ -144,17 +127,12 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
144127
}
145128

146129
private R desiredIndexAware(P primary, int i, Context<P> context) {
147-
return bulk ? desired(primary, i, context)
148-
: desired(primary, context);
130+
return bulk ? desired(primary, i, context) : desired(primary, context);
149131
}
150132

151133
public Optional<R> getSecondaryResource(P primary, Context<P> context) {
152-
return resourceDiscriminator.isEmpty() ? context.getSecondaryResource(resourceType())
153-
: resourceDiscriminator.get(0).distinguish(resourceType(), primary, context);
154-
}
155-
156-
protected Optional<R> getSecondaryResourceIndexAware(P primary, int index, Context<P> context) {
157-
return context.getSecondaryResource(resourceType(), resourceDiscriminator.get(index));
134+
return resourceDiscriminator == null ? context.getSecondaryResource(resourceType())
135+
: resourceDiscriminator.distinguish(resourceType(), primary, context);
158136
}
159137

160138
private void throwIfNull(R desired, P primary, String descriptor) {
@@ -192,7 +170,7 @@ protected R handleCreate(R desired, P primary, Context<P> context) {
192170
protected abstract void onCreated(ResourceID primaryResourceId, R created);
193171

194172
/**
195-
* Allows subclasses to perform additional processing on the updated resource if needed.
173+
* Allows sub-classes to perform additional processing on the updated resource if needed.
196174
*
197175
* @param primaryResourceId the {@link ResourceID} of the primary resource associated with the
198176
* newly updated resource
@@ -215,28 +193,36 @@ protected R desired(P primary, Context<P> context) {
215193
}
216194

217195
protected R desired(P primary, int index, Context<P> context) {
218-
throw new IllegalStateException(
219-
"Must be implemented for bulk DependentResource creation");
196+
throw new IllegalStateException("Must be implemented for bulk DependentResource creation");
220197
}
221198

222-
public AbstractDependentResource<R, P> setResourceDiscriminator(
223-
ResourceDiscriminator<R, P> resourceDiscriminator) {
224-
if (resourceDiscriminator != null) {
225-
this.resourceDiscriminator.add(resourceDiscriminator);
199+
public void delete(P primary, Context<P> context) {
200+
if (bulk) {
201+
deleteBulkResourcesIfRequired(0, primary, context);
202+
} else {
203+
handleDelete(primary, context);
226204
}
227-
return this;
228205
}
229206

230-
public ResourceDiscriminator<R, P> getResourceDiscriminator() {
231-
if (this.resourceDiscriminator.isEmpty()) {
232-
return null;
233-
} else {
234-
return this.resourceDiscriminator.get(0);
235-
}
207+
protected void handleDelete(P primary, Context<P> context) {
208+
throw new IllegalStateException("delete method be implemented if Deleter trait is supported");
209+
}
210+
211+
public void setResourceDiscriminator(
212+
ResourceDiscriminator<R, P> resourceDiscriminator) {
213+
this.resourceDiscriminator = resourceDiscriminator;
214+
}
215+
216+
protected boolean isCreatable() {
217+
return creatable;
218+
}
219+
220+
protected boolean isUpdatable() {
221+
return updatable;
236222
}
237223

238-
protected int lastKnownBulkSize() {
239-
return resourceDiscriminator.size();
224+
protected boolean isBulk() {
225+
return bulk;
240226
}
241227

242228
protected boolean getReturnEventSource() {

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: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -24,42 +24,19 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> depen
2424
static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
2525
Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
2626
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-
}
27+
return (actualResource, primary, context) -> {
28+
final var desired = dependentResource.desired(primary, context);
29+
return Result.computed(
30+
ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
31+
desired);
4332
};
4433
} 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-
}
34+
return (actualResource, primary, context) -> {
35+
final var desired = dependentResource.desired(primary, context);
36+
return Result.computed(
37+
ResourceComparators.compareConfigMapData((ConfigMap) desired,
38+
(ConfigMap) actualResource),
39+
desired);
6340
};
6441
} else {
6542
return new GenericKubernetesResourceMatcher(dependentResource);
@@ -72,12 +49,6 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
7249
return match(desired, actualResource, false);
7350
}
7451

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-
8152
public static <R extends HasMetadata> Result<R> match(
8253
R desired, R actualResource, boolean considerMetadata) {
8354
if (considerMetadata) {

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,25 +144,20 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
144144
}
145145

146146
public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
147-
return matcher.match(actualResource, primary, index, context);
147+
final var desired = desired(primary, index, context);
148+
return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
148149
}
149150

150-
public void delete(P primary, Context<P> context) {
151-
if (bulk) {
152-
deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context);
153-
} else {
154-
var resource = getSecondaryResource(primary, context);
155-
resource.ifPresent(r -> client.resource(r).delete());
156-
}
151+
protected void handleDelete(P primary, Context<P> context) {
152+
var resource = getSecondaryResource(primary, context);
153+
resource.ifPresent(r -> client.resource(r).delete());
157154
}
158155

159156
public void deleteBulkResourceWithIndex(P primary, R resource, int i, Context<P> context) {
160157
client.resource(resource).delete();
161158
}
162159

163-
@SuppressWarnings("unchecked")
164-
protected Resource<R> prepare(R desired,
165-
P primary, String actionName) {
160+
protected Resource<R> prepare(R desired, P primary, String actionName) {
166161
log.debug("{} target resource with type: {}, with id: {}",
167162
actionName,
168163
desired.getClass(),
@@ -180,7 +175,6 @@ protected Resource<R> prepare(R desired,
180175
protected InformerEventSource<R, P> createEventSource(EventSourceContext<P> context) {
181176
if (kubernetesDependentResourceConfig != null) {
182177
// sets the filters for the dependent resource, which are applied by parent class
183-
184178
onAddFilter = kubernetesDependentResourceConfig.onAddFilter();
185179
onUpdateFilter = kubernetesDependentResourceConfig.onUpdateFilter();
186180
onDeleteFilter = kubernetesDependentResourceConfig.onDeleteFilter();
@@ -203,7 +197,7 @@ protected InformerEventSource<R, P> createEventSource(EventSourceContext<P> cont
203197
}
204198

205199
private boolean useDefaultAnnotationsToIdentifyPrimary() {
206-
return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && creatable;
200+
return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && isCreatable();
207201
}
208202

209203
private void addDefaultSecondaryToPrimaryMapperAnnotations(R desired, P primary) {

0 commit comments

Comments
 (0)