From ec9e26c054ec32b1a86ea7c1f2b8a399be1759ad Mon Sep 17 00:00:00 2001
From: Timofei Larkin <lllamnyp@gmail.com>
Date: Tue, 29 Apr 2025 15:22:42 +0300
Subject: [PATCH] Fix virtual machine resource tracking

* Count Workload resources for pods by requests, not limits
* Do not count init container requests
* Prefix Workloads for pods with `pod-`, just like the other types to
  prevent possible name collisions (closes #787)

The previous version of the WorkloadMonitor controller incorrectly
summed resource limits on pods, rather than requests. This prevented it
from tracking the resource allocation for pods, which only had requests
specified, which is particularly the case for kubevirt's virtual machine
pods. Additionally, it counted the limits for all containers, including
init containers, which are short-lived and do not contribute much to the
total resource usage.

Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
---
 internal/controller/workload_controller.go    | 24 ++++++++++++-----
 .../controller/workload_controller_test.go    | 26 +++++++++++++++++++
 .../controller/workloadmonitor_controller.go  | 13 ++++------
 3 files changed, 49 insertions(+), 14 deletions(-)
 create mode 100644 internal/controller/workload_controller_test.go

diff --git a/internal/controller/workload_controller.go b/internal/controller/workload_controller.go
index 7b7f8406..3624e0e1 100644
--- a/internal/controller/workload_controller.go
+++ b/internal/controller/workload_controller.go
@@ -39,6 +39,15 @@ func (r *WorkloadReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	}
 
 	t := getMonitoredObject(w)
+
+	if t == nil {
+		err = r.Delete(ctx, w)
+		if err != nil {
+			logger.Error(err, "failed to delete workload")
+		}
+		return ctrl.Result{}, err
+	}
+
 	err = r.Get(ctx, types.NamespacedName{Name: t.GetName(), Namespace: t.GetNamespace()}, t)
 
 	// found object, nothing to do
@@ -68,20 +77,23 @@ func (r *WorkloadReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func getMonitoredObject(w *cozyv1alpha1.Workload) client.Object {
-	if strings.HasPrefix(w.Name, "pvc-") {
+	switch {
+	case strings.HasPrefix(w.Name, "pvc-"):
 		obj := &corev1.PersistentVolumeClaim{}
 		obj.Name = strings.TrimPrefix(w.Name, "pvc-")
 		obj.Namespace = w.Namespace
 		return obj
-	}
-	if strings.HasPrefix(w.Name, "svc-") {
+	case strings.HasPrefix(w.Name, "svc-"):
 		obj := &corev1.Service{}
 		obj.Name = strings.TrimPrefix(w.Name, "svc-")
 		obj.Namespace = w.Namespace
 		return obj
+	case strings.HasPrefix(w.Name, "pod-"):
+		obj := &corev1.Pod{}
+		obj.Name = strings.TrimPrefix(w.Name, "pod-")
+		obj.Namespace = w.Namespace
+		return obj
 	}
-	obj := &corev1.Pod{}
-	obj.Name = w.Name
-	obj.Namespace = w.Namespace
+	var obj client.Object
 	return obj
 }
diff --git a/internal/controller/workload_controller_test.go b/internal/controller/workload_controller_test.go
new file mode 100644
index 00000000..816c135e
--- /dev/null
+++ b/internal/controller/workload_controller_test.go
@@ -0,0 +1,26 @@
+package controller
+
+import (
+	"testing"
+
+	cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestUnprefixedMonitoredObjectReturnsNil(t *testing.T) {
+	w := &cozyv1alpha1.Workload{}
+	w.Name = "unprefixed-name"
+	obj := getMonitoredObject(w)
+	if obj != nil {
+		t.Errorf(`getMonitoredObject(&Workload{Name: "%s"}) == %v, want nil`, w.Name, obj)
+	}
+}
+
+func TestPodMonitoredObject(t *testing.T) {
+	w := &cozyv1alpha1.Workload{}
+	w.Name = "pod-mypod"
+	obj := getMonitoredObject(w)
+	if pod, ok := obj.(*corev1.Pod); !ok || pod.Name != "mypod" {
+		t.Errorf(`getMonitoredObject(&Workload{Name: "%s"}) == %v, want &Pod{Name: "mypod"}`, w.Name, obj)
+	}
+}
diff --git a/internal/controller/workloadmonitor_controller.go b/internal/controller/workloadmonitor_controller.go
index 330d1dc5..a1d4a501 100644
--- a/internal/controller/workloadmonitor_controller.go
+++ b/internal/controller/workloadmonitor_controller.go
@@ -212,15 +212,12 @@ func (r *WorkloadMonitorReconciler) reconcilePodForMonitor(
 ) error {
 	logger := log.FromContext(ctx)
 
-	// Combine both init containers and normal containers to sum resources properly
-	combinedContainers := append(pod.Spec.InitContainers, pod.Spec.Containers...)
-
-	// totalResources will store the sum of all container resource limits
+	// totalResources will store the sum of all container resource requests
 	totalResources := make(map[string]resource.Quantity)
 
-	// Iterate over all containers to aggregate their Limits
-	for _, container := range combinedContainers {
-		for name, qty := range container.Resources.Limits {
+	// Iterate over all containers to aggregate their requests
+	for _, container := range pod.Spec.Containers {
+		for name, qty := range container.Resources.Requests {
 			if existing, exists := totalResources[name.String()]; exists {
 				existing.Add(qty)
 				totalResources[name.String()] = existing
@@ -249,7 +246,7 @@ func (r *WorkloadMonitorReconciler) reconcilePodForMonitor(
 
 	workload := &cozyv1alpha1.Workload{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      pod.Name,
+			Name:      fmt.Sprintf("pod-%s", pod.Name),
 			Namespace: pod.Namespace,
 		},
 	}