Skip to content

Commit 93ac201

Browse files
fix: annotate pods with resolved OvercommitClass (#48)
Use the actual OvercommitClass resolved from pod, namespace, or default selection when setting the applied annotation, events, and mutation metrics. This fixes namespace-labeled pods intercepted by the default webhook being annotated with the default class instead of the class used for request calculation.
1 parent 7446ee6 commit 93ac201

8 files changed

Lines changed: 164 additions & 46 deletions

File tree

internal/utils/getOvercommitClass.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ func GetOvercommitClassSpec(ctx context.Context, name string, k8sClient client.C
4646
}
4747

4848
func GetDefaultSpec(ctx context.Context, k8sClient client.Client) (*overcommit.OvercommitClassSpec, error) {
49+
defaultClass, err := GetDefaultClass(ctx, k8sClient)
50+
if err != nil {
51+
return nil, err
52+
}
53+
54+
return &defaultClass.Spec, nil
55+
}
56+
57+
func GetDefaultClass(ctx context.Context, k8sClient client.Client) (*overcommit.OvercommitClass, error) {
4958
if k8sClient == nil {
5059
return nil, errors.New("client parameter cannot be nil")
5160
}
@@ -57,10 +66,10 @@ func GetDefaultSpec(ctx context.Context, k8sClient client.Client) (*overcommit.O
5766
}
5867

5968
// Find the default OvercommitClass
60-
for _, overcommitClass := range overcommitClasses.Items {
61-
if overcommitClass.Spec.IsDefault {
62-
podlog.Info("Default OvercommitClass found", "name", overcommitClass.Name)
63-
return &overcommitClass.Spec, nil
69+
for i := range overcommitClasses.Items {
70+
if overcommitClasses.Items[i].Spec.IsDefault {
71+
podlog.Info("Default OvercommitClass found", "name", overcommitClasses.Items[i].Name)
72+
return &overcommitClasses.Items[i], nil
6473
}
6574
}
6675

pkg/overcommit/calculate_values_from_labels.go

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,30 @@ package overcommit
99
import (
1010
"context"
1111

12-
"github.com/InditexTech/k8s-overcommit-operator/internal/metrics"
1312
"github.com/InditexTech/k8s-overcommit-operator/internal/utils"
1413
corev1 "k8s.io/api/core/v1"
1514
"sigs.k8s.io/controller-runtime/pkg/client"
1615
)
1716

17+
type overcommitResolution struct {
18+
className string
19+
cpuValue float64
20+
memoryValue float64
21+
ownerName string
22+
ownerKind string
23+
resolved bool
24+
}
25+
1826
// getNamespaceOvercommit gets the overcommit values from the namespace label or falls back to the default class.
19-
// Returns (1.0, 1.0) as safe no-op values when any error occurs to avoid mutating pods incorrectly.
20-
func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, k8sClient client.Client, label, ownerName, ownerKind string) (float64, float64) {
27+
// Returns safe no-op values when any error occurs to avoid mutating pods incorrectly.
28+
func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, k8sClient client.Client, label, ownerName, ownerKind string) overcommitResolution {
2129
// Get the namespace of the pod
2230
namespaceName := pod.Namespace
2331
var ns corev1.Namespace
2432
err := k8sClient.Get(ctx, client.ObjectKey{Name: namespaceName}, &ns)
2533
if err != nil {
2634
podlog.Error(err, "Error getting the namespace", "namespace", namespaceName)
27-
return 1.0, 1.0
35+
return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind}
2836
}
2937

3038
// Check if the overcommit class label is in the namespace
@@ -33,23 +41,35 @@ func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, k8sClient clie
3341
overcommitClass, err := utils.GetOvercommitClassSpec(ctx, val, k8sClient)
3442
if err != nil {
3543
podlog.Error(err, "Error getting the overcommit class", "overcommitClassLabel", val)
36-
return 1.0, 1.0
44+
return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind}
45+
}
46+
return overcommitResolution{
47+
className: val,
48+
cpuValue: overcommitClass.CpuOvercommit,
49+
memoryValue: overcommitClass.MemoryOvercommit,
50+
ownerName: ownerName,
51+
ownerKind: ownerKind,
52+
resolved: true,
3753
}
38-
metrics.K8sOvercommitPodMutated.WithLabelValues(val, ownerKind, ownerName, pod.Namespace).Inc()
39-
return overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit
4054
}
4155

4256
podlog.Info("Overcommit class not found in the namespace, using the default", "namespace", ns.Name)
43-
defaultClass, err := utils.GetDefaultSpec(ctx, k8sClient)
57+
defaultClass, err := utils.GetDefaultClass(ctx, k8sClient)
4458
if err != nil {
4559
podlog.Error(err, "Error getting the default overcommit class")
46-
return 1.0, 1.0
60+
return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind}
61+
}
62+
return overcommitResolution{
63+
className: defaultClass.Name,
64+
cpuValue: defaultClass.Spec.CpuOvercommit,
65+
memoryValue: defaultClass.Spec.MemoryOvercommit,
66+
ownerName: ownerName,
67+
ownerKind: ownerKind,
68+
resolved: true,
4769
}
48-
metrics.K8sOvercommitPodMutated.WithLabelValues("default", ownerKind, ownerName, pod.Namespace).Inc()
49-
return defaultClass.CpuOvercommit, defaultClass.MemoryOvercommit
5070
}
5171

52-
func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Client) (float64, float64) {
72+
func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Client) overcommitResolution {
5373
ownerName, ownerKind, err := utils.GetPodOwner(ctx, client, &pod)
5474
if err != nil {
5575
podlog.Error(err, "Error getting the pod owner")
@@ -59,7 +79,7 @@ func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Clie
5979
label, err := utils.GetOvercommitLabel(ctx, client)
6080
if err != nil {
6181
podlog.Error(err, "Error getting the overcommit label")
62-
return 1.0, 1.0
82+
return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind}
6383
}
6484
// Check if the pod has the overcommit class label
6585
value, exists := pod.Labels[label]
@@ -76,8 +96,14 @@ func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Clie
7696
// Overcommit class not found or some error, fall back to namespace/default
7797
return getNamespaceOvercommit(ctx, &pod, client, label, ownerName, ownerKind)
7898
}
79-
metrics.K8sOvercommitPodMutated.WithLabelValues(value, ownerKind, ownerName, pod.Namespace).Inc()
80-
return overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit
99+
return overcommitResolution{
100+
className: value,
101+
cpuValue: overcommitClass.CpuOvercommit,
102+
memoryValue: overcommitClass.MemoryOvercommit,
103+
ownerName: ownerName,
104+
ownerKind: ownerKind,
105+
resolved: true,
106+
}
81107
}
82108

83109
// Overcommit class not found, checking the namespace

pkg/overcommit/calculate_values_from_labels_test.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,29 @@ var _ = Describe("Overcommit Functions", func() {
1616

1717
Describe("getNamespaceOvercommit", func() {
1818
It("should return the correct overcommit values from the namespace", func() {
19-
cpuOvercommit, memoryOvercommit := getNamespaceOvercommit(context.TODO(), testPod, k8sClient, "inditex.com/overcommit-class", "ownerName", "ownerKind")
20-
Expect(cpuOvercommit).To(Equal(0.5))
21-
Expect(memoryOvercommit).To(Equal(0.5))
19+
resolution := getNamespaceOvercommit(context.TODO(), testPod, k8sClient, "inditex.com/overcommit-class", "ownerName", "ownerKind")
20+
Expect(resolution.className).To(Equal("test-class"))
21+
Expect(resolution.cpuValue).To(Equal(0.5))
22+
Expect(resolution.memoryValue).To(Equal(0.5))
2223
})
2324
})
2425

2526
Describe("checkOvercommitType", func() {
2627
It("should return the correct overcommit values from the pod", func() {
27-
cpuOvercommit, memoryOvercommit := checkOvercommitType(context.TODO(), *testPod, k8sClient)
28-
Expect(cpuOvercommit).To(Equal(0.5))
29-
Expect(memoryOvercommit).To(Equal(0.5))
28+
resolution := checkOvercommitType(context.TODO(), *testPod, k8sClient)
29+
Expect(resolution.className).To(Equal("test-class"))
30+
Expect(resolution.cpuValue).To(Equal(0.5))
31+
Expect(resolution.memoryValue).To(Equal(0.5))
3032
})
3133

3234
It("should fallback to namespace overcommit values if pod label is missing", func() {
33-
delete(testPod.Labels, "LABEL_OVERCOMMIT_CLASS")
34-
err := k8sClient.Update(context.TODO(), testPod)
35-
Expect(err).NotTo(HaveOccurred())
35+
pod := testPod.DeepCopy()
36+
delete(pod.Labels, "inditex.com/overcommit-class")
3637

37-
cpuOvercommit, memoryOvercommit := checkOvercommitType(context.TODO(), *testPod, k8sClient)
38-
Expect(cpuOvercommit).To(Equal(0.5))
39-
Expect(memoryOvercommit).To(Equal(0.5))
38+
resolution := checkOvercommitType(context.TODO(), *pod, k8sClient)
39+
Expect(resolution.className).To(Equal("test-class"))
40+
Expect(resolution.cpuValue).To(Equal(0.5))
41+
Expect(resolution.memoryValue).To(Equal(0.5))
4042
})
4143
})
4244
})

pkg/overcommit/make_overcommit.go

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ func mutateContainers(containers []corev1.Container, cpuValue float64, memoryVal
5252
}
5353

5454
func Overcommit(ctx context.Context, pod *corev1.Pod, recorder record.EventRecorder, client client.Client) {
55-
className := os.Getenv("OVERCOMMIT_CLASS_NAME")
55+
webhookClassName := os.Getenv("OVERCOMMIT_CLASS_NAME")
56+
resolution := checkOvercommitType(ctx, *pod, client)
57+
className := resolution.className
58+
if className == "" {
59+
className = webhookClassName
60+
}
5661

5762
metrics.K8sOvercommitOperatorPodsRequestedTotal.WithLabelValues(className).Inc()
5863

@@ -64,19 +69,20 @@ func Overcommit(ctx context.Context, pod *corev1.Pod, recorder record.EventRecor
6469
}
6570
}
6671

67-
cpuValue, memoryValue := checkOvercommitType(ctx, *pod, client)
68-
69-
mutateContainers(pod.Spec.Containers, cpuValue, memoryValue)
72+
mutateContainers(pod.Spec.Containers, resolution.cpuValue, resolution.memoryValue)
7073

7174
// Also mutate init containers on regular CREATE/UPDATE
7275
if len(pod.Spec.InitContainers) > 0 {
73-
mutateContainers(pod.Spec.InitContainers, cpuValue, memoryValue)
76+
mutateContainers(pod.Spec.InitContainers, resolution.cpuValue, resolution.memoryValue)
7477
}
7578

7679
// Mark the pod as mutated to prevent double-application on reinvocation
77-
setOvercommitAnnotation(pod, className, cpuValue, memoryValue)
80+
setOvercommitAnnotation(pod, className, resolution.cpuValue, resolution.memoryValue)
7881

7982
metrics.K8sOvercommitOperatorMutatedPodsTotal.WithLabelValues(className).Inc()
83+
if resolution.resolved {
84+
metrics.K8sOvercommitPodMutated.WithLabelValues(className, resolution.ownerKind, resolution.ownerName, pod.Namespace).Inc()
85+
}
8086

8187
recorder.Eventf(
8288
pod,
@@ -85,25 +91,31 @@ func Overcommit(ctx context.Context, pod *corev1.Pod, recorder record.EventRecor
8591
"Applied overcommit to Pod '%s': OvercommitClass = %s, CPU Overcommit = %.2f, Memory Overcommit = %.2f",
8692
pod.Name,
8793
className,
88-
cpuValue,
89-
memoryValue,
94+
resolution.cpuValue,
95+
resolution.memoryValue,
9096
)
9197
}
9298

9399
func OvercommitOnResize(ctx context.Context, pod *corev1.Pod, recorder record.EventRecorder, client client.Client) {
94-
className := os.Getenv("OVERCOMMIT_CLASS_NAME")
100+
webhookClassName := os.Getenv("OVERCOMMIT_CLASS_NAME")
101+
resolution := checkOvercommitType(ctx, *pod, client)
102+
className := resolution.className
103+
if className == "" {
104+
className = webhookClassName
105+
}
95106

96107
metrics.K8sOvercommitOperatorPodsRequestedTotal.WithLabelValues(className).Inc()
97108

98-
cpuValue, memoryValue := checkOvercommitType(ctx, *pod, client)
99-
100109
// On resize: only mutate regular containers, skip init containers.
101-
mutateContainers(pod.Spec.Containers, cpuValue, memoryValue)
110+
mutateContainers(pod.Spec.Containers, resolution.cpuValue, resolution.memoryValue)
102111

103112
// Update annotation with new values after resize
104-
setOvercommitAnnotation(pod, className, cpuValue, memoryValue)
113+
setOvercommitAnnotation(pod, className, resolution.cpuValue, resolution.memoryValue)
105114

106115
metrics.K8sOvercommitOperatorMutatedPodsTotal.WithLabelValues(className).Inc()
116+
if resolution.resolved {
117+
metrics.K8sOvercommitPodMutated.WithLabelValues(className, resolution.ownerKind, resolution.ownerName, pod.Namespace).Inc()
118+
}
107119

108120
recorder.Eventf(
109121
pod,
@@ -112,8 +124,8 @@ func OvercommitOnResize(ctx context.Context, pod *corev1.Pod, recorder record.Ev
112124
"Applied overcommit on resize to Pod '%s': OvercommitClass = %s, CPU Overcommit = %.2f, Memory Overcommit = %.2f",
113125
pod.Name,
114126
className,
115-
cpuValue,
116-
memoryValue,
127+
resolution.cpuValue,
128+
resolution.memoryValue,
117129
)
118130
}
119131

pkg/overcommit/make_overcommit_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ var _ = Describe("Overcommit", func() {
9595
Overcommit(context.Background(), pod, recorder, k8sClient)
9696

9797
Expect(pod.Spec.Containers[0].Resources.Requests).To(Equal(expectedRequests))
98+
Expect(pod.Annotations[AnnotationOvercommitApplied]).To(Equal("test-class"))
9899
})
99100
})
100101

@@ -112,6 +113,7 @@ var _ = Describe("Overcommit", func() {
112113
Overcommit(context.Background(), pod, recorder, k8sClient)
113114

114115
Expect(pod.Spec.Containers[0].Resources.Requests).To(Equal(expectedRequests))
116+
Expect(pod.Annotations[AnnotationOvercommitApplied]).To(Equal("test-class"))
115117
})
116118

117119
})

test/e2e/kuttl/1-03_validate_namespace_labeled/00-assert.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ kind: Pod
88
metadata:
99
name: busybox-test3
1010
namespace: test-3
11+
annotations:
12+
overcommit.inditex.dev/applied: "test3"
13+
overcommit.inditex.dev/cpu: "0.2500"
14+
overcommit.inditex.dev/memory: "0.2500"
1115
spec:
1216
containers:
1317
- name: busybox3
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# SPDX-FileCopyrightText: 2025 2025 INDUSTRIA DE DISEÑO TEXTIL S.A. (INDITEX S.A.)
2+
# SPDX-FileContributor: enriqueavi@inditex.com
3+
#
4+
# SPDX-License-Identifier: Apache-2.0
5+
6+
apiVersion: v1
7+
kind: Pod
8+
metadata:
9+
name: issue-47-pod
10+
namespace: issue-47-namespace
11+
annotations:
12+
overcommit.inditex.dev/applied: "issue-47-first"
13+
overcommit.inditex.dev/cpu: "0.2500"
14+
overcommit.inditex.dev/memory: "0.7500"
15+
spec:
16+
containers:
17+
- name: busybox
18+
resources:
19+
requests:
20+
memory: "768Mi"
21+
cpu: "250m"
22+
limits:
23+
memory: "1Gi"
24+
cpu: "1"
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# SPDX-FileCopyrightText: 2025 2025 INDUSTRIA DE DISEÑO TEXTIL S.A. (INDITEX S.A.)
2+
# SPDX-FileContributor: enriqueavi@inditex.com
3+
#
4+
# SPDX-License-Identifier: Apache-2.0
5+
6+
apiVersion: overcommit.inditex.dev/v1alphav1
7+
kind: OvercommitClass
8+
metadata:
9+
name: issue-47-first
10+
spec:
11+
cpuOvercommit: 0.25
12+
memoryOvercommit: 0.75
13+
isDefault: false
14+
excludedNamespaces: ".*(^(openshift|k8s-overcommit|kube).*).*"
15+
---
16+
apiVersion: v1
17+
kind: Namespace
18+
metadata:
19+
name: issue-47-namespace
20+
labels:
21+
inditex.com/overcommit-class: issue-47-first
22+
---
23+
apiVersion: v1
24+
kind: Pod
25+
metadata:
26+
name: issue-47-pod
27+
namespace: issue-47-namespace
28+
spec:
29+
containers:
30+
- name: busybox
31+
image: busybox
32+
command: ["sh", "-c", "sleep 3600"]
33+
resources:
34+
requests:
35+
memory: "100Mi"
36+
cpu: "100m"
37+
limits:
38+
memory: "1Gi"
39+
cpu: "1"

0 commit comments

Comments
 (0)