diff --git a/internal/utils/getOvercommitClass.go b/internal/utils/getOvercommitClass.go index e4c7cf0..fff1846 100644 --- a/internal/utils/getOvercommitClass.go +++ b/internal/utils/getOvercommitClass.go @@ -46,6 +46,15 @@ func GetOvercommitClassSpec(ctx context.Context, name string, k8sClient client.C } func GetDefaultSpec(ctx context.Context, k8sClient client.Client) (*overcommit.OvercommitClassSpec, error) { + defaultClass, err := GetDefaultClass(ctx, k8sClient) + if err != nil { + return nil, err + } + + return &defaultClass.Spec, nil +} + +func GetDefaultClass(ctx context.Context, k8sClient client.Client) (*overcommit.OvercommitClass, error) { if k8sClient == nil { return nil, errors.New("client parameter cannot be nil") } @@ -57,10 +66,10 @@ func GetDefaultSpec(ctx context.Context, k8sClient client.Client) (*overcommit.O } // Find the default OvercommitClass - for _, overcommitClass := range overcommitClasses.Items { - if overcommitClass.Spec.IsDefault { - podlog.Info("Default OvercommitClass found", "name", overcommitClass.Name) - return &overcommitClass.Spec, nil + for i := range overcommitClasses.Items { + if overcommitClasses.Items[i].Spec.IsDefault { + podlog.Info("Default OvercommitClass found", "name", overcommitClasses.Items[i].Name) + return &overcommitClasses.Items[i], nil } } diff --git a/pkg/overcommit/calculate_values_from_labels.go b/pkg/overcommit/calculate_values_from_labels.go index 92625ee..48f09e9 100644 --- a/pkg/overcommit/calculate_values_from_labels.go +++ b/pkg/overcommit/calculate_values_from_labels.go @@ -9,22 +9,30 @@ package overcommit import ( "context" - "github.com/InditexTech/k8s-overcommit-operator/internal/metrics" "github.com/InditexTech/k8s-overcommit-operator/internal/utils" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) +type overcommitResolution struct { + className string + cpuValue float64 + memoryValue float64 + ownerName string + ownerKind string + resolved bool +} + // getNamespaceOvercommit gets the overcommit values from the namespace label or falls back to the default class. -// Returns (1.0, 1.0) as safe no-op values when any error occurs to avoid mutating pods incorrectly. -func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, k8sClient client.Client, label, ownerName, ownerKind string) (float64, float64) { +// Returns safe no-op values when any error occurs to avoid mutating pods incorrectly. +func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, k8sClient client.Client, label, ownerName, ownerKind string) overcommitResolution { // Get the namespace of the pod namespaceName := pod.Namespace var ns corev1.Namespace err := k8sClient.Get(ctx, client.ObjectKey{Name: namespaceName}, &ns) if err != nil { podlog.Error(err, "Error getting the namespace", "namespace", namespaceName) - return 1.0, 1.0 + return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind} } // Check if the overcommit class label is in the namespace @@ -33,23 +41,35 @@ func getNamespaceOvercommit(ctx context.Context, pod *corev1.Pod, k8sClient clie overcommitClass, err := utils.GetOvercommitClassSpec(ctx, val, k8sClient) if err != nil { podlog.Error(err, "Error getting the overcommit class", "overcommitClassLabel", val) - return 1.0, 1.0 + return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind} + } + return overcommitResolution{ + className: val, + cpuValue: overcommitClass.CpuOvercommit, + memoryValue: overcommitClass.MemoryOvercommit, + ownerName: ownerName, + ownerKind: ownerKind, + resolved: true, } - metrics.K8sOvercommitPodMutated.WithLabelValues(val, ownerKind, ownerName, pod.Namespace).Inc() - return overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit } podlog.Info("Overcommit class not found in the namespace, using the default", "namespace", ns.Name) - defaultClass, err := utils.GetDefaultSpec(ctx, k8sClient) + defaultClass, err := utils.GetDefaultClass(ctx, k8sClient) if err != nil { podlog.Error(err, "Error getting the default overcommit class") - return 1.0, 1.0 + return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind} + } + return overcommitResolution{ + className: defaultClass.Name, + cpuValue: defaultClass.Spec.CpuOvercommit, + memoryValue: defaultClass.Spec.MemoryOvercommit, + ownerName: ownerName, + ownerKind: ownerKind, + resolved: true, } - metrics.K8sOvercommitPodMutated.WithLabelValues("default", ownerKind, ownerName, pod.Namespace).Inc() - return defaultClass.CpuOvercommit, defaultClass.MemoryOvercommit } -func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Client) (float64, float64) { +func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Client) overcommitResolution { ownerName, ownerKind, err := utils.GetPodOwner(ctx, client, &pod) if err != nil { podlog.Error(err, "Error getting the pod owner") @@ -59,7 +79,7 @@ func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Clie label, err := utils.GetOvercommitLabel(ctx, client) if err != nil { podlog.Error(err, "Error getting the overcommit label") - return 1.0, 1.0 + return overcommitResolution{cpuValue: 1.0, memoryValue: 1.0, ownerName: ownerName, ownerKind: ownerKind} } // Check if the pod has the overcommit class label value, exists := pod.Labels[label] @@ -76,8 +96,14 @@ func checkOvercommitType(ctx context.Context, pod corev1.Pod, client client.Clie // Overcommit class not found or some error, fall back to namespace/default return getNamespaceOvercommit(ctx, &pod, client, label, ownerName, ownerKind) } - metrics.K8sOvercommitPodMutated.WithLabelValues(value, ownerKind, ownerName, pod.Namespace).Inc() - return overcommitClass.CpuOvercommit, overcommitClass.MemoryOvercommit + return overcommitResolution{ + className: value, + cpuValue: overcommitClass.CpuOvercommit, + memoryValue: overcommitClass.MemoryOvercommit, + ownerName: ownerName, + ownerKind: ownerKind, + resolved: true, + } } // Overcommit class not found, checking the namespace diff --git a/pkg/overcommit/calculate_values_from_labels_test.go b/pkg/overcommit/calculate_values_from_labels_test.go index 0fe285a..7eda5b8 100644 --- a/pkg/overcommit/calculate_values_from_labels_test.go +++ b/pkg/overcommit/calculate_values_from_labels_test.go @@ -16,27 +16,29 @@ var _ = Describe("Overcommit Functions", func() { Describe("getNamespaceOvercommit", func() { It("should return the correct overcommit values from the namespace", func() { - cpuOvercommit, memoryOvercommit := getNamespaceOvercommit(context.TODO(), testPod, k8sClient, "inditex.com/overcommit-class", "ownerName", "ownerKind") - Expect(cpuOvercommit).To(Equal(0.5)) - Expect(memoryOvercommit).To(Equal(0.5)) + resolution := getNamespaceOvercommit(context.TODO(), testPod, k8sClient, "inditex.com/overcommit-class", "ownerName", "ownerKind") + Expect(resolution.className).To(Equal("test-class")) + Expect(resolution.cpuValue).To(Equal(0.5)) + Expect(resolution.memoryValue).To(Equal(0.5)) }) }) Describe("checkOvercommitType", func() { It("should return the correct overcommit values from the pod", func() { - cpuOvercommit, memoryOvercommit := checkOvercommitType(context.TODO(), *testPod, k8sClient) - Expect(cpuOvercommit).To(Equal(0.5)) - Expect(memoryOvercommit).To(Equal(0.5)) + resolution := checkOvercommitType(context.TODO(), *testPod, k8sClient) + Expect(resolution.className).To(Equal("test-class")) + Expect(resolution.cpuValue).To(Equal(0.5)) + Expect(resolution.memoryValue).To(Equal(0.5)) }) It("should fallback to namespace overcommit values if pod label is missing", func() { - delete(testPod.Labels, "LABEL_OVERCOMMIT_CLASS") - err := k8sClient.Update(context.TODO(), testPod) - Expect(err).NotTo(HaveOccurred()) + pod := testPod.DeepCopy() + delete(pod.Labels, "inditex.com/overcommit-class") - cpuOvercommit, memoryOvercommit := checkOvercommitType(context.TODO(), *testPod, k8sClient) - Expect(cpuOvercommit).To(Equal(0.5)) - Expect(memoryOvercommit).To(Equal(0.5)) + resolution := checkOvercommitType(context.TODO(), *pod, k8sClient) + Expect(resolution.className).To(Equal("test-class")) + Expect(resolution.cpuValue).To(Equal(0.5)) + Expect(resolution.memoryValue).To(Equal(0.5)) }) }) }) diff --git a/pkg/overcommit/make_overcommit.go b/pkg/overcommit/make_overcommit.go index b4da2e1..14b4f46 100644 --- a/pkg/overcommit/make_overcommit.go +++ b/pkg/overcommit/make_overcommit.go @@ -52,7 +52,12 @@ func mutateContainers(containers []corev1.Container, cpuValue float64, memoryVal } func Overcommit(ctx context.Context, pod *corev1.Pod, recorder record.EventRecorder, client client.Client) { - className := os.Getenv("OVERCOMMIT_CLASS_NAME") + webhookClassName := os.Getenv("OVERCOMMIT_CLASS_NAME") + resolution := checkOvercommitType(ctx, *pod, client) + className := resolution.className + if className == "" { + className = webhookClassName + } metrics.K8sOvercommitOperatorPodsRequestedTotal.WithLabelValues(className).Inc() @@ -64,19 +69,20 @@ func Overcommit(ctx context.Context, pod *corev1.Pod, recorder record.EventRecor } } - cpuValue, memoryValue := checkOvercommitType(ctx, *pod, client) - - mutateContainers(pod.Spec.Containers, cpuValue, memoryValue) + mutateContainers(pod.Spec.Containers, resolution.cpuValue, resolution.memoryValue) // Also mutate init containers on regular CREATE/UPDATE if len(pod.Spec.InitContainers) > 0 { - mutateContainers(pod.Spec.InitContainers, cpuValue, memoryValue) + mutateContainers(pod.Spec.InitContainers, resolution.cpuValue, resolution.memoryValue) } // Mark the pod as mutated to prevent double-application on reinvocation - setOvercommitAnnotation(pod, className, cpuValue, memoryValue) + setOvercommitAnnotation(pod, className, resolution.cpuValue, resolution.memoryValue) metrics.K8sOvercommitOperatorMutatedPodsTotal.WithLabelValues(className).Inc() + if resolution.resolved { + metrics.K8sOvercommitPodMutated.WithLabelValues(className, resolution.ownerKind, resolution.ownerName, pod.Namespace).Inc() + } recorder.Eventf( pod, @@ -85,25 +91,31 @@ func Overcommit(ctx context.Context, pod *corev1.Pod, recorder record.EventRecor "Applied overcommit to Pod '%s': OvercommitClass = %s, CPU Overcommit = %.2f, Memory Overcommit = %.2f", pod.Name, className, - cpuValue, - memoryValue, + resolution.cpuValue, + resolution.memoryValue, ) } func OvercommitOnResize(ctx context.Context, pod *corev1.Pod, recorder record.EventRecorder, client client.Client) { - className := os.Getenv("OVERCOMMIT_CLASS_NAME") + webhookClassName := os.Getenv("OVERCOMMIT_CLASS_NAME") + resolution := checkOvercommitType(ctx, *pod, client) + className := resolution.className + if className == "" { + className = webhookClassName + } metrics.K8sOvercommitOperatorPodsRequestedTotal.WithLabelValues(className).Inc() - cpuValue, memoryValue := checkOvercommitType(ctx, *pod, client) - // On resize: only mutate regular containers, skip init containers. - mutateContainers(pod.Spec.Containers, cpuValue, memoryValue) + mutateContainers(pod.Spec.Containers, resolution.cpuValue, resolution.memoryValue) // Update annotation with new values after resize - setOvercommitAnnotation(pod, className, cpuValue, memoryValue) + setOvercommitAnnotation(pod, className, resolution.cpuValue, resolution.memoryValue) metrics.K8sOvercommitOperatorMutatedPodsTotal.WithLabelValues(className).Inc() + if resolution.resolved { + metrics.K8sOvercommitPodMutated.WithLabelValues(className, resolution.ownerKind, resolution.ownerName, pod.Namespace).Inc() + } recorder.Eventf( pod, @@ -112,8 +124,8 @@ func OvercommitOnResize(ctx context.Context, pod *corev1.Pod, recorder record.Ev "Applied overcommit on resize to Pod '%s': OvercommitClass = %s, CPU Overcommit = %.2f, Memory Overcommit = %.2f", pod.Name, className, - cpuValue, - memoryValue, + resolution.cpuValue, + resolution.memoryValue, ) } diff --git a/pkg/overcommit/make_overcommit_test.go b/pkg/overcommit/make_overcommit_test.go index ffb8bec..73c1558 100644 --- a/pkg/overcommit/make_overcommit_test.go +++ b/pkg/overcommit/make_overcommit_test.go @@ -95,6 +95,7 @@ var _ = Describe("Overcommit", func() { Overcommit(context.Background(), pod, recorder, k8sClient) Expect(pod.Spec.Containers[0].Resources.Requests).To(Equal(expectedRequests)) + Expect(pod.Annotations[AnnotationOvercommitApplied]).To(Equal("test-class")) }) }) @@ -112,6 +113,7 @@ var _ = Describe("Overcommit", func() { Overcommit(context.Background(), pod, recorder, k8sClient) Expect(pod.Spec.Containers[0].Resources.Requests).To(Equal(expectedRequests)) + Expect(pod.Annotations[AnnotationOvercommitApplied]).To(Equal("test-class")) }) }) diff --git a/test/e2e/kuttl/1-03_validate_namespace_labeled/00-assert.yaml b/test/e2e/kuttl/1-03_validate_namespace_labeled/00-assert.yaml index e34af23..6903188 100644 --- a/test/e2e/kuttl/1-03_validate_namespace_labeled/00-assert.yaml +++ b/test/e2e/kuttl/1-03_validate_namespace_labeled/00-assert.yaml @@ -8,6 +8,10 @@ kind: Pod metadata: name: busybox-test3 namespace: test-3 + annotations: + overcommit.inditex.dev/applied: "test3" + overcommit.inditex.dev/cpu: "0.2500" + overcommit.inditex.dev/memory: "0.2500" spec: containers: - name: busybox3 diff --git a/test/e2e/kuttl/1-10_validate_namespace_label_applied_annotation/00-assert.yaml b/test/e2e/kuttl/1-10_validate_namespace_label_applied_annotation/00-assert.yaml new file mode 100644 index 0000000..ed7d74f --- /dev/null +++ b/test/e2e/kuttl/1-10_validate_namespace_label_applied_annotation/00-assert.yaml @@ -0,0 +1,24 @@ +# SPDX-FileCopyrightText: 2025 2025 INDUSTRIA DE DISEÑO TEXTIL S.A. (INDITEX S.A.) +# SPDX-FileContributor: enriqueavi@inditex.com +# +# SPDX-License-Identifier: Apache-2.0 + +apiVersion: v1 +kind: Pod +metadata: + name: issue-47-pod + namespace: issue-47-namespace + annotations: + overcommit.inditex.dev/applied: "issue-47-first" + overcommit.inditex.dev/cpu: "0.2500" + overcommit.inditex.dev/memory: "0.7500" +spec: + containers: + - name: busybox + resources: + requests: + memory: "768Mi" + cpu: "250m" + limits: + memory: "1Gi" + cpu: "1" diff --git a/test/e2e/kuttl/1-10_validate_namespace_label_applied_annotation/00-setup.yaml b/test/e2e/kuttl/1-10_validate_namespace_label_applied_annotation/00-setup.yaml new file mode 100644 index 0000000..bf13ecf --- /dev/null +++ b/test/e2e/kuttl/1-10_validate_namespace_label_applied_annotation/00-setup.yaml @@ -0,0 +1,39 @@ +# SPDX-FileCopyrightText: 2025 2025 INDUSTRIA DE DISEÑO TEXTIL S.A. (INDITEX S.A.) +# SPDX-FileContributor: enriqueavi@inditex.com +# +# SPDX-License-Identifier: Apache-2.0 + +apiVersion: overcommit.inditex.dev/v1alphav1 +kind: OvercommitClass +metadata: + name: issue-47-first +spec: + cpuOvercommit: 0.25 + memoryOvercommit: 0.75 + isDefault: false + excludedNamespaces: ".*(^(openshift|k8s-overcommit|kube).*).*" +--- +apiVersion: v1 +kind: Namespace +metadata: + name: issue-47-namespace + labels: + inditex.com/overcommit-class: issue-47-first +--- +apiVersion: v1 +kind: Pod +metadata: + name: issue-47-pod + namespace: issue-47-namespace +spec: + containers: + - name: busybox + image: busybox + command: ["sh", "-c", "sleep 3600"] + resources: + requests: + memory: "100Mi" + cpu: "100m" + limits: + memory: "1Gi" + cpu: "1"