Merge pull request #936 from yastij/automated-cherry-pick-of-#926-upstream-release-0.6

Automated cherry pick of #926: Fix race condition that leads to dropping the biosuuid

Author: k8s-ci-robot

controllers/haproxyloadbalancer_controller.go

@@ -555,6 +555,19 @@ func (r haproxylbReconciler) reconcileVMPre7(ctx *context.HAProxyLoadBalancerCon
 		// Copy the HAProxyLoadBalancer's VM clone spec into the VSphereVM's
 		// clone spec.
 		ctx.HAProxyLoadBalancer.Spec.VirtualMachineConfiguration.DeepCopyInto(&vm.Spec.VirtualMachineCloneSpec)
+
+		objectKey, err := ctrlclient.ObjectKeyFromObject(vm)
+		if err != nil {
+			return err
+		}
+		existingVM := &infrav1.VSphereVM{}
+		if err := r.Client.Get(ctx, objectKey, existingVM); err != nil {
+			if apierrors.IsNotFound(err) {
+				return nil
+			}
+			return err
+		}
+		vm.Spec.BiosUUID = existingVM.Spec.BiosUUID
 		return nil
 	}
 	if _, err := ctrlutil.CreateOrUpdate(ctx, ctx.Client, vm, mutateFn); err != nil {

controllers/vspheremachine_controller.go

@@ -315,7 +315,7 @@ func (r machineReconciler) reconcileNormal(ctx *context.MachineContext) (reconci
 	}
 
 	// TODO(akutz) Determine the version of vSphere.
-	vm, err := r.reconcileNormalPre7(ctx)
+	vm, err := r.reconcileNormalPre7(ctx, vsphereVM)
 	if err != nil {
 		if apierrors.IsAlreadyExists(err) {
 			return reconcile.Result{}, nil
@@ -368,7 +368,7 @@ func (r machineReconciler) reconcileNormal(ctx *context.MachineContext) (reconci
 	return reconcile.Result{}, nil
 }
 
-func (r machineReconciler) reconcileNormalPre7(ctx *context.MachineContext) (runtime.Object, error) {
+func (r machineReconciler) reconcileNormalPre7(ctx *context.MachineContext, vsphereVM *infrav1.VSphereVM) (runtime.Object, error) {
 	// Create or update the VSphereVM resource.
 	vm := &infrav1.VSphereVM{
 		ObjectMeta: metav1.ObjectMeta{
@@ -433,6 +433,9 @@ func (r machineReconciler) reconcileNormalPre7(ctx *context.MachineContext) (run
 		if vm.Spec.ResourcePool == "" {
 			vm.Spec.ResourcePool = vsphereCloudConfig.ResourcePool
 		}
+		if vsphereVM != nil {
+			vm.Spec.BiosUUID = vsphereVM.Spec.BiosUUID
+		}
 		return nil
 	}
 	if _, err := ctrlutil.CreateOrUpdate(ctx, ctx.Client, vm, mutateFn); err != nil {

controllers/vspherevm_controller.go

@@ -222,7 +222,6 @@ func (r vmReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr error)
 				vmContext.Logger.Error(err, "failed to get VSphereVM while exiting reconcile")
 				return false, nil
 			}
-
 			// If the remote resource version is not the same as the local
 			// resource version, then it means we were able to get a resource
 			// newer than the one we already had.
@@ -324,7 +323,21 @@ func (r vmReconciler) reconcileNormal(ctx *context.VMContext) (reconcile.Result,
 	}
 
 	// Update the VSphereVM's BIOS UUID.
-	ctx.VSphereVM.Spec.BiosUUID = vm.BiosUUID
+	ctx.Logger.Info("vm bios-uuid", "biosuuid", vm.BiosUUID)
+
+	// defensive check to ensure we are not removing the biosUUID
+	if vm.BiosUUID != "" {
+		ctx.VSphereVM.Spec.BiosUUID = vm.BiosUUID
+	} else {
+		return reconcile.Result{}, errors.Errorf("bios uuid is empty while VM is ready")
+	}
+
+	// patch the vsphereVM early to ensure that the task is
+	// reflected in the status right away, this avoid situations
+	// of concurrent clones
+	if err := ctx.Patch(); err != nil {
+		ctx.Logger.Error(err, "patch failed", "vspherevm", ctx.VSphereVM)
+	}
 
 	// Update the VSphereVM's network status.
 	r.reconcileNetwork(ctx, vm)

pkg/services/govmomi/errors.go

@@ -16,17 +16,21 @@ limitations under the License.
 
 package govmomi
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/vmware/govmomi/find"
+)
 
 // errNotFound is returned by the findVM function when a VM is not found.
 type errNotFound struct {
-	instanceUUID bool
-	uuid         string
+	uuid            string
+	byInventoryPath string
 }
 
 func (e errNotFound) Error() string {
-	if e.instanceUUID {
-		return fmt.Sprintf("vm with instance uuid %s not found", e.uuid)
+	if e.byInventoryPath != "" {
+		return fmt.Sprintf("vm with inventory path %s not found", e.byInventoryPath)
 	}
 	return fmt.Sprintf("vm with bios uuid %s not found", e.uuid)
 }
@@ -40,10 +44,19 @@ func isNotFound(err error) bool {
 	}
 }
 
+func isVirtualMachineNotFound(err error) bool {
+	switch err.(type) {
+	case *find.NotFoundError:
+		return true
+	default:
+		return false
+	}
+}
+
 func wasNotFoundByBIOSUUID(err error) bool {
 	switch err.(type) {
 	case errNotFound, *errNotFound:
-		if err.(errNotFound).uuid != "" && !err.(errNotFound).instanceUUID {
+		if err.(errNotFound).uuid != "" && err.(errNotFound).byInventoryPath == "" {
 			return true
 		}
 		return false

pkg/services/govmomi/util.go

@@ -18,6 +18,7 @@ package govmomi
 
 import (
 	gonet "net"
+	"path"
 
 	"github.com/pkg/errors"
 	"github.com/vmware/govmomi/object"
@@ -49,15 +50,19 @@ func sanitizeIPAddrs(ctx *context.VMContext, ipAddrs []string) []string {
 //   1. If the BIOS UUID is available, then it is used to find the VM.
 //   2. Lacking the BIOS UUID, the VM is queried by its instance UUID,
 //      which was assigned the value of the VSphereVM resource's UID string.
+//   3. If it is not found by instance UUID, fallback to an inventory path search
+//      using the vm folder path and the VSphereVM name
 func findVM(ctx *context.VMContext) (types.ManagedObjectReference, error) {
 	if biosUUID := ctx.VSphereVM.Spec.BiosUUID; biosUUID != "" {
 		objRef, err := ctx.Session.FindByBIOSUUID(ctx, biosUUID)
 		if err != nil {
 			return types.ManagedObjectReference{}, err
 		}
 		if objRef == nil {
+			ctx.Logger.Info("vm not found by bios uuid", "biosuuid", biosUUID)
 			return types.ManagedObjectReference{}, errNotFound{uuid: biosUUID}
 		}
+		ctx.Logger.Info("vm found by bios uuid", "vmref", objRef.Reference())
 		return objRef.Reference(), nil
 	}
 
@@ -67,8 +72,24 @@ func findVM(ctx *context.VMContext) (types.ManagedObjectReference, error) {
 		return types.ManagedObjectReference{}, err
 	}
 	if objRef == nil {
-		return types.ManagedObjectReference{}, errNotFound{instanceUUID: true, uuid: instanceUUID}
+		// fallback to use inventory paths
+		folder, err := ctx.Session.Finder.FolderOrDefault(ctx, ctx.VSphereVM.Spec.Folder)
+		if err != nil {
+			return types.ManagedObjectReference{}, errors.Wrapf(err, "unable to get folder for %s/%s", ctx.VSphereVM.Namespace, ctx.VSphereVM.Name)
+		}
+		inventoryPath := path.Join(folder.InventoryPath, ctx.VSphereVM.Name)
+		ctx.Logger.Info("using inventory path to find vm", "path", inventoryPath)
+		vm, err := ctx.Session.Finder.VirtualMachine(ctx, inventoryPath)
+		if err != nil {
+			if isVirtualMachineNotFound(err) {
+				return types.ManagedObjectReference{}, errNotFound{byInventoryPath: inventoryPath}
+			}
+			return types.ManagedObjectReference{}, err
+		}
+		ctx.Logger.Info("vm found by name", "vmref", vm.Reference())
+		return vm.Reference(), nil
 	}
+	ctx.Logger.Info("vm found by instance uuid", "vmref", objRef.Reference())
 	return objRef.Reference(), nil
 }
 

pkg/services/govmomi/vcenter/clone.go

@@ -182,12 +182,6 @@ func Clone(ctx *context.VMContext, bootstrapData []byte) error {
 
 	ctx.VSphereVM.Status.TaskRef = task.Reference().Value
 
-	// patch the vsphereVM here to ensure that the task is
-	// reflected in the status right away, this avoid situations
-	// of concurrent clones
-	if err := ctx.Patch(); err != nil {
-		ctx.Logger.Error(err, "patch failed", "vspherevm", ctx.VSphereVM)
-	}
 	return nil
 }