Fix virtual machine resource tracking (#904) (#916)

* 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.

(cherry picked from commit 1e59e5fbb6)
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>

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
 }

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)
+	}
+}

internal/controller/workloadmonitor_controller.go

@@ -198,15 +198,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
@@ -235,7 +232,7 @@ func (r *WorkloadMonitorReconciler) reconcilePodForMonitor(
 
 	workload := &cozyv1alpha1.Workload{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      pod.Name,
+			Name:      fmt.Sprintf("pod-%s", pod.Name),
 			Namespace: pod.Namespace,
 		},
 	}