Skip to content

Commit 71e00ed

Browse files
authored
fix: infinite resource updates due to canonical format conversion of resource requirements (#2568)
* refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> * refactor: fix import order Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]>
1 parent b4c6e0d commit 71e00ed

22 files changed

+1161
-77
lines changed

operator-framework-core/pom.xml

+5
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@
101101
<artifactId>junit-jupiter-engine</artifactId>
102102
<scope>test</scope>
103103
</dependency>
104+
<dependency>
105+
<groupId>org.junit.jupiter</groupId>
106+
<artifactId>junit-jupiter-params</artifactId>
107+
<scope>test</scope>
108+
</dependency>
104109
<dependency>
105110
<groupId>org.mockito</groupId>
106111
<artifactId>mockito-core</artifactId>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import java.util.List;
4+
import java.util.Map;
5+
import java.util.Optional;
6+
7+
import io.fabric8.kubernetes.api.model.Container;
8+
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
9+
import io.fabric8.kubernetes.api.model.PodTemplateSpec;
10+
import io.fabric8.kubernetes.api.model.Quantity;
11+
import io.fabric8.kubernetes.api.model.ResourceRequirements;
12+
13+
/**
14+
* Sanitizes the {@link ResourceRequirements} in the containers of a pair of {@link PodTemplateSpec}
15+
* instances.
16+
* <p>
17+
* When the sanitizer finds a mismatch in the structure of the given templates, before it gets to
18+
* the nested resource limits and requests, it returns early without fixing the actual map. This is
19+
* an optimization because the given templates will anyway differ at this point. This means we do
20+
* not have to attempt to sanitize the resources for these use cases, since there will anyway be an
21+
* update of the K8s resource.
22+
* <p>
23+
* The algorithm traverses the whole template structure because we need the actual and desired
24+
* {@link Quantity} instances to compare their numerical amount. Using the
25+
* {@link GenericKubernetesResource#get(Map, Object...)} shortcut would need to create new instances
26+
* just for the sanitization check.
27+
*/
28+
class ResourceRequirementsSanitizer {
29+
30+
static void sanitizeResourceRequirements(final Map<String, Object> actualMap,
31+
final PodTemplateSpec actualTemplate, final PodTemplateSpec desiredTemplate) {
32+
if (actualTemplate == null || desiredTemplate == null) {
33+
return;
34+
}
35+
if (actualTemplate.getSpec() == null || desiredTemplate.getSpec() == null) {
36+
return;
37+
}
38+
sanitizeResourceRequirements(actualMap, actualTemplate.getSpec().getInitContainers(),
39+
desiredTemplate.getSpec().getInitContainers(), "initContainers");
40+
sanitizeResourceRequirements(actualMap, actualTemplate.getSpec().getContainers(),
41+
desiredTemplate.getSpec().getContainers(), "containers");
42+
}
43+
44+
private static void sanitizeResourceRequirements(final Map<String, Object> actualMap,
45+
final List<Container> actualContainers, final List<Container> desiredContainers,
46+
final String containerPath) {
47+
int containers = desiredContainers.size();
48+
if (containers == actualContainers.size()) {
49+
for (int containerIndex = 0; containerIndex < containers; containerIndex++) {
50+
var desiredContainer = desiredContainers.get(containerIndex);
51+
var actualContainer = actualContainers.get(containerIndex);
52+
if (!desiredContainer.getName().equals(actualContainer.getName())) {
53+
return;
54+
}
55+
sanitizeResourceRequirements(actualMap, actualContainer.getResources(),
56+
desiredContainer.getResources(),
57+
containerPath, containerIndex);
58+
}
59+
}
60+
}
61+
62+
private static void sanitizeResourceRequirements(final Map<String, Object> actualMap,
63+
final ResourceRequirements actualResource, final ResourceRequirements desiredResource,
64+
final String containerPath, final int containerIndex) {
65+
if (desiredResource == null || actualResource == null) {
66+
return;
67+
}
68+
sanitizeQuantities(actualMap, actualResource.getRequests(), desiredResource.getRequests(),
69+
containerPath, containerIndex, "requests");
70+
sanitizeQuantities(actualMap, actualResource.getLimits(), desiredResource.getLimits(),
71+
containerPath, containerIndex, "limits");
72+
}
73+
74+
@SuppressWarnings("unchecked")
75+
private static void sanitizeQuantities(final Map<String, Object> actualMap,
76+
final Map<String, Quantity> actualResource, final Map<String, Quantity> desiredResource,
77+
final String containerPath, final int containerIndex, final String quantityPath) {
78+
Optional.ofNullable(
79+
GenericKubernetesResource.get(actualMap, "spec", "template", "spec", containerPath,
80+
containerIndex, "resources", quantityPath))
81+
.map(Map.class::cast)
82+
.filter(m -> m.size() == desiredResource.size())
83+
.ifPresent(m -> actualResource.forEach((key, actualQuantity) -> {
84+
var desiredQuantity = desiredResource.get(key);
85+
if (desiredQuantity == null) {
86+
return;
87+
}
88+
// check if the string representation of the Quantity instances is equal
89+
if (actualQuantity.getAmount().equals(desiredQuantity.getAmount())
90+
&& actualQuantity.getFormat().equals(desiredQuantity.getFormat())) {
91+
return;
92+
}
93+
// check if the numerical amount of the Quantity instances is equal
94+
if (actualQuantity.equals(desiredQuantity)) {
95+
// replace the actual Quantity with the desired Quantity to prevent a resource update
96+
m.replace(key, desiredQuantity.toString());
97+
}
98+
}));
99+
}
100+
}

0 commit comments

Comments
 (0)