Skip to content

feat: cache desired state #2831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: cache desired state #2831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 46 additions & 31 deletions ...main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
private final boolean updatable = this instanceof Updater;
private final boolean deletable = this instanceof Deleter;
private final DependentResourceReconciler<R, P> dependentResourceReconciler;
private final ThreadLocal<R> desiredCache = new ThreadLocal<>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if using ThreadLocal is the best for this purpose, if some later call the dependent from reconcile method it will happen on different thread so desired won't be available

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not sure either if that's the proper way to do it…

protected Creator<R, P> creator;
protected Updater<R, P> updater;
protected String name;
Expand Down Expand Up @@ -67,52 +68,56 @@ public ReconcileResult<R> reconcile(P primary, Context<P> context) {
}

protected ReconcileResult<R> reconcile(P primary, R actualResource, Context<P> context) {
if (creatable() || updatable()) {
if (actualResource == null) {
if (creatable) {
var desired = desired(primary, context);
throwIfNull(desired, primary, "Desired");
logForOperation("Creating", primary, desired);
var createdResource = handleCreate(desired, primary, context);
return ReconcileResult.resourceCreated(createdResource);
}
} else {
if (updatable()) {
final Matcher.Result<R> match = match(actualResource, primary, context);
if (!match.matched()) {
final var desired = match.computedDesired().orElseGet(() -> desired(primary, context));
try {
if (creatable() || updatable()) {
if (actualResource == null) {
if (creatable) {
var desired = cachedDesired(primary, context);
throwIfNull(desired, primary, "Desired");
logForOperation("Updating", primary, desired);
var updatedResource = handleUpdate(actualResource, desired, primary, context);
return ReconcileResult.resourceUpdated(updatedResource);
logForOperation("Creating", primary, desired);
var createdResource = handleCreate(desired, primary, context);
return ReconcileResult.resourceCreated(createdResource);
}
} else {
if (updatable()) {
final Result<R> match = match(actualResource, primary, context);
if (!match.matched()) {
final var desired =
match.computedDesired().orElseGet(() -> cachedDesired(primary, context));
throwIfNull(desired, primary, "Desired");
logForOperation("Updating", primary, desired);
var updatedResource = handleUpdate(actualResource, desired, primary, context);
return ReconcileResult.resourceUpdated(updatedResource);
} else {
log.debug(
"Update skipped for dependent {} as it matched the existing one",
actualResource instanceof HasMetadata
? ResourceID.fromResource((HasMetadata) actualResource)
: getClass().getSimpleName());
}
} else {
log.debug(
"Update skipped for dependent {} as it matched the existing one",
"Update skipped for dependent {} implement Updater interface to modify it",
actualResource instanceof HasMetadata
? ResourceID.fromResource((HasMetadata) actualResource)
: getClass().getSimpleName());
}
} else {
log.debug(
"Update skipped for dependent {} implement Updater interface to modify it",
actualResource instanceof HasMetadata
? ResourceID.fromResource((HasMetadata) actualResource)
: getClass().getSimpleName());
}
} else {
log.debug(
"Dependent {} is read-only, implement Creator and/or Updater interfaces to modify it",
getClass().getSimpleName());
}
} else {
log.debug(
"Dependent {} is read-only, implement Creator and/or Updater interfaces to modify it",
getClass().getSimpleName());
return ReconcileResult.noOperation(actualResource);
} finally {
desiredCache.remove();
}
return ReconcileResult.noOperation(actualResource);
}

public abstract Result<R> match(R resource, P primary, Context<P> context);

@Override
public Optional<R> getSecondaryResource(P primary, Context<P> context) {

var secondaryResources = context.getSecondaryResources(resourceType());
if (secondaryResources.isEmpty()) {
return Optional.empty();
Expand All @@ -136,7 +141,7 @@ public Optional<R> getSecondaryResource(P primary, Context<P> context) {
*/
protected Optional<R> selectTargetSecondaryResource(
Set<R> secondaryResources, P primary, Context<P> context) {
R desired = desired(primary, context);
R desired = cachedDesired(primary, context);
var targetResources = secondaryResources.stream().filter(r -> r.equals(desired)).toList();
if (targetResources.size() > 1) {
throw new IllegalStateException(
Expand Down Expand Up @@ -199,6 +204,16 @@ protected R handleUpdate(R actual, R desired, P primary, Context<P> context) {
return updated;
}

protected R cachedDesired(P primary, Context<P> context) {
var desired = desiredCache.get();
if (desired != null) {
return desired;
}
desired = desired(primary, context);
desiredCache.set(desired);
return desired;
}

protected R desired(P primary, Context<P> context) {
throw new IllegalStateException(
"desired method must be implemented if this DependentResource can be created and/or"
Expand Down
2 changes: 1 addition & 1 deletion ...a/io/javaoperatorsdk/operator/processing/dependent/AbstractExternalDependentResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ protected void handleExplicitStateCreation(P primary, R created, Context<P> cont

@Override
public Matcher.Result<R> match(R resource, P primary, Context<P> context) {
var desired = desired(primary, context);
var desired = cachedDesired(primary, context);
return Matcher.Result.computed(resource.equals(desired), desired);
}

Expand Down
4 changes: 2 additions & 2 deletions ...peratorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public static <R extends HasMetadata, P extends HasMetadata> Matcher.Result<R> m
Context<P> context,
boolean labelsAndAnnotationsEquality,
String... ignorePaths) {
final var desired = dependentResource.desired(primary, context);
final var desired = dependentResource.cachedDesired(primary, context);
return match(desired, actualResource, labelsAndAnnotationsEquality, context, ignorePaths);
}

Expand All @@ -135,7 +135,7 @@ public static <R extends HasMetadata, P extends HasMetadata> Matcher.Result<R> m
boolean specEquality,
boolean labelsAndAnnotationsEquality,
String... ignorePaths) {
final var desired = dependentResource.desired(primary, context);
final var desired = dependentResource.cachedDesired(primary, context);
return match(
desired, actualResource, labelsAndAnnotationsEquality, specEquality, context, ignorePaths);
}
Expand Down
8 changes: 4 additions & 4 deletions ...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public R update(R actual, R desired, P primary, Context<P> context) {

@Override
public Result<R> match(R actualResource, P primary, Context<P> context) {
final var desired = desired(primary, context);
final var desired = cachedDesired(primary, context);
return match(actualResource, desired, primary, context);
}

Expand Down Expand Up @@ -286,16 +286,16 @@ protected Optional<R> selectTargetSecondaryResource(
* @return id of the target managed resource
*/
protected ResourceID targetSecondaryResourceID(P primary, Context<P> context) {
return ResourceID.fromResource(desired(primary, context));
return ResourceID.fromResource(cachedDesired(primary, context));
}

protected boolean addOwnerReference() {
return garbageCollected;
}

@Override
protected R desired(P primary, Context<P> context) {
return super.desired(primary, context);
protected R cachedDesired(P primary, Context<P> context) {
return super.cachedDesired(primary, context);
}

@Override
Expand Down