Skip to content

fix: ignore server managed fields for SSA matching #2309

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.

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

Merged
merged 3 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ public class SSABasedGenericKubernetesResourceMatcher<R extends HasMetadata> {
public static final String APPLY_OPERATION = "Apply";
public static final String DOT_KEY = ".";

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

@SuppressWarnings("unchecked")
public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L> getInstance() {
return INSTANCE;
Expand All @@ -58,11 +62,10 @@ public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L
private static final Logger log =
LoggerFactory.getLogger(SSABasedGenericKubernetesResourceMatcher.class);


@SuppressWarnings("unchecked")
public boolean matches(R actual, R desired, Context<?> context) {
var optionalManagedFieldsEntry =
checkIfFieldManagerExists(actual, context.getControllerConfiguration().fieldManager());
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
Expand All @@ -86,7 +89,8 @@ public boolean matches(R actual, R desired, Context<?> context) {

var prunedActual = new HashMap<String, Object>(actualMap.size());
keepOnlyManagedFields(prunedActual, actualMap,
managedFieldsEntry.getFieldsV1().getAdditionalProperties(), objectMapper);
managedFieldsEntry.getFieldsV1().getAdditionalProperties(),
objectMapper);

removeIrrelevantValues(desiredMap);

Expand All @@ -110,9 +114,8 @@ private void sanitizeState(R actual, R desired, Map<String, Object> actualMap) {
for (int i = 0; i < claims; i++) {
if (desiredStatefulSet.getSpec().getVolumeClaimTemplates().get(i).getSpec()
.getVolumeMode() == null) {
Optional
.ofNullable(GenericKubernetesResource.get(actualMap, "spec", "volumeClaimTemplates",
i, "spec"))
Optional.ofNullable(
GenericKubernetesResource.get(actualMap, "spec", "volumeClaimTemplates", i, "spec"))
.map(Map.class::cast).ifPresent(m -> m.remove("volumeMode"));
}
if (desiredStatefulSet.getSpec().getVolumeClaimTemplates().get(i).getStatus() == null) {
Expand All @@ -131,6 +134,7 @@ private static void removeIrrelevantValues(Map<String, Object> desiredMap) {
var metadata = (Map<String, Object>) desiredMap.get(METADATA_KEY);
metadata.remove(NAME_KEY);
metadata.remove(NAMESPACE_KEY);
IGNORED_METADATA.forEach(metadata::remove);
if (metadata.isEmpty()) {
desiredMap.remove(METADATA_KEY);
}
Expand Down Expand Up @@ -163,7 +167,8 @@ private static void keepOnlyManagedFields(Map<String, Object> result,
} else {
// basically if we should traverse further
fillResultsAndTraverseFurther(result, actualMap, managedFields, objectMapper, key,
keyInActual, managedFieldValue);
keyInActual,
managedFieldValue);
}
} else {
// this should handle the case when the value is complex in the actual map (not just a
Expand All @@ -181,8 +186,9 @@ private static void keepOnlyManagedFields(Map<String, Object> result,

@SuppressWarnings("unchecked")
private static void fillResultsAndTraverseFurther(Map<String, Object> result,
Map<String, Object> actualMap, Map<String, Object> managedFields,
KubernetesSerialization objectMapper, String key, String keyInActual,
Map<String, Object> actualMap,
Map<String, Object> managedFields, KubernetesSerialization objectMapper, String key,
String keyInActual,
Object managedFieldValue) {
var emptyMapValue = new HashMap<String, Object>();
result.put(keyInActual, emptyMapValue);
Expand Down Expand Up @@ -223,8 +229,9 @@ private static void handleListKeyEntrySet(Map<String, Object> result,
if (DOT_KEY.equals(listEntry.getKey())) {
continue;
}
var actualListEntry = selectListEntryBasedOnKey(keyWithoutPrefix(listEntry.getKey()),
actualValueList, objectMapper);
var actualListEntry =
selectListEntryBasedOnKey(keyWithoutPrefix(listEntry.getKey()), actualValueList,
objectMapper);
targetValuesByIndex.put(actualListEntry.getKey(), actualListEntry.getValue());
managedEntryByIndex.put(actualListEntry.getKey(), (Map<String, Object>) listEntry.getValue());
}
Expand Down Expand Up @@ -301,8 +308,7 @@ private static boolean isKeyPrefixedSkippingDotKey(Set<Map.Entry<String, Object>
@SuppressWarnings("unchecked")
private static java.util.Map.Entry<Integer, Map<String, Object>> selectListEntryBasedOnKey(
String key,
List<Map<String, Object>> values,
KubernetesSerialization objectMapper) {
List<Map<String, Object>> values, KubernetesSerialization objectMapper) {
Map<String, Object> ids = objectMapper.unmarshal(key, Map.class);
List<Map<String, Object>> possibleTargets = new ArrayList<>(1);
int index = -1;
Expand All @@ -314,9 +320,8 @@ private static java.util.Map.Entry<Integer, Map<String, Object>> selectListEntry
}
}
if (possibleTargets.isEmpty()) {
throw new IllegalStateException(
"Cannot find list element for key:" + key + ", in map: "
+ values.stream().map(Map::keySet).collect(Collectors.toList()));
throw new IllegalStateException("Cannot find list element for key:" + key + ", in map: "
+ values.stream().map(Map::keySet).collect(Collectors.toList()));
}
if (possibleTargets.size() > 1) {
throw new IllegalStateException(
Expand All @@ -327,7 +332,6 @@ private static java.util.Map.Entry<Integer, Map<String, Object>> selectListEntry
return new AbstractMap.SimpleEntry<>(finalIndex, possibleTargets.get(0));
}


private Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String fieldManager) {
var targetManagedFields = actual.getMetadata().getManagedFields().stream()
// Only the apply operations are interesting for us since those were created properly be SSA
Expand All @@ -338,14 +342,14 @@ private Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String
.collect(Collectors.toList());
if (targetManagedFields.isEmpty()) {
log.debug("No field manager exists for resource {} with name: {} and operation Apply ",
actual.getKind(), actual.getMetadata().getName());
actual.getKind(),
actual.getMetadata().getName());
return Optional.empty();
}
// this should not happen in theory
if (targetManagedFields.size() > 1) {
throw new OperatorException(
"More than one field manager exists with name: " + fieldManager + "in resource: " +
actual.getKind() + " with name: " + actual.getMetadata().getName());
throw new OperatorException("More than one field manager exists with name: " + fieldManager
+ "in resource: " + actual.getKind() + " with name: " + actual.getMetadata().getName());
}
return Optional.of(targetManagedFields.get(0));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ metadata:
annotations:
deployment.kubernetes.io/revision: "1"
creationTimestamp: "2023-06-01T08:43:47Z"
generation: 1
generation: 2
managedFields:
- apiVersion: apps/v1
fieldsType: FieldsV1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: apps/v1 # for versions before 1.9.0 use apps/v1beta2
kind: Deployment
metadata:
name: "test"
generation: 1
spec:
progressDeadlineSeconds: 600
revisionHistoryLimit: 10
Expand Down
Loading