fix: remove GC-owned/namespace-scoped resources from CleanOrphanedResources

Signed-off-by: Liam Beckman <lbeckman314@gmail.com>

Assisted-by: Claude:claude-sonnet-4-5

Author: lbeckman314

compute/kubernetes/backend.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strings"
 	"time"
 
 	"dario.cat/mergo"
@@ -596,81 +595,35 @@ func (b *Backend) CleanOrphanedResources(ctx context.Context) {
 	namespace := b.conf.Kubernetes.JobsNamespace
 	taskIDs := make(map[string]struct{})
 
-	// Collect task IDs from each resource type
-	if pvcs, err := b.client.CoreV1().PersistentVolumeClaims(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=funnel"}); err == nil {
-		if err != nil {
-			b.log.Error("backlog cleanup: listing PVCs", err)
-		}
-		for _, r := range pvcs.Items {
-			if id, ok := r.Labels["taskId"]; ok {
-				taskIDs[id] = struct{}{}
-			}
-		}
-	}
+	// Collect task IDs from resources that cleanResources manages directly.
+	// ConfigMaps, PVCs, Roles, and RoleBindings are now owned by the Job via ownerReferences
+	// and are garbage-collected by Kubernetes automatically — they are intentionally excluded here.
 
-	// PVs
-	if pvs, err := b.client.CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("app=funnel,namespace=%s", namespace)}); err == nil {
-		if err != nil {
-			b.log.Error("backlog cleanup: listing PVs", err)
-		}
+	// PVs (cluster-scoped; cannot be owned by a namespaced Job, so must be cleaned explicitly)
+	pvs, err := b.client.CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{LabelSelector: fmt.Sprintf("app=funnel,namespace=%s", namespace)})
+	if err != nil {
+		b.log.Error("backlog cleanup: listing PVs", err)
+	} else {
 		for _, r := range pvs.Items {
 			if id, ok := r.Labels["taskId"]; ok {
 				taskIDs[id] = struct{}{}
 			}
 		}
 	}
 
-	// ConfigMaps
-	if cms, err := b.client.CoreV1().ConfigMaps(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=funnel"}); err == nil {
-		if err != nil {
-			b.log.Error("backlog cleanup: listing ConfigMaps", err)
-		}
-		const cmPrefix = "funnel-worker-config-"
-		for _, r := range cms.Items {
-			if id, ok := r.Labels["taskId"]; ok {
-				taskIDs[id] = struct{}{}
-			} else if strings.HasPrefix(r.Name, cmPrefix) {
-				taskIDs[strings.TrimPrefix(r.Name, cmPrefix)] = struct{}{}
-			}
-		}
-	}
-
-	// ServiceAccounts
-	if sas, err := b.client.CoreV1().ServiceAccounts(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=funnel"}); err == nil {
-		if err != nil {
-			b.log.Error("backlog cleanup: listing ServiceAccounts", err)
-		}
+	// ServiceAccounts (shared SAs are not owned by a Job; task-scoped SAs may also be orphaned
+	// if they were created before ownerRef support was added)
+	sas, err := b.client.CoreV1().ServiceAccounts(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=funnel"})
+	if err != nil {
+		b.log.Error("backlog cleanup: listing ServiceAccounts", err)
+	} else {
 		for _, r := range sas.Items {
 			if id, ok := r.Labels["taskId"]; ok {
 				taskIDs[id] = struct{}{}
 			}
 		}
 	}
 
-	// Roles
-	if roles, err := b.client.RbacV1().Roles(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=funnel"}); err == nil {
-		if err != nil {
-			b.log.Error("backlog cleanup: listing Roles", err)
-		}
-		for _, r := range roles.Items {
-			if id, ok := r.Labels["taskId"]; ok {
-				taskIDs[id] = struct{}{}
-			}
-		}
-	}
-
-	// RoleBindings
-	if rbs, err := b.client.RbacV1().RoleBindings(namespace).List(ctx, metav1.ListOptions{LabelSelector: "app=funnel"}); err == nil {
-		if err != nil {
-			b.log.Error("backlog cleanup: listing RoleBindings", err)
-		}
-		for _, r := range rbs.Items {
-			if id, ok := r.Labels["taskId"]; ok {
-				taskIDs[id] = struct{}{}
-			}
-		}
-	}
-
 	// TODO: Add Executor Jobs here beacause orphaned tasks can result in orphaned jobs
 
 	for taskID := range taskIDs {

compute/kubernetes/backend_test.go

@@ -86,14 +86,6 @@ func TestTaskSubmission(t *testing.T) {
 	// Create a fake Kubernetes client
 	fakeClient := fake.NewSimpleClientset()
 
-	// Inject a deterministic UID on every Job create so ownerRef propagation can be verified.
-	const testJobUID = "test-job-uid-1234"
-	fakeClient.PrependReactor("create", "jobs", func(action k8stesting.Action) (bool, runtime.Object, error) {
-		obj := action.(k8stesting.CreateAction).GetObject().(*batchv1.Job)
-		obj.UID = testJobUID
-		return false, obj, nil
-	})
-
 	// Create a mock configuration
 	conf := config.DefaultConfig()
 	conf.Kubernetes.Namespace = "test-namespace"
@@ -163,17 +155,20 @@ spec:
 		t.Errorf("expected Job name '%s', got '%s'", task.Id, job.Name)
 	}
 
-	// Verify that the ConfigMap was created with the Job's UID in its ownerRef.
-	configMapName := "funnel-worker-config-" + task.Id
-	cm, err := fakeClient.CoreV1().ConfigMaps(conf.Kubernetes.JobsNamespace).Get(context.Background(), configMapName, metav1.GetOptions{})
+	// Seed a PV so we can verify cleanResources deletes it.
+	pvName := "funnel-worker-pv-" + task.Id
+	_, err = fakeClient.CoreV1().PersistentVolumes().Create(context.Background(), &corev1.PersistentVolume{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: pvName,
+			Labels: map[string]string{
+				"app":       "funnel",
+				"taskId":    task.Id,
+				"namespace": conf.Kubernetes.JobsNamespace,
+			},
+		},
+	}, metav1.CreateOptions{})
 	if err != nil {
-		t.Fatalf("failed to get ConfigMap: %v", err)
-	}
-	if len(cm.OwnerReferences) == 0 {
-		t.Fatal("expected ConfigMap to have an ownerReference, but got none")
-	}
-	if got := cm.OwnerReferences[0].UID; got != testJobUID {
-		t.Errorf("expected ConfigMap ownerRef UID %q, got %q", testJobUID, got)
+		t.Fatalf("failed to create test PV: %v", err)
 	}
 
 	// Clean up resources
@@ -188,10 +183,11 @@ spec:
 		t.Error("expected Job to be deleted, but it still exists")
 	}
 
-	// ConfigMap deletion is handled by Kubernetes garbage collection via ownerReferences,
-	// not explicitly by cleanResources. The fake clientset does not simulate cascading GC,
-	// so we only verify the ownerRef is set correctly (asserted above).
-
+	// Verify that the PV was deleted
+	_, err = fakeClient.CoreV1().PersistentVolumes().Get(context.Background(), pvName, metav1.GetOptions{})
+	if err == nil {
+		t.Error("expected PV to be deleted, but it still exists")
+	}
 }
 
 func TestSubmit_AppliesNodeSelectorAndTolerationsToWorkerJob(t *testing.T) {