Skip to content

Commit d30b9f7

Browse files
csviricsviri
committed
Revert "refactor: isolate index handling to BulkDependentResource interface"
This reverts commit e2e6b18.
1 parent e2e6b18 commit d30b9f7

File tree

10 files changed

+156
-106
lines changed

10 files changed

+156
-106
lines changed

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

+54-39
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.javaoperatorsdk.operator.processing.dependent;
22

3+
import java.util.ArrayList;
4+
import java.util.List;
35
import java.util.Optional;
46

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

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

2527
protected Creator<R, P> creator;
2628
protected Updater<R, P> updater;
27-
private final BulkDependentResource<R, P> bulkDependentResource;
28-
private ResourceDiscriminator<R, P> resourceDiscriminator;
29-
private int currentCount;
29+
protected BulkDependentResource<R, P> bulkDependentResource;
30+
31+
private final List<ResourceDiscriminator<R, P>> resourceDiscriminator = new ArrayList<>(1);
3032

3133
@SuppressWarnings("unchecked")
3234
public AbstractDependentResource() {
@@ -40,33 +42,48 @@ public AbstractDependentResource() {
4042
public ReconcileResult<R> reconcile(P primary, Context<P> context) {
4143
if (bulk) {
4244
final var count = bulkDependentResource.count(primary, context);
43-
deleteBulkResourcesIfRequired(count, primary, context);
45+
deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context);
46+
adjustDiscriminators(count);
4447
@SuppressWarnings("unchecked")
4548
final ReconcileResult<R>[] results = new ReconcileResult[count];
4649
for (int i = 0; i < count; i++) {
4750
results[i] = reconcileIndexAware(primary, i, context);
4851
}
49-
currentCount = count;
5052
return ReconcileResult.aggregatedResult(results);
5153
} else {
5254
return reconcileIndexAware(primary, 0, context);
5355
}
5456
}
5557

56-
protected void deleteBulkResourcesIfRequired(int targetCount, P primary, Context<P> context) {
57-
if (targetCount >= currentCount) {
58+
protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary,
59+
Context<P> context) {
60+
if (targetCount >= actualCount) {
5861
return;
5962
}
60-
for (int i = targetCount; i < currentCount; i++) {
61-
var resource = bulkDependentResource.getSecondaryResource(primary, i, context);
63+
for (int i = targetCount; i < actualCount; i++) {
64+
var resource = getSecondaryResourceIndexAware(primary, i, context);
6265
var index = i;
6366
resource.ifPresent(
6467
r -> bulkDependentResource.deleteBulkResourceWithIndex(primary, r, index, context));
6568
}
6669
}
6770

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+
6885
protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> context) {
69-
Optional<R> maybeActual = bulk ? bulkDependentResource.getSecondaryResource(primary, i, context)
86+
Optional<R> maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context)
7087
: getSecondaryResource(primary, context);
7188
if (creatable || updatable) {
7289
if (maybeActual.isEmpty()) {
@@ -82,7 +99,7 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
8299
if (updatable) {
83100
final Matcher.Result<R> match;
84101
if (bulk) {
85-
match = bulkDependentResource.match(actual, primary, i, context);
102+
match = updater.match(actual, primary, i, context);
86103
} else {
87104
match = updater.match(actual, primary, context);
88105
}
@@ -107,12 +124,17 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, int i, Context<P> co
107124
}
108125

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

113131
public Optional<R> getSecondaryResource(P primary, Context<P> context) {
114-
return resourceDiscriminator == null ? context.getSecondaryResource(resourceType())
115-
: resourceDiscriminator.distinguish(resourceType(), primary, 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));
116138
}
117139

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

175197
protected R desired(P primary, int index, Context<P> context) {
176-
throw new IllegalStateException("Must be implemented for bulk DependentResource creation");
177-
}
178-
179-
public void delete(P primary, Context<P> context) {
180-
if (bulk) {
181-
deleteBulkResourcesIfRequired(0, primary, context);
182-
} else {
183-
handleDelete(primary, context);
184-
}
185-
}
186-
187-
protected void handleDelete(P primary, Context<P> context) {
188-
throw new IllegalStateException("delete method be implemented if Deleter trait is supported");
198+
throw new IllegalStateException(
199+
"Must be implemented for bulk DependentResource creation");
189200
}
190201

191-
public void setResourceDiscriminator(
202+
public AbstractDependentResource<R, P> setResourceDiscriminator(
192203
ResourceDiscriminator<R, P> resourceDiscriminator) {
193-
this.resourceDiscriminator = resourceDiscriminator;
204+
if (resourceDiscriminator != null) {
205+
this.resourceDiscriminator.add(resourceDiscriminator);
206+
}
207+
return this;
194208
}
195209

196-
protected boolean isCreatable() {
197-
return creatable;
210+
public ResourceDiscriminator<R, P> getResourceDiscriminator() {
211+
if (this.resourceDiscriminator.isEmpty()) {
212+
return null;
213+
} else {
214+
return this.resourceDiscriminator.get(0);
215+
}
198216
}
199217

200-
protected boolean isUpdatable() {
201-
return updatable;
218+
protected int lastKnownBulkSize() {
219+
return resourceDiscriminator.size();
202220
}
203221

204-
protected boolean isBulk() {
205-
return bulk;
206-
}
207222
}

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

+2-19
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
package io.javaoperatorsdk.operator.processing.dependent;
22

3-
import java.util.Optional;
4-
53
import io.fabric8.kubernetes.api.model.HasMetadata;
64
import io.javaoperatorsdk.operator.api.reconciler.Context;
5+
import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
76
import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
8-
import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
97

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

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);
33+
ResourceDiscriminator<R, P> getResourceDiscriminator(int index);
5134

5235
}

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

+3-7
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,8 @@
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-
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");
16+
throw new IllegalStateException();
2317
}
18+
19+
Matcher.Result<R> match(R actualResource, P primary, int index, Context<P> context);
2420
}

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

+6
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,10 @@ 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+
}
1925
}

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

+15
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,19 @@ 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);
98113
}

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

+4
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,8 @@ 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+
}
1115
}

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

+40-11
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,42 @@ 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 (actualResource, primary, context) -> {
28-
final var desired = dependentResource.desired(primary, context);
29-
return Result.computed(
30-
ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
31-
desired);
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+
}
3243
};
3344
} else if (ConfigMap.class.isAssignableFrom(resourceType)) {
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);
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+
}
4063
};
4164
} else {
4265
return new GenericKubernetesResourceMatcher(dependentResource);
@@ -49,6 +72,12 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
4972
return match(desired, actualResource, false);
5073
}
5174

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+
5281
public static <R extends HasMetadata> Result<R> match(
5382
R desired, R actualResource, boolean considerMetadata) {
5483
if (considerMetadata) {

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

+10-6
Original file line numberDiff line numberDiff line change
@@ -135,19 +135,23 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
135135
}
136136

137137
public Result<R> match(R actualResource, P primary, int index, Context<P> context) {
138-
final var desired = desired(primary, index, context);
139-
return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
138+
return matcher.match(actualResource, primary, index, context);
140139
}
141140

142-
protected void handleDelete(P primary, Context<P> context) {
143-
var resource = getSecondaryResource(primary, context);
144-
resource.ifPresent(r -> client.resource(r).delete());
141+
public void delete(P primary, Context<P> context) {
142+
if (bulk) {
143+
deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context);
144+
} else {
145+
var resource = getSecondaryResource(primary, context);
146+
resource.ifPresent(r -> client.resource(r).delete());
147+
}
145148
}
146149

147150
public void deleteBulkResourceWithIndex(P primary, R resource, int i, Context<P> context) {
148151
client.resource(resource).delete();
149152
}
150153

154+
@SuppressWarnings("unchecked")
151155
protected Resource<R> prepare(R desired, P primary, String actionName) {
152156
log.debug("{} target resource with type: {}, with id: {}",
153157
actionName,
@@ -188,7 +192,7 @@ protected InformerEventSource<R, P> createEventSource(EventSourceContext<P> cont
188192
}
189193

190194
private boolean useDefaultAnnotationsToIdentifyPrimary() {
191-
return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && isCreatable();
195+
return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && creatable;
192196
}
193197

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

0 commit comments

Comments
 (0)