Skip to content

Commit cca933d

Browse files
agent-platform-auto-pr[bot]iamluc
agent-platform-auto-pr[bot]
and
iamluc
authored
[Backport 7.65.x] [clusteragent] Fix wrong computation of the init container resources (#36756)
Co-authored-by: Luc Vieillescazes <[email protected]>
1 parent d2f6a45 commit cca933d

File tree

3 files changed

+165
-21
lines changed

3 files changed

+165
-21
lines changed

‎pkg/clusteragent/admission/mutate/autoinstrumentation/auto_instrumentation.go

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ func podSumRessourceRequirements(pod *corev1.Pod) corev1.ResourceRequirements {
293293

294294
for _, k := range [2]corev1.ResourceName{corev1.ResourceMemory, corev1.ResourceCPU} {
295295
// Take max(initContainer ressource)
296+
maxInitContainerLimit := resource.Quantity{}
297+
maxInitContainerRequest := resource.Quantity{}
296298
for i := range pod.Spec.InitContainers {
297299
c := &pod.Spec.InitContainers[i]
298300
if initContainerIsSidecar(c) {
@@ -301,19 +303,18 @@ func podSumRessourceRequirements(pod *corev1.Pod) corev1.ResourceRequirements {
301303
continue
302304
}
303305
if limit, ok := c.Resources.Limits[k]; ok {
304-
existing := ressourceRequirement.Limits[k]
305-
if limit.Cmp(existing) == 1 {
306-
ressourceRequirement.Limits[k] = limit
306+
if limit.Cmp(maxInitContainerLimit) == 1 {
307+
maxInitContainerLimit = limit
307308
}
308309
}
309310
if request, ok := c.Resources.Requests[k]; ok {
310-
existing := ressourceRequirement.Requests[k]
311-
if request.Cmp(existing) == 1 {
312-
ressourceRequirement.Requests[k] = request
311+
if request.Cmp(maxInitContainerRequest) == 1 {
312+
maxInitContainerRequest = request
313313
}
314314
}
315315
}
316316

317+
// Take sum(container resources) + sum(sidecar containers)
317318
limitSum := resource.Quantity{}
318319
reqSum := resource.Quantity{}
319320
for i := range pod.Spec.Containers {
@@ -338,15 +339,24 @@ func podSumRessourceRequirements(pod *corev1.Pod) corev1.ResourceRequirements {
338339
}
339340
}
340341

341-
// Take max(sum(container resources) + sum(sidecar container resources))
342-
existingLimit := ressourceRequirement.Limits[k]
343-
if limitSum.Cmp(existingLimit) == 1 {
344-
ressourceRequirement.Limits[k] = limitSum
342+
// Take max(max(initContainer resources), sum(container resources) + sum(sidecar containers))
343+
if limitSum.Cmp(maxInitContainerLimit) == 1 {
344+
maxInitContainerLimit = limitSum
345+
}
346+
if reqSum.Cmp(maxInitContainerRequest) == 1 {
347+
maxInitContainerRequest = reqSum
348+
}
349+
350+
// Ensure that the limit is greater or equal to the request
351+
if maxInitContainerRequest.Cmp(maxInitContainerLimit) == 1 {
352+
maxInitContainerLimit = maxInitContainerRequest
345353
}
346354

347-
existingReq := ressourceRequirement.Requests[k]
348-
if reqSum.Cmp(existingReq) == 1 {
349-
ressourceRequirement.Requests[k] = reqSum
355+
if maxInitContainerLimit.CmpInt64(0) == 1 {
356+
ressourceRequirement.Limits[k] = maxInitContainerLimit
357+
}
358+
if maxInitContainerRequest.CmpInt64(0) == 1 {
359+
ressourceRequirement.Requests[k] = maxInitContainerRequest
350360
}
351361
}
352362

‎pkg/clusteragent/admission/mutate/autoinstrumentation/auto_instrumentation_test.go

Lines changed: 138 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,6 +1052,8 @@ func TestInjectLibInitContainer(t *testing.T) {
10521052
wantErr bool
10531053
wantCPU string
10541054
wantMem string
1055+
limitCPU string
1056+
limitMem string
10551057
secCtx *corev1.SecurityContext
10561058
}{
10571059
{
@@ -1469,6 +1471,124 @@ func TestInjectLibInitContainer(t *testing.T) {
14691471
wantCPU: "700m",
14701472
wantMem: "151Mi",
14711473
},
1474+
{
1475+
name: "init_container_request_greater_than_limit",
1476+
pod: &corev1.Pod{
1477+
ObjectMeta: metav1.ObjectMeta{
1478+
Name: "java-pod",
1479+
},
1480+
Spec: corev1.PodSpec{
1481+
InitContainers: []corev1.Container{{Name: "i1", Resources: corev1.ResourceRequirements{
1482+
Limits: corev1.ResourceList{}, // No limits
1483+
Requests: corev1.ResourceList{
1484+
corev1.ResourceCPU: resource.MustParse("200m"),
1485+
corev1.ResourceMemory: resource.MustParse("200Mi"),
1486+
},
1487+
}}},
1488+
Containers: []corev1.Container{{Name: "c1", Resources: corev1.ResourceRequirements{
1489+
Limits: corev1.ResourceList{
1490+
corev1.ResourceCPU: resource.MustParse("100m"),
1491+
corev1.ResourceMemory: resource.MustParse("100Mi"),
1492+
},
1493+
Requests: corev1.ResourceList{
1494+
corev1.ResourceCPU: resource.MustParse("100m"),
1495+
corev1.ResourceMemory: resource.MustParse("100Mi"),
1496+
},
1497+
}}},
1498+
},
1499+
},
1500+
image: "gcr.io/datadoghq/dd-lib-java-init:v1",
1501+
lang: java,
1502+
wantErr: false,
1503+
wantCPU: "200m",
1504+
wantMem: "200Mi",
1505+
limitCPU: "200m",
1506+
limitMem: "200Mi",
1507+
},
1508+
{
1509+
name: "containers_request_greater_than_limit",
1510+
pod: &corev1.Pod{
1511+
ObjectMeta: metav1.ObjectMeta{
1512+
Name: "java-pod",
1513+
},
1514+
Spec: corev1.PodSpec{
1515+
InitContainers: []corev1.Container{{Name: "i1", Resources: corev1.ResourceRequirements{
1516+
Limits: corev1.ResourceList{
1517+
corev1.ResourceCPU: resource.MustParse("100m"),
1518+
corev1.ResourceMemory: resource.MustParse("100Mi"),
1519+
},
1520+
Requests: corev1.ResourceList{
1521+
corev1.ResourceCPU: resource.MustParse("100m"),
1522+
corev1.ResourceMemory: resource.MustParse("100Mi"),
1523+
},
1524+
}}},
1525+
Containers: []corev1.Container{{Name: "c1", Resources: corev1.ResourceRequirements{
1526+
Limits: corev1.ResourceList{}, // No limits
1527+
Requests: corev1.ResourceList{
1528+
corev1.ResourceCPU: resource.MustParse("200m"),
1529+
corev1.ResourceMemory: resource.MustParse("200Mi"),
1530+
},
1531+
}}},
1532+
},
1533+
},
1534+
image: "gcr.io/datadoghq/dd-lib-java-init:v1",
1535+
lang: java,
1536+
wantErr: false,
1537+
wantCPU: "200m",
1538+
wantMem: "200Mi",
1539+
limitCPU: "200m",
1540+
limitMem: "200Mi",
1541+
},
1542+
{
1543+
name: "sidecar_container_request_greater_than_limit",
1544+
pod: &corev1.Pod{
1545+
ObjectMeta: metav1.ObjectMeta{
1546+
Name: "java-pod",
1547+
},
1548+
Spec: corev1.PodSpec{
1549+
InitContainers: []corev1.Container{
1550+
{
1551+
Name: "init-container-1",
1552+
Resources: corev1.ResourceRequirements{
1553+
Limits: corev1.ResourceList{
1554+
corev1.ResourceCPU: resource.MustParse("501m"),
1555+
corev1.ResourceMemory: resource.MustParse("101Mi"),
1556+
},
1557+
Requests: corev1.ResourceList{
1558+
corev1.ResourceCPU: resource.MustParse("501m"),
1559+
corev1.ResourceMemory: resource.MustParse("101Mi"),
1560+
},
1561+
},
1562+
}, {
1563+
Name: "sidecar-container-1",
1564+
RestartPolicy: pointer.Ptr(corev1.ContainerRestartPolicyAlways),
1565+
Resources: corev1.ResourceRequirements{
1566+
Limits: corev1.ResourceList{},
1567+
Requests: corev1.ResourceList{
1568+
corev1.ResourceCPU: resource.MustParse("500m"),
1569+
corev1.ResourceMemory: resource.MustParse("50Mi"),
1570+
},
1571+
},
1572+
},
1573+
},
1574+
Containers: []corev1.Container{{Name: "c1", Resources: corev1.ResourceRequirements{
1575+
Limits: corev1.ResourceList{
1576+
corev1.ResourceCPU: resource.MustParse("200m"),
1577+
corev1.ResourceMemory: resource.MustParse("101Mi"),
1578+
},
1579+
Requests: corev1.ResourceList{
1580+
corev1.ResourceCPU: resource.MustParse("200m"),
1581+
corev1.ResourceMemory: resource.MustParse("101Mi"),
1582+
},
1583+
}}},
1584+
},
1585+
},
1586+
image: "gcr.io/datadoghq/dd-lib-java-init:v1",
1587+
lang: java,
1588+
wantErr: false,
1589+
wantCPU: "700m",
1590+
wantMem: "151Mi",
1591+
},
14721592
{
14731593
name: "todo",
14741594
pod: &corev1.Pod{
@@ -1579,17 +1699,27 @@ func TestInjectLibInitContainer(t *testing.T) {
15791699

15801700
req := tt.pod.Spec.InitContainers[initalInitContainerCount].Resources.Requests[corev1.ResourceCPU]
15811701
lim := tt.pod.Spec.InitContainers[initalInitContainerCount].Resources.Limits[corev1.ResourceCPU]
1582-
wantCPUQuantity := resource.MustParse(tt.wantCPU)
1583-
t.Log("CPU wants:", wantCPUQuantity.String(), "actual lim: ", lim.String())
1584-
require.Zero(t, wantCPUQuantity.Cmp(req)) // Cmp returns 0 if equal
1585-
require.Zero(t, wantCPUQuantity.Cmp(lim))
1702+
requestCPUQuantity := resource.MustParse(tt.wantCPU)
1703+
limitCPUQuantity := requestCPUQuantity
1704+
if tt.limitCPU != "" {
1705+
limitCPUQuantity = resource.MustParse(tt.limitCPU)
1706+
}
1707+
1708+
t.Log("expected CPU request/limit:", requestCPUQuantity.String(), "/", limitCPUQuantity.String(), ", actual request/limit:", req.String(), "/", lim.String())
1709+
require.Zero(t, requestCPUQuantity.Cmp(req), "expected CPU request: %s, actual: %s", requestCPUQuantity.String(), req.String()) // Cmp returns 0 if equal
1710+
require.Zero(t, limitCPUQuantity.Cmp(lim), "expected CPU limit: %s, actual: %s", limitCPUQuantity.String(), lim.String())
15861711

15871712
req = tt.pod.Spec.InitContainers[initalInitContainerCount].Resources.Requests[corev1.ResourceMemory]
15881713
lim = tt.pod.Spec.InitContainers[initalInitContainerCount].Resources.Limits[corev1.ResourceMemory]
1589-
wantMemQuantity := resource.MustParse(tt.wantMem)
1590-
t.Log("memeory wants:", wantMemQuantity.String(), "actual lim: ", lim.String())
1591-
require.Zero(t, wantMemQuantity.Cmp(req))
1592-
require.Zero(t, wantMemQuantity.Cmp(lim))
1714+
requestMemQuantity := resource.MustParse(tt.wantMem)
1715+
limitMemQuantity := requestMemQuantity
1716+
if tt.limitMem != "" {
1717+
limitMemQuantity = resource.MustParse(tt.limitMem)
1718+
}
1719+
1720+
t.Log("expected memory request/limit:", requestMemQuantity.String(), "/", limitMemQuantity.String(), ", actual request/limit:", req.String(), "/", lim.String())
1721+
require.Zero(t, requestMemQuantity.Cmp(req), "expected memory request: %s, actual: %s", requestMemQuantity.String(), req.String())
1722+
require.Zero(t, limitMemQuantity.Cmp(lim), "expected memory limit: %s, actual: %s", limitMemQuantity.String(), lim.String())
15931723

15941724
expSecCtx := tt.pod.Spec.InitContainers[0].SecurityContext
15951725
require.Equal(t, tt.secCtx, expSecCtx)

‎releasenotes-dca/notes/autoinstrumentation-fix-resources-computation-9f00f3aa904f5b5d.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
Fix wrong computation of the init container resources in the autoinstrumentation webhook.

0 commit comments

Comments
 (0)