add comments and small code refactor per PR comments

Author: wanyufe

controllers/cloudstackmachine_controller.go

@@ -223,11 +223,14 @@ func (r *CloudStackMachineReconciliationRunner) GetOrCreateVMInstance() (retRes
 		r.Recorder.Eventf(r.ReconciliationSubject, "Warning", "Creating", CSMachineCreationFailed, err.Error())
 	}
 	if err == nil && !controllerutil.ContainsFinalizer(r.ReconciliationSubject, infrav1.MachineFinalizer) { // Fetched or Created?
+		// adding a finalizer will require CPAC reconcile-delete trying to destroy associated VM through instanceID,
+		// so if err is not nil, which means it could not get associated VM through instanceID or name, we should not add finalizer to CloudStackMachine,
+		// otherwise, reconciler-delete will be stuck trying to wait instanceID to ba available.
 		controllerutil.AddFinalizer(r.ReconciliationSubject, infrav1.MachineFinalizer)
 		r.Recorder.Eventf(r.ReconciliationSubject, "Normal", "Created", CSMachineCreationSuccess)
 		r.Log.Info(CSMachineCreationSuccess, "instanceStatus", r.ReconciliationSubject.Status)
 	}
-	
+
 	return ctrl.Result{}, err
 }
 

controllers/cloudstackmachine_controller_test.go

@@ -72,7 +72,7 @@ var _ = Describe("CloudStackMachineReconciler", func() {
 				key := client.ObjectKey{Namespace: dummies.ClusterNameSpace, Name: dummies.CSMachine1.Name}
 				if err := k8sClient.Get(ctx, key, tempMachine); err == nil {
 					if tempMachine.Status.Ready == true {
-						return true
+						return len(tempMachine.ObjectMeta.Finalizers) > 0
 					}
 				}
 				return false

pkg/cloud/instance.go

@@ -275,13 +275,13 @@ func (c *client) GetOrCreateVMInstance(
 		listVirtualMachineParams.SetZoneid(fd.Spec.Zone.ID)
 		listVirtualMachineParams.SetNetworkid(fd.Spec.Zone.Network.ID)
 		listVirtualMachineParams.SetName(csMachine.Name)
-		if listVirtualMachinesResponse, err2 := c.cs.VirtualMachine.ListVirtualMachines(listVirtualMachineParams); err2 == nil && listVirtualMachinesResponse.Count > 0 {
-			csMachine.Spec.InstanceID = pointer.StringPtr(listVirtualMachinesResponse.VirtualMachines[0].Id)
-			csMachine.Status.InstanceState = listVirtualMachinesResponse.VirtualMachines[0].State
-		} else {
+		listVirtualMachinesResponse, err2 := c.cs.VirtualMachine.ListVirtualMachines(listVirtualMachineParams)
+		if err2 != nil || listVirtualMachinesResponse.Count <= 0 {
 			c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(err2)
 			return err
 		}
+		csMachine.Spec.InstanceID = pointer.StringPtr(listVirtualMachinesResponse.VirtualMachines[0].Id)
+		csMachine.Status.InstanceState = listVirtualMachinesResponse.VirtualMachines[0].State
 	} else {
 		csMachine.Spec.InstanceID = pointer.StringPtr(deployVMResp.Id)
 		csMachine.Status.Status = pointer.String(metav1.StatusSuccess)