From 618f93966230b7aa0c4fbea691398c6acb7dfcfe Mon Sep 17 00:00:00 2001 From: Sid Shukla Date: Tue, 30 Jul 2024 20:31:21 +0200 Subject: [PATCH] address review comments --- controllers/nutanixmachine_controller.go | 61 +++++++++++++----------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/controllers/nutanixmachine_controller.go b/controllers/nutanixmachine_controller.go index af9e4021f1..ce56c748a5 100644 --- a/controllers/nutanixmachine_controller.go +++ b/controllers/nutanixmachine_controller.go @@ -314,26 +314,23 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r // Check if VMUUID is absent if vmUUID == "" { - log.Info(fmt.Sprintf("VMUUID was not found in spec for VM %s. Skipping delete", vmName)) + log.Info(fmt.Sprintf("VM UUID was not found in spec for VM %s. Skipping delete", vmName)) log.Info(fmt.Sprintf("Removing finalizers for VM %s during delete reconciliation", vmName)) ctrlutil.RemoveFinalizer(rctx.NutanixMachine, infrav1.NutanixMachineFinalizer) return reconcile.Result{}, nil } - // Search for VM by UUID vm, err := FindVMByUUID(ctx, v3Client, vmUUID) - // Error while finding VM if err != nil { - errorMsg := fmt.Errorf("error finding vm %s with uuid %s: %v", vmName, vmUUID, err) - log.Error(errorMsg, "error finding vm") + errorMsg := fmt.Errorf("error finding VM %s with UUID %s: %v", vmName, vmUUID, err) + log.Error(errorMsg, "error finding VM") conditions.MarkFalse(rctx.NutanixMachine, infrav1.VMProvisionedCondition, infrav1.DeletionFailed, capiv1.ConditionSeverityWarning, errorMsg.Error()) return reconcile.Result{}, errorMsg } - // Vm not found if vm == nil { - log.Info(fmt.Sprintf("no vm found with UUID %s ... Already deleted? Skipping delete", vmUUID)) - log.Info(fmt.Sprintf("Removing finalizers for VM %s during delete reconciliation", vmName)) + log.Info(fmt.Sprintf("no VM found with UUID %s: assuming it is already deleted; skipping delete", vmUUID)) + log.Info(fmt.Sprintf("removing finalizers for VM %s during delete reconciliation", vmName)) ctrlutil.RemoveFinalizer(rctx.NutanixMachine, infrav1.NutanixMachineFinalizer) return reconcile.Result{}, nil } @@ -350,7 +347,7 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r log.V(1).Info(fmt.Sprintf("VM %s with UUID %s was found.", *vm.Spec.Name, vmUUID)) lastTaskUUID, err := GetTaskUUIDFromVM(vm) if err != nil { - errorMsg := fmt.Errorf("error occurred fetching task UUID from vm: %v", err) + errorMsg := fmt.Errorf("error occurred fetching task UUID from VM: %v", err) log.Error(errorMsg, "error fetching task UUID") conditions.MarkFalse(rctx.NutanixMachine, infrav1.VMProvisionedCondition, infrav1.DeletionFailed, capiv1.ConditionSeverityWarning, errorMsg.Error()) return reconcile.Result{}, errorMsg @@ -366,7 +363,7 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r log.Info(fmt.Sprintf("VM %s task with UUID %s still in progress. Requeuing", vmName, vmUUID)) return reconcile.Result{RequeueAfter: 5 * time.Second}, nil } - log.V(1).Info(fmt.Sprintf("No running tasks anymore... Initiating delete for vm %s with UUID %s", vmName, vmUUID)) + log.V(1).Info(fmt.Sprintf("no running tasks anymore... Initiating delete for VM %s with UUID %s", vmName, vmUUID)) } else { log.V(1).Info(fmt.Sprintf("no task UUID found on VM %s. Starting delete.", *vm.Spec.Name)) } @@ -382,25 +379,8 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r } if vgDetachNeeded { - createV4Client, err := isPrismCentralV4Compatible(ctx, v3Client) - if err != nil { - log.Info("error occurred while checking compatibility for Prism Central v4 APIs", "error", err.Error()) - } - - if createV4Client { - log.Info("Creating Prism Central v4 client for cluster", "cluster", rctx.NutanixCluster.Name) - v4Client, err := getPrismCentralV4ClientForCluster(ctx, rctx.NutanixCluster, r.SecretInformer, r.ConfigMapInformer) - if err != nil { - log.Error(err, "error occurred while fetching Prism Central v4 client") - return reconcile.Result{}, err - } - - if err := detachVolumeGroupsFromVM(ctx, v4Client, vmUUID); err != nil { - errorMsg := fmt.Errorf("failed to detach volume groups from VM %s with UUID %s: %v", vmName, vmUUID, err) - conditions.MarkFalse(rctx.NutanixMachine, infrav1.VMProvisionedCondition, infrav1.VolumeGroupDetachFailed, capiv1.ConditionSeverityWarning, errorMsg.Error()) - log.Error(errorMsg, "failed to detach volume groups", "cluster", rctx.NutanixCluster.Name) - return reconcile.Result{}, err - } + if err := r.detachVolumeGroups(rctx, vmUUID); err != nil { + return reconcile.Result{}, err } } @@ -416,6 +396,29 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r return reconcile.Result{RequeueAfter: 5 * time.Second}, nil } +func (r *NutanixMachineReconciler) detachVolumeGroups(rctx *nctx.MachineContext, vmUUID string) error { + createV4Client, err := isPrismCentralV4Compatible(rctx.Context, rctx.NutanixClient) + if err != nil { + err := fmt.Errorf("error occurred while checking compatibility for Prism Central v4 APIs: %w", err) + return err + } + + if createV4Client { + v4Client, err := getPrismCentralV4ClientForCluster(rctx.Context, rctx.NutanixCluster, r.SecretInformer, r.ConfigMapInformer) + if err != nil { + err := fmt.Errorf("error occurred while fetching Prism Central v4 client: %w", err) + return err + } + + if err := detachVolumeGroupsFromVM(rctx.Context, v4Client, vmUUID); err != nil { + err := fmt.Errorf("failed to detach volume groups from VM %s with UUID %s: %w", rctx.Machine.Name, vmUUID, err) + return err + } + } + + return nil +} + func (r *NutanixMachineReconciler) reconcileNormal(rctx *nctx.MachineContext) (reconcile.Result, error) { log := ctrl.LoggerFrom(rctx.Context) if rctx.NutanixMachine.Status.FailureReason != nil || rctx.NutanixMachine.Status.FailureMessage != nil {