Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit e0776a4

Browse files
csvirimetacosm
csviri
and
metacosm
committedOct 19, 2022
refactor: improve bulk dependent resource api (#1525)
* feat: decouple event source from cache + list discriminator (#1378) * feat: bulk dependent resources (#1448) * feat: optional eventsource on dependent resources (#1479) * refactor: simplify handling of reused event sources (#1518) Co-authored-by: Chris Laprun <[email protected]> * refactor: isolate index handling to BulkDependentResource interface (#1517) * feat: key based bulk resource creation (#1521) * improvement: bulk dependent resource api * merge Co-authored-by: Chris Laprun <[email protected]> Co-authored-by: Chris Laprun <[email protected]>
1 parent 919f61c commit e0776a4

File tree

5 files changed

+37
-42
lines changed

5 files changed

+37
-42
lines changed
 

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

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,22 @@ protected AbstractDependentResource() {
3939
@Override
4040
public ReconcileResult<R> reconcile(P primary, Context<P> context) {
4141
if (bulk) {
42-
final var targetKeys = bulkDependentResource.targetKeys(primary, context);
42+
final var targetResources = bulkDependentResource.desiredResources(primary, context);
43+
4344
Map<String, R> actualResources =
4445
bulkDependentResource.getSecondaryResources(primary, context);
4546

46-
deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context);
47-
final List<ReconcileResult<R>> results = new ArrayList<>(targetKeys.size());
47+
deleteBulkResourcesIfRequired(targetResources.keySet(), actualResources, primary, context);
48+
final List<ReconcileResult<R>> results = new ArrayList<>(targetResources.size());
49+
50+
targetResources.forEach((key, resource) -> {
51+
results.add(reconcileIndexAware(primary, actualResources.get(key), resource, key, context));
52+
});
4853

49-
for (String key : targetKeys) {
50-
results.add(reconcileIndexAware(primary, actualResources.get(key), key, context));
51-
}
5254
return ReconcileResult.aggregatedResult(results);
5355
} else {
5456
var actualResource = getSecondaryResource(primary, context);
55-
return reconcileIndexAware(primary, actualResource.orElse(null), null, context);
57+
return reconcileIndexAware(primary, actualResource.orElse(null), null, null, context);
5658
}
5759
}
5860

@@ -66,12 +68,13 @@ protected void deleteBulkResourcesIfRequired(Set targetKeys, Map<String, R> actu
6668
});
6769
}
6870

69-
protected ReconcileResult<R> reconcileIndexAware(P primary, R resource, String key,
71+
protected ReconcileResult<R> reconcileIndexAware(P primary, R actualResource, R desiredResource,
72+
String key,
7073
Context<P> context) {
7174
if (creatable || updatable) {
72-
if (resource == null) {
75+
if (actualResource == null) {
7376
if (creatable) {
74-
var desired = desiredIndexAware(primary, key, context);
77+
var desired = bulkAwareDesired(primary, desiredResource, context);
7578
throwIfNull(desired, primary, "Desired");
7679
logForOperation("Creating", primary, desired);
7780
var createdResource = handleCreate(desired, primary, context);
@@ -81,32 +84,34 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, R resource, String k
8184
if (updatable) {
8285
final Matcher.Result<R> match;
8386
if (bulk) {
84-
match = bulkDependentResource.match(resource, primary, key, context);
87+
match =
88+
bulkDependentResource.match(actualResource, desiredResource, primary, key, context);
8589
} else {
86-
match = updater.match(resource, primary, context);
90+
match = updater.match(actualResource, primary, context);
8791
}
8892
if (!match.matched()) {
8993
final var desired =
90-
match.computedDesired().orElse(desiredIndexAware(primary, key, context));
94+
match.computedDesired().orElse(bulkAwareDesired(primary, desiredResource, context));
9195
throwIfNull(desired, primary, "Desired");
9296
logForOperation("Updating", primary, desired);
93-
var updatedResource = handleUpdate(resource, desired, primary, context);
97+
var updatedResource = handleUpdate(actualResource, desired, primary, context);
9498
return ReconcileResult.resourceUpdated(updatedResource);
9599
}
96100
} else {
97-
log.debug("Update skipped for dependent {} as it matched the existing one", resource);
101+
log.debug("Update skipped for dependent {} as it matched the existing one",
102+
actualResource);
98103
}
99104
}
100105
} else {
101106
log.debug(
102107
"Dependent {} is read-only, implement Creator and/or Updater interfaces to modify it",
103108
getClass().getSimpleName());
104109
}
105-
return ReconcileResult.noOperation(resource);
110+
return ReconcileResult.noOperation(actualResource);
106111
}
107112

108-
private R desiredIndexAware(P primary, String key, Context<P> context) {
109-
return bulk ? bulkDependentResource.desired(primary, key, context)
113+
private R bulkAwareDesired(P primary, R alreadyComputedDesire, Context<P> context) {
114+
return bulk ? alreadyComputedDesire
110115
: desired(primary, context);
111116
}
112117

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

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

33
import java.util.Map;
4-
import java.util.Set;
54

65
import io.fabric8.kubernetes.api.model.HasMetadata;
76
import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -20,12 +19,10 @@ public interface BulkDependentResource<R, P extends HasMetadata>
2019
/**
2120
* @return number of resources to create
2221
*/
23-
Set<String> targetKeys(P primary, Context<P> context);
22+
Map<String, R> desiredResources(P primary, Context<P> context);
2423

2524
Map<String, R> getSecondaryResources(P primary, Context<P> context);
2625

27-
R desired(P primary, String key, Context<P> context);
28-
2926
/**
3027
* Used to delete resource if the desired count is lower than the actual count of a resource.
3128
*
@@ -50,6 +47,6 @@ public interface BulkDependentResource<R, P extends HasMetadata>
5047
* convenience methods ({@link Result#nonComputed(boolean)} and
5148
* {@link Result#computed(boolean, Object)})
5249
*/
53-
Result<R> match(R actualResource, P primary, String key, Context<P> context);
50+
Result<R> match(R actualResource, R desired, P primary, String key, Context<P> context);
5451

5552
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,17 +139,15 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
139139
return matcher.match(actualResource, primary, context);
140140
}
141141

142-
public Result<R> match(R actualResource, P primary, String key, Context<P> context) {
143-
final var desired = bulkDependentResource.desired(primary, key, context);
144-
return GenericKubernetesResourceMatcher.match((R) desired, actualResource, false);
142+
public Result<R> match(R actualResource, R desired, P primary, String key, Context<P> context) {
143+
return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
145144
}
146145

147146
protected void handleDelete(P primary, Context<P> context) {
148147
var resource = getSecondaryResource(primary, context);
149148
resource.ifPresent(r -> client.resource(r).delete());
150149
}
151150

152-
153151
public void deleteBulkResource(P primary, R resource, String key, Context<P> context) {
154152
client.resource(resource).delete();
155153
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,17 @@ public ConfigMapDeleterBulkDependentResource() {
3232
}
3333

3434
@Override
35-
public Set<String> targetKeys(BulkDependentTestCustomResource primary,
35+
public Map<String, ConfigMap> desiredResources(BulkDependentTestCustomResource primary,
3636
Context<BulkDependentTestCustomResource> context) {
3737
var number = primary.getSpec().getNumberOfResources();
38-
Set<String> res = new HashSet<>();
38+
Map<String, ConfigMap> res = new HashMap<>();
3939
for (int i = 0; i < number; i++) {
40-
res.add(Integer.toString(i));
40+
var key = Integer.toString(i);
41+
res.put(key, desired(primary, key, context));
4142
}
4243
return res;
4344
}
4445

45-
@Override
4646
public ConfigMap desired(BulkDependentTestCustomResource primary, String key,
4747
Context<BulkDependentTestCustomResource> context) {
4848
ConfigMap configMap = new ConfigMap();

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,14 @@ private ResourceID toResourceID(ExternalResource externalResource) {
6060
}
6161

6262
@Override
63-
public Set<String> targetKeys(BulkDependentTestCustomResource primary,
63+
public Map<String, ExternalResource> desiredResources(BulkDependentTestCustomResource primary,
6464
Context<BulkDependentTestCustomResource> context) {
6565
var number = primary.getSpec().getNumberOfResources();
66-
Set<String> res = new HashSet<>();
66+
Map<String, ExternalResource> res = new HashMap<>();
6767
for (int i = 0; i < number; i++) {
68-
res.add(Integer.toString(i));
68+
var key = Integer.toString(i);
69+
res.put(key, new ExternalResource(toExternalResourceId(primary, key),
70+
primary.getSpec().getAdditionalData()));
6971
}
7072
return res;
7173
}
@@ -84,13 +86,6 @@ public Map<String, ExternalResource> getSecondaryResources(
8486
r -> r));
8587
}
8688

87-
@Override
88-
public ExternalResource desired(BulkDependentTestCustomResource primary, String key,
89-
Context<BulkDependentTestCustomResource> context) {
90-
return new ExternalResource(toExternalResourceId(primary, key),
91-
primary.getSpec().getAdditionalData());
92-
}
93-
9489
@Override
9590
public void deleteBulkResource(BulkDependentTestCustomResource primary, ExternalResource resource,
9691
String key,
@@ -100,9 +95,9 @@ public void deleteBulkResource(BulkDependentTestCustomResource primary, External
10095

10196
@Override
10297
public Matcher.Result<ExternalResource> match(ExternalResource actualResource,
98+
ExternalResource desired,
10399
BulkDependentTestCustomResource primary, String index,
104100
Context<BulkDependentTestCustomResource> context) {
105-
var desired = desired(primary, index, context);
106101
return Matcher.Result.computed(desired.equals(actualResource), desired);
107102
}
108103
}

0 commit comments

Comments
 (0)
Please sign in to comment.