OCPBUGS-32592: Ensure Agent isn't unbound if paused (#140)

Originally, the condition to not unbind the Agent
was based on if the AgentMachine is paused AND if the
AgentMachine had a deletionTimestamp. This caused
the Agent to be unbound when the deletionTimestamp
didn't exist yet on the AgentMachine, and is flawed
logic.
This change updates the condition to make sure we
don't continue to unbind the Agent if the AgentMachine
is at all paused.

Author: CrystalChun

controllers/agentmachine_controller.go

@@ -208,7 +208,11 @@ func (r *AgentMachineReconciler) handleDeletionHook(ctx context.Context, log log
 	}
 
 	// Skip agent unbind if AgentMachine is paused
-	if paused := agentMachine.Annotations[clusterv1.PausedAnnotation]; !agentMachine.ObjectMeta.DeletionTimestamp.IsZero() && paused == "true" {
+	if paused := agentMachine.Annotations[clusterv1.PausedAnnotation]; paused == "true" {
+		if agentMachine.ObjectMeta.DeletionTimestamp.IsZero() {
+			log.Info("AgentMachine paused, but not being deleted yet")
+			return nil, nil
+		}
 		log.Info("Skipping unbinding agent since agent machine is paused. Removing machine delete hook annotation")
 		if err := r.removeHookAndFinalizer(ctx, machine, agentMachine); err != nil {
 			log.Error(err)

controllers/agentmachine_controller_test.go

@@ -646,6 +646,26 @@ var _ = Describe("agentmachine reconcile", func() {
 			Expect(agent.Spec.ClusterDeploymentName).NotTo(BeNil())
 			Expect(agent.Spec.ClusterDeploymentName.Name).To(BeEquivalentTo("cluster-deployment-agentMachine-1"))
 		})
+
+		It("doesn't unbind the agent if the AgentMachine doesn't have a deletion timestamp", func() {
+			agentMachine.ObjectMeta.Annotations = map[string]string{clusterv1.PausedAnnotation: "true"}
+			controllerutil.AddFinalizer(agentMachine, AgentMachineFinalizerName)
+			Expect(c.Update(ctx, agentMachine)).To(Succeed())
+
+			// mark the machine for deletion
+			Expect(c.Get(ctx, types.NamespacedName{Namespace: machine.Namespace, Name: machine.Name}, machine)).To(BeNil())
+			conditions.MarkFalse(machine, clusterv1.PreTerminateDeleteHookSucceededCondition, clusterv1.WaitingExternalHookReason, clusterv1.ConditionSeverityInfo, "")
+			machine.Annotations = map[string]string{machineDeleteHookName: ""}
+			Expect(c.Update(ctx, machine)).To(BeNil())
+
+			result, err := amr.Reconcile(ctx, newAgentMachineRequest(agentMachine))
+			Expect(err).To(BeNil())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			Expect(c.Get(ctx, types.NamespacedName{Name: "agent-1", Namespace: testNamespace}, agent)).To(Succeed())
+			Expect(agent.Spec.ClusterDeploymentName).NotTo(BeNil())
+			Expect(agent.Spec.ClusterDeploymentName.Name).To(BeEquivalentTo("cluster-deployment-agentMachine-1"))
+		})
 	})
 })