Skip to content

Commit e61739c

Browse files
afalhambra-hivemqafalhambra-hivemq
committed
fix: Fix infinite resource updates due empty EnvVars
Signed-off-by: Antonio Fernandez Alhambra <antonio.alhambra@hivemq.com>
1 parent 9057d5f commit e61739c

6 files changed

Lines changed: 621 additions & 328 deletions

File tree

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

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import java.util.List;
4+
import java.util.Map;
5+
import java.util.Objects;
6+
import java.util.Optional;
7+
8+
import io.fabric8.kubernetes.api.model.Container;
9+
import io.fabric8.kubernetes.api.model.EnvVar;
10+
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
11+
import io.fabric8.kubernetes.api.model.PodTemplateSpec;
12+
import io.fabric8.kubernetes.api.model.Quantity;
13+
import io.fabric8.kubernetes.api.model.ResourceRequirements;
14+
15+
/**
16+
* Sanitizes the {@link ResourceRequirements} and the {@link EnvVar} in the containers of a pair of
17+
* {@link PodTemplateSpec} instances.
18+
*
19+
* <p>When the sanitizer finds a mismatch in the structure of the given templates, before it gets to
20+
* the nested fields, it returns early without fixing the actual map. This is an optimization
21+
* because the given templates will anyway differ at this point. This means we do not have to
22+
* attempt to sanitize the fields for these use cases, since there will anyway be an update of the
23+
* K8s resource.
24+
*
25+
* <p>The algorithm traverses the whole template structure because we need the actual and desired
26+
* {@link Quantity} and {@link EnvVar} instances. Using the {@link
27+
* GenericKubernetesResource#get(Map, Object...)} shortcut would need to create new instances just
28+
* for the sanitization check.
29+
*/
30+
class PodTemplateSpecSanitizer {
31+
32+
static void sanitizePodTemplateSpec(
33+
final Map<String, Object> actualMap,
34+
final PodTemplateSpec actualTemplate,
35+
final PodTemplateSpec desiredTemplate) {
36+
if (actualTemplate == null || desiredTemplate == null) {
37+
return;
38+
}
39+
if (actualTemplate.getSpec() == null || desiredTemplate.getSpec() == null) {
40+
return;
41+
}
42+
sanitizePodTemplateSpec(
43+
actualMap,
44+
actualTemplate.getSpec().getInitContainers(),
45+
desiredTemplate.getSpec().getInitContainers(),
46+
"initContainers");
47+
sanitizePodTemplateSpec(
48+
actualMap,
49+
actualTemplate.getSpec().getContainers(),
50+
desiredTemplate.getSpec().getContainers(),
51+
"containers");
52+
}
53+
54+
private static void sanitizePodTemplateSpec(
55+
final Map<String, Object> actualMap,
56+
final List<Container> actualContainers,
57+
final List<Container> desiredContainers,
58+
final String containerPath) {
59+
int containers = desiredContainers.size();
60+
if (containers == actualContainers.size()) {
61+
for (int containerIndex = 0; containerIndex < containers; containerIndex++) {
62+
final var desiredContainer = desiredContainers.get(containerIndex);
63+
final var actualContainer = actualContainers.get(containerIndex);
64+
if (!desiredContainer.getName().equals(actualContainer.getName())) {
65+
return;
66+
}
67+
sanitizeEnvVars(
68+
actualMap,
69+
actualContainer.getEnv(),
70+
desiredContainer.getEnv(),
71+
containerPath,
72+
containerIndex);
73+
sanitizeResourceRequirements(
74+
actualMap,
75+
actualContainer.getResources(),
76+
desiredContainer.getResources(),
77+
containerPath,
78+
containerIndex);
79+
}
80+
}
81+
}
82+
83+
private static void sanitizeResourceRequirements(
84+
final Map<String, Object> actualMap,
85+
final ResourceRequirements actualResource,
86+
final ResourceRequirements desiredResource,
87+
final String containerPath,
88+
final int containerIndex) {
89+
if (desiredResource == null || actualResource == null) {
90+
return;
91+
}
92+
sanitizeQuantities(
93+
actualMap,
94+
actualResource.getRequests(),
95+
desiredResource.getRequests(),
96+
containerPath,
97+
containerIndex,
98+
"requests");
99+
sanitizeQuantities(
100+
actualMap,
101+
actualResource.getLimits(),
102+
desiredResource.getLimits(),
103+
containerPath,
104+
containerIndex,
105+
"limits");
106+
}
107+
108+
@SuppressWarnings("unchecked")
109+
private static void sanitizeQuantities(
110+
final Map<String, Object> actualMap,
111+
final Map<String, Quantity> actualResource,
112+
final Map<String, Quantity> desiredResource,
113+
final String containerPath,
114+
final int containerIndex,
115+
final String quantityPath) {
116+
Optional.ofNullable(
117+
GenericKubernetesResource.get(
118+
actualMap,
119+
"spec",
120+
"template",
121+
"spec",
122+
containerPath,
123+
containerIndex,
124+
"resources",
125+
quantityPath))
126+
.map(Map.class::cast)
127+
.filter(m -> m.size() == desiredResource.size())
128+
.ifPresent(
129+
m ->
130+
actualResource.forEach(
131+
(key, actualQuantity) -> {
132+
final var desiredQuantity = desiredResource.get(key);
133+
if (desiredQuantity == null) {
134+
return;
135+
}
136+
// check if the string representation of the Quantity instances is equal
137+
if (actualQuantity.getAmount().equals(desiredQuantity.getAmount())
138+
&& actualQuantity.getFormat().equals(desiredQuantity.getFormat())) {
139+
return;
140+
}
141+
// check if the numerical amount of the Quantity instances is equal
142+
if (actualQuantity.equals(desiredQuantity)) {
143+
// replace the actual Quantity with the desired Quantity to prevent a
144+
// resource update
145+
m.replace(key, desiredQuantity.toString());
146+
}
147+
}));
148+
}
149+
150+
@SuppressWarnings("unchecked")
151+
private static void sanitizeEnvVars(
152+
final Map<String, Object> actualMap,
153+
final List<EnvVar> actualEnvVars,
154+
final List<EnvVar> desiredEnvVars,
155+
final String containerPath,
156+
final int containerIndex) {
157+
if (desiredEnvVars.isEmpty() || actualEnvVars.isEmpty()) {
158+
return;
159+
}
160+
Optional.ofNullable(
161+
GenericKubernetesResource.get(
162+
actualMap, "spec", "template", "spec", containerPath, containerIndex, "env"))
163+
.map(List.class::cast)
164+
.ifPresent(
165+
envVars ->
166+
actualEnvVars.forEach(
167+
actualEnvVar -> {
168+
final var actualEnvVarName = actualEnvVar.getName();
169+
final var actualEnvVarValue = actualEnvVar.getValue();
170+
// check if the actual EnvVar value string is not null or the desired EnvVar
171+
// already contains the same EnvVar name with a non empty EnvVar value
172+
final var isDesiredEnvVarEmpty =
173+
hasEnvVarNoEmptyValue(actualEnvVarName, desiredEnvVars);
174+
if (actualEnvVarValue != null || isDesiredEnvVarEmpty) {
175+
return;
176+
}
177+
envVars.stream()
178+
.filter(
179+
envVar ->
180+
((Map<String, String>) envVar)
181+
.get("name")
182+
.equals(actualEnvVarName))
183+
// add the actual EnvVar value with an empty string to prevent a
184+
// resource update
185+
.forEach(envVar -> ((Map<String, String>) envVar).put("value", ""));
186+
}));
187+
}
188+
189+
private static boolean hasEnvVarNoEmptyValue(
190+
final String envVarName, final List<EnvVar> envVars) {
191+
return envVars.stream()
192+
.anyMatch(
193+
envVar ->
194+
Objects.equals(envVarName, envVar.getName())
195+
&& envVar.getValue() != null
196+
&& !envVar.getValue().isEmpty());
197+
}
198+
}

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

Lines changed: 0 additions & 100 deletions
This file was deleted.

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import com.github.difflib.DiffUtils;
3232
import com.github.difflib.UnifiedDiffUtils;
3333

34-
import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceRequirementsSanitizer.sanitizeResourceRequirements;
34+
import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.PodTemplateSpecSanitizer.sanitizePodTemplateSpec;
3535

3636
/**
3737
* Matches the actual state on the server vs the desired state. Based on the managedFields of SSA.
@@ -190,20 +190,23 @@ private void sanitizeState(R actual, R desired, Map<String, Object> actualMap) {
190190
}
191191
}
192192
}
193-
sanitizeResourceRequirements(actualMap, actualSpec.getTemplate(), desiredSpec.getTemplate());
193+
sanitizePodTemplateSpec(actualMap, actualSpec.getTemplate(), desiredSpec.getTemplate());
194194
} else if (actual instanceof Deployment actualDeployment
195195
&& desired instanceof Deployment desiredDeployment) {
196-
sanitizeResourceRequirements(actualMap,
196+
sanitizePodTemplateSpec(
197+
actualMap,
197198
actualDeployment.getSpec().getTemplate(),
198199
desiredDeployment.getSpec().getTemplate());
199200
} else if (actual instanceof ReplicaSet actualReplicaSet
200201
&& desired instanceof ReplicaSet desiredReplicaSet) {
201-
sanitizeResourceRequirements(actualMap,
202+
sanitizePodTemplateSpec(
203+
actualMap,
202204
actualReplicaSet.getSpec().getTemplate(),
203205
desiredReplicaSet.getSpec().getTemplate());
204206
} else if (actual instanceof DaemonSet actualDaemonSet
205207
&& desired instanceof DaemonSet desiredDaemonSet) {
206-
sanitizeResourceRequirements(actualMap,
208+
sanitizePodTemplateSpec(
209+
actualMap,
207210
actualDaemonSet.getSpec().getTemplate(),
208211
desiredDaemonSet.getSpec().getTemplate());
209212
}

0 commit comments

Comments
 (0)