fix(meta): derive VM role from pod owner for snapshot decisions

ShouldSnapshotVM under main-only parsed the VM name with a last-dash
heuristic, so a toolbox named with a trailing -0 was misread as the main
agent and snapshotted. Role now comes from the pod's CocoonSet owner via
ExtractAgentSlot; the deprecated ExtractSlotFromVMName/InferRoleFromVMName
pair is removed.

Author: CMGS

meta/meta_test.go

@@ -14,21 +14,6 @@ func TestVMNamingHelpers(t *testing.T) {
 	if got := VMNameForPod("prod", "toolbox"); got != "vk-prod-toolbox" {
 		t.Fatalf("pod vm name mismatch: got %q", got)
 	}
-	if got := ExtractSlotFromVMName("vk-prod-demo-2"); got != 2 {
-		t.Fatalf("slot mismatch: got %d", got)
-	}
-	if got := ExtractSlotFromVMName("vk-prod-toolbox"); got != -1 {
-		t.Fatalf("expected non-slot vm name to return -1, got %d", got)
-	}
-}
-
-func TestInferRoleFromVMName(t *testing.T) {
-	if got := InferRoleFromVMName("vk-prod-demo-0"); got != RoleMain {
-		t.Fatalf("expected role %q, got %q", RoleMain, got)
-	}
-	if got := InferRoleFromVMName("vk-prod-demo-3"); got != RoleSubAgent {
-		t.Fatalf("expected role %q, got %q", RoleSubAgent, got)
-	}
 }
 
 func TestExtractAgentSlot(t *testing.T) {
@@ -54,10 +39,9 @@ func TestExtractAgentSlot(t *testing.T) {
 			want:      3,
 		},
 		{
-			// The legacy ExtractSlotFromVMName would mis-read this as
-			// slot 2 because it splits at the last dash. ExtractAgentSlot
-			// rejects it because the suffix after the agent prefix
-			// contains a dash.
+			// A naive last-dash split would misread this as slot 2;
+			// ExtractAgentSlot rejects it because the suffix after the
+			// agent prefix contains a dash.
 			name:      "toolbox with trailing digit is not an agent slot",
 			ns:        "prod",
 			cocoonSet: "demo",
@@ -120,6 +104,50 @@ func TestInferRoleFromAgentSlot(t *testing.T) {
 	}
 }
 
+func TestRoleForPod(t *testing.T) {
+	cocoonSetOwner := []metav1.OwnerReference{{Kind: KindCocoonSet, Name: "cs"}}
+	cases := []struct {
+		name   string
+		pod    *corev1.Pod
+		vmName string
+		want   string
+	}{
+		{
+			name:   "agent slot 0 is main",
+			pod:    &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ns", OwnerReferences: cocoonSetOwner}},
+			vmName: "vk-ns-cs-0",
+			want:   RoleMain,
+		},
+		{
+			name:   "agent slot 2 is sub-agent",
+			pod:    &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ns", OwnerReferences: cocoonSetOwner}},
+			vmName: "vk-ns-cs-2",
+			want:   RoleSubAgent,
+		},
+		{
+			// Regression: toolbox "app-0" builds VM name "vk-ns-cs-app-0",
+			// which a naive last-dash split would misread as agent slot 0.
+			name:   "toolbox named app-0 is not main",
+			pod:    &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ns", OwnerReferences: cocoonSetOwner}},
+			vmName: "vk-ns-cs-app-0",
+			want:   RoleToolbox,
+		},
+		{
+			name:   "no CocoonSet owner is toolbox",
+			pod:    &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ns"}},
+			vmName: "vk-ns-cs-0",
+			want:   RoleToolbox,
+		},
+	}
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := RoleForPod(tt.pod, tt.vmName); got != tt.want {
+				t.Errorf("got %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestConnectionType(t *testing.T) {
 	cases := []struct {
 		name       string

meta/owner.go

@@ -18,9 +18,18 @@ func HasCocoonTolerationKey(tolerations []corev1.Toleration) bool {
 
 // IsOwnedByCocoonSet reports whether any owner reference is a CocoonSet.
 func IsOwnedByCocoonSet(ownerRefs []metav1.OwnerReference) bool {
-	return slices.ContainsFunc(ownerRefs, func(ref metav1.OwnerReference) bool {
-		return ref.Kind == KindCocoonSet
-	})
+	return CocoonSetOwnerName(ownerRefs) != ""
+}
+
+// CocoonSetOwnerName returns the name of the CocoonSet owner reference, or
+// "" if none is present.
+func CocoonSetOwnerName(ownerRefs []metav1.OwnerReference) string {
+	for _, ref := range ownerRefs {
+		if ref.Kind == KindCocoonSet {
+			return ref.Name
+		}
+	}
+	return ""
 }
 
 // OwnerDeploymentName extracts the deployment name from a ReplicaSet

meta/vmname.go

@@ -3,6 +3,8 @@ package meta
 import (
 	"strconv"
 	"strings"
+
+	corev1 "k8s.io/api/core/v1"
 )
 
 // VMNameForDeployment builds a deterministic VM name from a deployment and slot index.
@@ -56,32 +58,11 @@ func InferRoleFromAgentSlot(slot int) string {
 	}
 }
 
-// ExtractSlotFromVMName parses the trailing slot index from a VM name,
-// or -1 if absent.
-//
-// Deprecated: misclassifies toolbox names with numeric suffixes (e.g.
-// "vk-NS-CS-db-2" → slot 2). Prefer ExtractAgentSlot.
-func ExtractSlotFromVMName(vmName string) int {
-	_, after, ok := lastCut(vmName, "-")
-	if !ok {
-		return -1
-	}
-	n, err := strconv.Atoi(after)
-	if err != nil {
-		return -1
-	}
-	return n
-}
-
-// InferRoleFromVMName returns RoleMain for slot 0, RoleSubAgent otherwise.
-//
-// Deprecated: shares the toolbox-collision bug of ExtractSlotFromVMName.
-// Prefer InferRoleFromAgentSlot(ExtractAgentSlot(ns, cs, vmName)).
-func InferRoleFromVMName(vmName string) string {
-	if ExtractSlotFromVMName(vmName) == 0 {
-		return RoleMain
-	}
-	return RoleSubAgent
+// RoleForPod derives a pod's role (RoleMain, RoleSubAgent, RoleToolbox)
+// from its CocoonSet owner and VM name.
+func RoleForPod(pod *corev1.Pod, vmName string) string {
+	cocoonSet := CocoonSetOwnerName(pod.OwnerReferences)
+	return InferRoleFromAgentSlot(ExtractAgentSlot(pod.Namespace, cocoonSet, vmName))
 }
 
 // lastCut is like strings.Cut but splits at the last occurrence of sep.

meta/vmspec.go

@@ -115,13 +115,13 @@ func FromToolboxSpec(spec cocoonv1.ToolboxSpec, vmName string, snapshotPolicy co
 	}
 }
 
-// ShouldSnapshotVM reports whether the VM should be snapshotted based on its SnapshotPolicy.
-func ShouldSnapshotVM(spec VMSpec) bool {
+// ShouldSnapshotVM reports whether the VM should be snapshotted based on its SnapshotPolicy and role.
+func ShouldSnapshotVM(spec VMSpec, role string) bool {
 	switch cocoonv1.SnapshotPolicy(spec.SnapshotPolicy).Default() {
 	case cocoonv1.SnapshotPolicyNever:
 		return false
 	case cocoonv1.SnapshotPolicyMainOnly:
-		return ExtractSlotFromVMName(spec.VMName) == 0
+		return role == RoleMain
 	default:
 		return true
 	}

meta/vmspec_test.go

@@ -218,28 +218,30 @@ func TestShouldSnapshotVM(t *testing.T) {
 	cases := []struct {
 		name   string
 		policy cocoonv1.SnapshotPolicy
-		vmName string
+		role   string
 		want   bool
 	}{
-		{"always/slot0", cocoonv1.SnapshotPolicyAlways, "vk-prod-demo-0", true},
-		{"always/slot3", cocoonv1.SnapshotPolicyAlways, "vk-prod-demo-3", true},
-		{"always/toolbox", cocoonv1.SnapshotPolicyAlways, "vk-prod-my-tb", true},
-		{"empty-defaults-to-always", "", "vk-prod-demo-0", true},
-		{"empty-defaults-to-always/sub", "", "vk-prod-demo-2", true},
-
-		{"never/slot0", cocoonv1.SnapshotPolicyNever, "vk-prod-demo-0", false},
-		{"never/slot3", cocoonv1.SnapshotPolicyNever, "vk-prod-demo-3", false},
-		{"never/toolbox", cocoonv1.SnapshotPolicyNever, "vk-prod-my-tb", false},
-
-		{"main-only/slot0", cocoonv1.SnapshotPolicyMainOnly, "vk-prod-demo-0", true},
-		{"main-only/slot3", cocoonv1.SnapshotPolicyMainOnly, "vk-prod-demo-3", false},
-		{"main-only/toolbox", cocoonv1.SnapshotPolicyMainOnly, "vk-prod-my-tb", false},
+		{"always/main", cocoonv1.SnapshotPolicyAlways, RoleMain, true},
+		{"always/sub-agent", cocoonv1.SnapshotPolicyAlways, RoleSubAgent, true},
+		{"always/toolbox", cocoonv1.SnapshotPolicyAlways, RoleToolbox, true},
+		{"always/empty-role", cocoonv1.SnapshotPolicyAlways, "", true},
+		{"empty-policy-defaults-to-always", "", RoleToolbox, true},
+
+		{"never/main", cocoonv1.SnapshotPolicyNever, RoleMain, false},
+		{"never/sub-agent", cocoonv1.SnapshotPolicyNever, RoleSubAgent, false},
+		{"never/toolbox", cocoonv1.SnapshotPolicyNever, RoleToolbox, false},
+		{"never/empty-role", cocoonv1.SnapshotPolicyNever, "", false},
+
+		{"main-only/main", cocoonv1.SnapshotPolicyMainOnly, RoleMain, true},
+		{"main-only/sub-agent", cocoonv1.SnapshotPolicyMainOnly, RoleSubAgent, false},
+		{"main-only/toolbox", cocoonv1.SnapshotPolicyMainOnly, RoleToolbox, false},
+		{"main-only/empty-role", cocoonv1.SnapshotPolicyMainOnly, "", false},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			spec := VMSpec{VMName: c.vmName, SnapshotPolicy: string(c.policy)}
-			if got := ShouldSnapshotVM(spec); got != c.want {
-				t.Errorf("ShouldSnapshotVM(%s, %q) = %v, want %v", c.policy, c.vmName, got, c.want)
+			spec := VMSpec{SnapshotPolicy: string(c.policy)}
+			if got := ShouldSnapshotVM(spec, c.role); got != c.want {
+				t.Errorf("ShouldSnapshotVM(%s, %q) = %v, want %v", c.policy, c.role, got, c.want)
 			}
 		})
 	}