Skip to content

Commit 5386ed3

Browse files
csviricsviri
committed
wip
1 parent ee06def commit 5386ed3

File tree

4 files changed

+76
-20
lines changed

4 files changed

+76
-20
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public interface ControllerConfiguration<P extends HasMetadata> extends Resource
2323
@SuppressWarnings("rawtypes")
2424
RateLimiter DEFAULT_RATE_LIMITER = LinearRateLimiter.deactivatedRateLimiter();
2525
String DEFAULT_FIELD_MANAGER = "controller";
26+
String FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER = "fabric8-kubernetes-client";
2627

2728
default String getName() {
2829
return ensureValidName(null, getAssociatedReconcilerClassName());

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

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616

1717
// https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#fieldsv1-v1-meta
1818
// https://github.com/kubernetes-sigs/structured-merge-diff
19-
// todo add migration integration test
19+
// https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-field-management.html
2020
public class SSABasedGenericKubernetesResourceMatcher<R extends HasMetadata> {
2121

2222
@SuppressWarnings("rawtypes")
2323
private static final SSABasedGenericKubernetesResourceMatcher INSTANCE =
2424
new SSABasedGenericKubernetesResourceMatcher<>();
25+
public static final String APPLY_OPERATION = "Apply";
26+
public static final String DOT_KEY = ".";
2527

2628
@SuppressWarnings("unchecked")
2729
public static <L extends HasMetadata> SSABasedGenericKubernetesResourceMatcher<L> getInstance() {
@@ -45,6 +47,7 @@ public boolean matches(R actual, R desired, Context<?> context) {
4547
checkIfFieldManagerExists(actual,
4648
context.getControllerConfiguration().fieldManager());
4749
// the update will add the field manager
50+
// todo check if it's an apply, since if not it could be a different field manager?
4851
if (optionalManagedFieldsEntry.isEmpty()) {
4952
return false;
5053
}
@@ -57,6 +60,8 @@ public boolean matches(R actual, R desired, Context<?> context) {
5760
var actualMap = objectMapper.convertValue(actual, typeRef);
5861
var desiredMap = objectMapper.convertValue(desired, typeRef);
5962

63+
log.trace("Original actual: \n {} \n original desired: \n {} ", actual, desiredMap);
64+
6065
var prunedActual = new HashMap<String, Object>();
6166
pruneActualAccordingManagedFields(prunedActual, actualMap,
6267
managedFieldsEntry.getFieldsV1().getAdditionalProperties(), objectMapper);
@@ -102,12 +107,15 @@ private void pruneActualAccordingManagedFields(Map<String, Object> result,
102107
String targetKey = key.substring(2);
103108
var managedFieldValue = entry.getValue();
104109
if (managedFieldValue instanceof Map && (!((Map) managedFieldValue).isEmpty())) {
105-
var managedListEntrySet = ((Map<String, Object>) managedFieldValue).entrySet();
106-
var firstManagedFieldEntry = managedListEntrySet.iterator().next();
107-
if (firstManagedFieldEntry.getKey().startsWith(K_PREFIX)) {
110+
var managedEntrySet = ((Map<String, Object>) managedFieldValue).entrySet();
111+
112+
if (isListEntrySet(managedEntrySet)) {
108113
var valueList = new ArrayList<>();
109114
result.put(targetKey, valueList);
110-
for (Map.Entry<String, Object> listEntry : managedListEntrySet) {
115+
for (Map.Entry<String, Object> listEntry : managedEntrySet) {
116+
if (DOT_KEY.equals(listEntry.getKey())) {
117+
continue;
118+
}
111119
var emptyResMapValue = new HashMap<String, Object>();
112120
valueList.add(emptyResMapValue);
113121
var actualListEntry = selectListEntryBasedOnKey(listEntry.getKey().substring(2),
@@ -137,6 +145,16 @@ private void pruneActualAccordingManagedFields(Map<String, Object> result,
137145

138146
}
139147

148+
private boolean isListEntrySet(Set<Map.Entry<String, Object>> managedEntrySet) {
149+
var iterator = managedEntrySet.iterator();
150+
var managedFieldEntry = iterator.next();
151+
// todo unit test
152+
if (managedFieldEntry.getKey().equals(DOT_KEY)) {
153+
managedFieldEntry = iterator.next();
154+
}
155+
return managedFieldEntry.getKey().startsWith(K_PREFIX);
156+
}
157+
140158
private Map<String, Object> selectListEntryBasedOnKey(String key,
141159
List<Map<String, Object>> values,
142160
ObjectMapper objectMapper) {
@@ -163,13 +181,18 @@ private Map<String, Object> selectListEntryBasedOnKey(String key,
163181

164182
private Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String fieldManager) {
165183
var targetManagedFields = actual.getMetadata().getManagedFields().stream()
166-
.filter(f -> f.getManager().equals(fieldManager))
184+
// Only the apply operations are interesting for us since those were created properly be SSA
185+
// Patch. An update can be present with same fieldManager when migrating and having the same
186+
// field manager name.
187+
.filter(
188+
f -> f.getManager().equals(fieldManager) && f.getOperation().equals(APPLY_OPERATION))
167189
.collect(Collectors.toList());
168190
if (targetManagedFields.isEmpty()) {
169-
log.debug("No field manager exists for resource {} with name: {} ",
191+
log.debug("No field manager exists for resource {} with name: {} and operation Apply ",
170192
actual, actual.getMetadata().getName());
171193
return Optional.empty();
172194
}
195+
// this should not happen in theory
173196
if (targetManagedFields.size() > 1) {
174197
log.debug("More field managers exists with name: {} in resource: {} with name: {} ",
175198
fieldManager,

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ void checksIfAddsNotAddedByController() {
4040
assertThat(matcher.matches(actual, desired, mockedContext)).isTrue();
4141
}
4242

43+
// todo test lists with more entries
44+
4345
private <R> R loadResource(String fileName, Class<R> clazz) {
4446
return ReconcilerUtils.loadYaml(clazz, SSABasedGenericKubernetesResourceMatcherTest.class,
4547
fileName);

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import org.junit.jupiter.api.Test;
66
import org.junit.jupiter.api.TestInfo;
77

8+
import io.fabric8.kubernetes.api.model.ConfigMap;
89
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
910
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
1011
import io.fabric8.kubernetes.client.KubernetesClient;
@@ -16,6 +17,7 @@
1617
import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource;
1718
import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent;
1819

20+
import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER;
1921
import static org.assertj.core.api.Assertions.assertThat;
2022
import static org.awaitility.Awaitility.await;
2123

@@ -30,9 +32,11 @@ class DependentSSAMigrationIT {
3032

3133
@BeforeEach
3234
void setup(TestInfo testInfo) {
35+
SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0);
3336
LocallyRunOperatorExtension.applyCrd(DependnetSSACustomResource.class, client);
3437
testInfo.getTestMethod().ifPresent(method -> {
3538
namespace = KubernetesResourceUtil.sanitizeName(method.getName());
39+
cleanup();
3640
client.namespaces().resource(new NamespaceBuilder().withMetadata(new ObjectMetaBuilder()
3741
.withName(namespace)
3842
.build()).build()).create();
@@ -48,16 +52,33 @@ void cleanup() {
4852

4953
@Test
5054
void migratesFromLegacyToWorksAndBack() {
51-
SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0);
52-
var legacyOperator = createOperator(client, true);
53-
DependnetSSACustomResource testResource = reconcileWithLegacyOpetor(legacyOperator);
55+
var legacyOperator = createOperator(client, true, null);
56+
DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
5457

55-
var operator = createOperator(client, false);
58+
var operator = createOperator(client, false, null);
5659
testResource = reconcileWithNewApproach(testResource, operator);
60+
var cm = getDependentConfigMap();
61+
assertThat(cm.getMetadata().getManagedFields()).hasSize(2);
5762

5863
reconcileAgainWithLegacy(legacyOperator, testResource);
5964
}
6065

66+
@Test
67+
void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() {
68+
var legacyOperator = createOperator(client, true, null);
69+
DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator);
70+
71+
var operator = createOperator(client, false,
72+
FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER);
73+
reconcileWithNewApproach(testResource, operator);
74+
75+
var cm = getDependentConfigMap();
76+
77+
assertThat(cm.getMetadata().getManagedFields()).hasSize(2);
78+
assertThat(cm.getMetadata().getManagedFields())
79+
.allMatch(fm -> fm.getManager().equals(FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER));
80+
}
81+
6182
private void reconcileAgainWithLegacy(Operator legacyOperator,
6283
DependnetSSACustomResource testResource) {
6384
legacyOperator.start();
@@ -67,7 +88,7 @@ private void reconcileAgainWithLegacy(Operator legacyOperator,
6788
client.resource(testResource).update();
6889

6990
await().untilAsserted(() -> {
70-
var cm = client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get();
91+
var cm = getDependentConfigMap();
7192
assertThat(cm.getData()).containsEntry(SSAConfigMapDependent.DATA_KEY, INITIAL_VALUE);
7293
});
7394

@@ -79,10 +100,8 @@ private DependnetSSACustomResource reconcileWithNewApproach(
79100
operator.start();
80101

81102
await().untilAsserted(() -> {
82-
var cm = client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get();
103+
var cm = getDependentConfigMap();
83104
assertThat(cm).isNotNull();
84-
// todo there will be a second manager? both managing
85-
assertThat(cm.getMetadata().getManagedFields()).hasSize(2);
86105
assertThat(cm.getData()).hasSize(1);
87106
});
88107

@@ -91,20 +110,24 @@ private DependnetSSACustomResource reconcileWithNewApproach(
91110
testResource = client.resource(testResource).update();
92111

93112
await().untilAsserted(() -> {
94-
var cm = client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get();
113+
var cm = getDependentConfigMap();
95114
assertThat(cm.getData()).containsEntry(SSAConfigMapDependent.DATA_KEY, CHANGED_VALUE);
96115
});
97116
operator.stop();
98117
return testResource;
99118
}
100119

101-
private DependnetSSACustomResource reconcileWithLegacyOpetor(Operator legacyOperator) {
120+
private ConfigMap getDependentConfigMap() {
121+
return client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get();
122+
}
123+
124+
private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) {
102125
legacyOperator.start();
103126

104127
var testResource = client.resource(testResource()).create();
105128

106129
await().untilAsserted(() -> {
107-
var cm = client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get();
130+
var cm = getDependentConfigMap();
108131
assertThat(cm).isNotNull();
109132
assertThat(cm.getMetadata().getManagedFields()).hasSize(1);
110133
assertThat(cm.getData()).hasSize(1);
@@ -114,11 +137,18 @@ private DependnetSSACustomResource reconcileWithLegacyOpetor(Operator legacyOper
114137
return testResource;
115138
}
116139

117-
private Operator createOperator(KubernetesClient client, boolean legacyDependentHandling) {
140+
141+
private Operator createOperator(KubernetesClient client, boolean legacyDependentHandling,
142+
String fieldManager) {
118143
Operator operator = new Operator(client,
119144
o -> o.withLegacyCreateUpdateAndMatchingForDependentResources(legacyDependentHandling)
120145
.withCloseClientOnStop(false));
121-
operator.register(new DependentSSAReconciler(), o -> o.settingNamespace(namespace));
146+
operator.register(new DependentSSAReconciler(), o -> {
147+
o.settingNamespace(namespace);
148+
if (fieldManager != null) {
149+
o.withFieldManager(fieldManager);
150+
}
151+
});
122152
return operator;
123153
}
124154

0 commit comments

Comments
 (0)