fix: avoid cross-patching worker pool agents

Author: rxbn

Makefile

@@ -3,7 +3,7 @@
 # To re-generate a bundle for another specific version without changing the standard setup, you can:
 # - use the VERSION as arg of the bundle target (e.g make bundle VERSION=0.0.3)
 # - use environment variables to overwrite this value (e.g export VERSION=0.0.3)
-VERSION ?= 0.0.22
+VERSION ?= 0.0.23
 
 # CHANNELS define the bundle channels used in the bundle.
 # Add a new line here if you would like to change its default config. (E.g CHANNELS = "candidate,fast,stable")

config/default/kustomization.yaml

@@ -36,7 +36,7 @@ resources:
 images:
   - name: controller
     newName: ghcr.io/containeroo/agent-forge-operator
-    newTag: v0.0.22
+    newTag: v0.0.23
 
 # Uncomment the patches line if you enable Metrics
 patches:

internal/controller/planner.go

@@ -71,6 +71,7 @@ type AgentInfo struct {
 	Approved          bool
 	SpecRole          string
 	RoleLabel         string
+	PoolLabel         string
 	Hostname          string
 	InventoryHostname string
 	MAC               string
@@ -138,7 +139,10 @@ func buildPlan(pool *agentforgev1alpha1.VsphereAgentPool, snapshot PoolSnapshot)
 		if agent.Bound {
 			continue
 		}
-		if agent.Approved && agent.SpecRole == pool.Spec.Agent.Role && agent.RoleLabel == pool.Spec.Agent.Role && agent.Hostname != "" {
+		if !agentNeedsPatch(pool, agent) {
+			continue
+		}
+		if !agentPatchEligible(pool, snapshot, agent) {
 			continue
 		}
 		agentsToPatch = append(agentsToPatch, agent)
@@ -215,6 +219,39 @@ func buildPlan(pool *agentforgev1alpha1.VsphereAgentPool, snapshot PoolSnapshot)
 	}
 }
 
+func agentNeedsPatch(pool *agentforgev1alpha1.VsphereAgentPool, agent AgentInfo) bool {
+	if desiredPoolLabel, hasPoolLabel := pool.Spec.Agent.Labels[poolLabelKey]; hasPoolLabel && agent.PoolLabel != desiredPoolLabel {
+		return true
+	}
+	return !agent.Approved || agent.SpecRole != pool.Spec.Agent.Role || agent.RoleLabel != pool.Spec.Agent.Role || agent.Hostname == ""
+}
+
+func agentPatchEligible(pool *agentforgev1alpha1.VsphereAgentPool, snapshot PoolSnapshot, agent AgentInfo) bool {
+	if agentAssociatedWithOwnedVM(snapshot.OwnedVMs, agent) {
+		return true
+	}
+	if desiredPoolLabel, hasPoolLabel := pool.Spec.Agent.Labels[poolLabelKey]; hasPoolLabel {
+		return agent.PoolLabel == desiredPoolLabel
+	}
+	return snapshot.WaitingAgentMachines > 0 || snapshot.AgentMachinesWithoutAgent > 0
+}
+
+func agentAssociatedWithOwnedVM(vms []agentforgev1alpha1.OwnedVMStatus, agent AgentInfo) bool {
+	hostname := agentObservedHostname(agent)
+	for _, vm := range vms {
+		if vm.Name == "" {
+			continue
+		}
+		if vmMatchesAgentIdentity(vm, agent) || vmMatchesAgentRef(vm, agent) {
+			return true
+		}
+		if hostname != "" && vm.Name == hostname && !vmIdentityConflictsAgent(vm, agent) {
+			return true
+		}
+	}
+	return false
+}
+
 func countPendingOwnedVMs(vms []agentforgev1alpha1.OwnedVMStatus) int32 {
 	var count int32
 	for _, vm := range vms {

internal/controller/planner_test.go

@@ -119,6 +119,42 @@ func TestBuildPlanPatchesUnapprovedAgents(t *testing.T) {
 	}
 }
 
+func TestBuildPlanDoesNotPatchUnownedAgentsWithoutDemand(t *testing.T) {
+	pool := testPool()
+
+	plan := buildPlan(pool, PoolSnapshot{
+		WaitingAgentMachines: 0,
+		MatchingAgents: []AgentInfo{
+			{Name: "agent-1", Bound: false, Approved: false, RoleLabel: ""},
+		},
+	})
+
+	if len(plan.AgentsToPatch) != 0 {
+		t.Fatalf("AgentsToPatch = %#v, want none for unowned Agent without demand", plan.AgentsToPatch)
+	}
+	if len(plan.Actions) != 1 || plan.Actions[0].Type != actionNoop {
+		t.Fatalf("actions = %#v, want one Noop", plan.Actions)
+	}
+}
+
+func TestBuildPlanPatchesAgentAssociatedWithOwnedVMWithoutDemand(t *testing.T) {
+	pool := testPool()
+
+	plan := buildPlan(pool, PoolSnapshot{
+		WaitingAgentMachines: 0,
+		MatchingAgents: []AgentInfo{
+			{Name: "agent-1", Bound: false, Approved: false, RoleLabel: "", MAC: "00-50-56-b2-a1-9f"},
+		},
+		OwnedVMs: []agentforgev1alpha1.OwnedVMStatus{
+			{Name: "owned-vm", Phase: phaseProvisioning, MACAddress: "00-50-56-b2-a1-9f"},
+		},
+	})
+
+	if len(plan.AgentsToPatch) != 1 || plan.AgentsToPatch[0].Name != "agent-1" {
+		t.Fatalf("AgentsToPatch = %#v, want owned Agent patched", plan.AgentsToPatch)
+	}
+}
+
 func TestBuildPlanDeletesOrphanedOwnedVMsWithoutExcessAgents(t *testing.T) {
 	pool := testPool()
 

internal/controller/vsphereagentpool_controller.go

@@ -760,6 +760,7 @@ func (r *VsphereAgentPoolReconciler) listMatchingAgents(ctx context.Context, poo
 			Approved:          approved,
 			SpecRole:          specRole,
 			RoleLabel:         labels[roleLabelKey],
+			PoolLabel:         labels[poolLabelKey],
 			Hostname:          specHostname,
 			InventoryHostname: inventoryHostname,
 			MAC:               normalizeMAC(agentPrimaryMAC(obj)),
@@ -834,17 +835,6 @@ func assignedAgentHostnames(pool *agentforgev1alpha1.VsphereAgentPool, agents []
 	assigned := map[string]string{}
 	reserved := map[string]struct{}{}
 	for _, agent := range agents {
-		if agent.Hostname == "" {
-			continue
-		}
-		assigned[agent.Name] = agent.Hostname
-		reserved[agent.Hostname] = struct{}{}
-	}
-
-	for _, agent := range agents {
-		if assigned[agent.Name] != "" {
-			continue
-		}
 		for _, vm := range pool.Status.OwnedVMs {
 			if vm.Name == "" || vm.Phase == phaseBound {
 				continue
@@ -859,6 +849,17 @@ func assignedAgentHostnames(pool *agentforgev1alpha1.VsphereAgentPool, agents []
 			reserved[vm.Name] = struct{}{}
 			break
 		}
+	}
+
+	for _, agent := range agents {
+		if assigned[agent.Name] != "" || agent.Hostname == "" {
+			continue
+		}
+		assigned[agent.Name] = agent.Hostname
+		reserved[agent.Hostname] = struct{}{}
+	}
+
+	for _, agent := range agents {
 		if assigned[agent.Name] != "" {
 			continue
 		}

internal/controller/vsphereagentpool_controller_test.go

@@ -1450,11 +1450,16 @@ func TestReconcilePatchesCandidateAgentWithPoolDiscriminatorLabel(t *testing.T)
 	pool := reconcileTestPool()
 	pool.Spec.Agent.Labels[poolLabelKey] = "worker-32c128g"
 	pool.Status.OwnedVMs = []agentforgev1alpha1.OwnedVMStatus{
-		newOwnedVMStatus("demo-worker-32c128g-ab12"),
+		{
+			Name:       "demo-worker-32c128g-ab12",
+			Phase:      phaseProvisioning,
+			MACAddress: "00-50-56-b2-a1-9f",
+		},
 	}
 	am := testAgentMachine(testControlPlaneNamespace, testNodePool, "demo/demo-worker")
 	infraEnv := testInfraEnv(testNamespace, testInfraEnvName, "https://example.invalid/discovery.iso")
 	agent := testCandidateAgent(testNamespace, "abcdef12-3456-7890-abcd-ef1234567890")
+	setAgentPrimaryMAC(t, agent, "00:50:56:b2:a1:9f")
 
 	k8sClient := fake.NewClientBuilder().
 		WithScheme(scheme).
@@ -1726,6 +1731,7 @@ func TestAssignedAgentHostnamesUsesVMIdentityBeforeFreeSlot(t *testing.T) {
 
 	hostnames := assignedAgentHostnames(pool, []AgentInfo{{
 		Name:     "agent-1",
+		Hostname: "demo-worker-stale",
 		BIOSUUID: "22222222-2222-2222-2222-222222222222",
 	}})
 
@@ -2475,6 +2481,13 @@ func setAgentInventoryHostname(t *testing.T, agent *unstructured.Unstructured, h
 	}
 }
 
+func setAgentPrimaryMAC(t *testing.T, agent *unstructured.Unstructured, mac string) {
+	t.Helper()
+	if err := unstructured.SetNestedSlice(agent.Object, []any{map[string]any{"macAddress": mac}}, "status", "inventory", "interfaces"); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func findCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition {
 	for i := range conditions {
 		if conditions[i].Type == conditionType {