Skip to content

rough in of tracking things that are defaults and normalizations #2766

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

rough in of tracking things that are defaults and normalizations #2766

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
20 changes: 14 additions & 6 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 @@ -69,12 +69,18 @@ public R create(R desired, P primary, Context<P> context) {
}
addMetadata(false, null, desired, primary, context);
final var resource = prepare(context, desired, primary, "Creating");
return useSSA(context)
? resource
.fieldManager(context.getControllerConfiguration().fieldManager())
.forceConflicts()
.serverSideApply()
: resource.create();
var result =
useSSA(context)
? resource
.fieldManager(context.getControllerConfiguration().fieldManager())
.forceConflicts()
.serverSideApply()
: resource.create();
if (useSSA(context)) {
SSABasedGenericKubernetesResourceMatcher.getInstance()
.findDefaultsAndNormalizations(result, desired, context);
}
return result;
}

public R update(R actual, R desired, P primary, Context<P> context) {
Expand All @@ -92,6 +98,8 @@ public R update(R actual, R desired, P primary, Context<P> context) {
.fieldManager(context.getControllerConfiguration().fieldManager())
.forceConflicts()
.serverSideApply();
SSABasedGenericKubernetesResourceMatcher.getInstance()
.findDefaultsAndNormalizations(actual, desired, context);
} else {
var updatedActual = GenericResourceUpdater.updateResource(actual, desired, context);
updatedResource = prepare(context, updatedActual, primary, "Updating").update();
Expand Down
148 changes: 131 additions & 17 deletions ...dk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -27,6 +28,7 @@
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.LoggingUtils;
import io.javaoperatorsdk.operator.processing.event.ResourceID;

import com.github.difflib.DiffUtils;
import com.github.difflib.UnifiedDiffUtils;
Expand Down Expand Up @@ -60,7 +62,13 @@ public class SSABasedGenericKubernetesResourceMatcher<R extends HasMetadata> {
new SSABasedGenericKubernetesResourceMatcher<>();

private static final List<String> IGNORED_METADATA =
List.of("creationTimestamp", "deletionTimestamp", "generation", "selfLink", "uid");
List.of(
"creationTimestamp",
"deletionTimestamp",
"generation",
"selfLink",
"uid",
"resourceVersion");

@SuppressWarnings("unchecked")
public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L> getInstance() {
Expand All @@ -79,16 +87,134 @@ public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L
private static final Logger log =
LoggerFactory.getLogger(SSABasedGenericKubernetesResourceMatcher.class);

@SuppressWarnings("unchecked")
record SSAState(Map<String, Object> desired, Map<String, Object> actual) {}

record CacheKey(ResourceID id, int hash) {}

private LinkedHashMap<CacheKey, SSAState> defaultsAndNormalizations =
new LinkedHashMap<CacheKey, SSAState>();

public void findDefaultsAndNormalizations(R actual, R desired, Context<?> context) {
SSAState state = getSSAState(actual, desired, context);

if (state == null) {
// exception?
}

// minimize by removing eveything that appears in both
minimizeMaps(state.desired, state.actual);

if (!state.desired.isEmpty() || !state.actual.isEmpty()) {
defaultsAndNormalizations.put(
new CacheKey(ResourceID.fromResource(actual), desired.hashCode()), state);
}
}

void minimizeMaps(Map<String, Object> actual, Map<String, Object> desired) {
for (Iterator<Map.Entry<String, Object>> iter = desired.entrySet().iterator();
iter.hasNext(); ) {
var entry = iter.next();
var desiredValue = entry.getValue();
var actualValue = actual.get(entry.getKey());
if (Objects.equals(desiredValue, actualValue)) {
iter.remove();
actual.remove(entry.getKey());
} else if (desiredValue instanceof Map dm) {
if (actualValue instanceof Map am) {
minimizeMaps(am, dm);
if (am.isEmpty()) {
actual.remove(entry.getKey());
}
}
if (dm.isEmpty()) {
iter.remove();
}
} else if (desiredValue instanceof List dl) {
if (actualValue instanceof List al) {
minimizeLists(al, dl);
if ((dl.isEmpty() || dl.stream().allMatch(Objects::isNull))
&& (al.isEmpty() || al.stream().allMatch(Objects::isNull))) {
iter.remove();
actual.remove(entry.getKey());
}
}
}
}
}

void minimizeLists(List<Object> actual, List<Object> desired) {
for (int i = 0; i < desired.size(); i++) {
Object desiredValue = desired.get(i);
if (actual.size() > i) {
Object actualValue = actual.get(i);
if (Objects.equals(desiredValue, actualValue)) {
actual.set(i, null);
desired.set(i, null);
} else if (desiredValue instanceof Map dm) {
if (actualValue instanceof Map am) {
minimizeMaps(am, dm);
if (am.isEmpty()) {
actual.set(i, null);
}
}
if (dm.isEmpty()) {
desired.set(i, null);
}
} else if (desiredValue instanceof List dl) {
if (actualValue instanceof List al) {
minimizeLists(al, dl);
if ((dl.isEmpty() || dl.stream().allMatch(Objects::isNull))
&& (al.isEmpty() || al.stream().allMatch(Objects::isNull))) {
actual.set(i, null);
desired.set(i, null);
}
}
}
}
}
}

public boolean matches(R actual, R desired, Context<?> context) {
SSAState state = getSSAState(actual, desired, context);

if (state == null) {
return false;
}

SSAState overrides = defaultsAndNormalizations.get(new CacheKey(ResourceID.fromResource(actual), desired.hashCode()));
if (overrides != null) {
// if containsAll(overrides.desired, state.desired) && containsAll(overrides.actual, state.actual)
Copy link
Collaborator

@csviri csviri Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I assume if this is not true just return false?

Copy link
Collaborator

@csviri csviri Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If it is true might not always mean match, or?

Copy link
Collaborator Author

@shawkins shawkins Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I assume here goes return true?

It wouldn't yet because the maps have been minimized they will only contain entries that represent normalizations, or defaults - it's basically a diff, but in map form. So an entry that is only in the actual map, that would be a default, and an entry that appears in both but with different values is a normalization.

We know that the desired state matches up to the java hash, but to make sure we're not seeing a false positive we'll still double check that the desired and actual states here contain everything from the diffs - if they do, we can prune all that and proceed with the rest of the comparison - reasonably, but possibly not completely, assured that we've made the correct assumption.

I'm minimizing the maps because I'm not sure we know how long we'll have to hold on to these entries to prevent the looping behavior. The temporary resource cache and previous annotation will prevent us from consuming our own modification, but if another entity modifies (or increments the resourceVersion in any way) the resource, that will trigger our reconciliation and increment of the previous annotation and so forth. If we think that the number of these we would have to track is sufficiently limited, then we could use the full desired state as the key instead of a weak hash.


//
}

var matches = matches(state.actual(), state.desired(), actual, desired, context);
if (!matches && log.isDebugEnabled() && LoggingUtils.isNotSensitiveResource(desired)) {
var diff =
getDiff(
state.actual(), state.desired(), context.getClient().getKubernetesSerialization());
log.debug(
"Diff between actual and desired state for resource: {} with name: {} in namespace: {}"
+ " is:\n"
+ "{}",
actual.getKind(),
actual.getMetadata().getName(),
actual.getMetadata().getNamespace(),
diff);
}
return matches;
}

@SuppressWarnings("unchecked")
private SSAState getSSAState(R actual, R desired, Context<?> context) {
var optionalManagedFieldsEntry =
checkIfFieldManagerExists(actual, context.getControllerConfiguration().fieldManager());
// If no field is managed by our controller, that means the controller hasn't touched the
// resource yet and the resource probably doesn't match the desired state. Not matching here
// means that the resource will need to be updated and since this will be done using SSA, the
// fields our controller cares about will become managed by it
if (optionalManagedFieldsEntry.isEmpty()) {
return false;
return null;
}

var managedFieldsEntry = optionalManagedFieldsEntry.orElseThrow();
Expand All @@ -109,20 +235,8 @@ public boolean matches(R actual, R desired, Context<?> context) {
objectMapper);

removeIrrelevantValues(desiredMap);

var matches = matches(prunedActual, desiredMap, actual, desired, context);
if (!matches && log.isDebugEnabled() && LoggingUtils.isNotSensitiveResource(desired)) {
var diff = getDiff(prunedActual, desiredMap, objectMapper);
log.debug(
"Diff between actual and desired state for resource: {} with name: {} in namespace: {}"
+ " is:\n"
+ "{}",
actual.getKind(),
actual.getMetadata().getName(),
actual.getMetadata().getNamespace(),
diff);
}
return matches;
removeIrrelevantValues(actualMap);
return new SSAState(desiredMap, prunedActual);
}

/**
Expand Down
Loading