Skip to content

Commit

Permalink
Remove Inventory path fallback for GetVM
Browse files Browse the repository at this point in the history 
Originally, we'd try to find a VM via its MoID, with a fallback to the
Inventory by path. Since then, we've added the Instance and Bios UUIDs
as preferred ways to find a VM. VMs are now created with a known
Instance ID - that of the k8s VM UID - that we can use for lookup.

This removes lookup depending on k8s objects to determine if the VM
exists that have a separate lifecycle than the VM.
  • Loading branch information
Bryan Venteicher committed Dec 10, 2024
1 parent c5fecaf commit 65fe835
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 165 deletions.
80 changes: 3 additions & 77 deletions pkg/providers/vsphere/vcenter/getvm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,18 @@ import (
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/vim25"
vimtypes "github.com/vmware/govmomi/vim25/types"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"

vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
"github.com/vmware-tanzu/vm-operator/pkg/topology"
)

// GetVirtualMachine gets the VM from VC, either by the MoID, UUID, or the inventory path.
// GetVirtualMachine gets the VM from VC, either by the Instance UUID, BIOS UUID, or MoID.
func GetVirtualMachine(
vmCtx pkgctx.VirtualMachineContext,
k8sClient ctrlclient.Client,
vimClient *vim25.Client,
datacenter *object.Datacenter,
finder *find.Finder) (*object.VirtualMachine, error) {

// Find by instance UUID.
// Find by Instance UUID.
if id := vmCtx.VM.UID; id != "" {
if vm, err := findVMByUUID(vmCtx, vimClient, datacenter, string(id), true); err == nil {
return vm, nil
Expand All @@ -46,8 +42,7 @@ func GetVirtualMachine(
}
}

// Find by inventory path.
return findVMByInventory(vmCtx, k8sClient, vimClient, finder)
return nil, nil
}

func findVMByMoID(
Expand Down Expand Up @@ -91,72 +86,3 @@ func findVMByUUID(
vmCtx.Logger.V(4).Info("Found VM via UUID", "uuid", uuid, "isInstanceUUID", isInstanceUUID)
return vm, nil
}

func findVMByInventory(
vmCtx pkgctx.VirtualMachineContext,
k8sClient ctrlclient.Client,
vimClient *vim25.Client,
finder *find.Finder) (*object.VirtualMachine, error) {

// Note that we'll usually only get here to find the VM via its inventory path when we're first
// creating the VM. To determine the path, we need the NS Folder MoID and the VM's ResourcePolicy,
// if set, and we'll fetch these again as a part of createVirtualMachine(). For now, just re-fetch
// but we could pass the Folder MoID and ResourcePolicy to save a bit of duplicated work.

folderMoID, err := topology.GetNamespaceFolderMoID(vmCtx, k8sClient, vmCtx.VM.Namespace)
if err != nil {
return nil, err
}

// While we strictly only need the Folder's ManagedObjectReference below, use the Finder
// here to check if it actually exists.
folder, err := GetFolderByMoID(vmCtx, finder, folderMoID)
if err != nil {
return nil, fmt.Errorf("failed to get namespace Folder: %w", err)
}

// When the VM has a ResourcePolicy, the VM is placed in a child folder under the namespace's folder.
var policyName string
if reserved := vmCtx.VM.Spec.Reserved; reserved != nil {
policyName = reserved.ResourcePolicyName
}
if policyName != "" {
resourcePolicy := &vmopv1.VirtualMachineSetResourcePolicy{}

key := ctrlclient.ObjectKey{Name: policyName, Namespace: vmCtx.VM.Namespace}
if err := k8sClient.Get(vmCtx, key, resourcePolicy); err != nil {
// Note that if VM does not exist, and we're about to create it, the ResourcePolicy is Get()
// again so the corresponding condition is almost always true if we don't hit an error here.
// Creating the VM with an explicit InstanceUUID is the easiest way out to avoid this.
return nil, fmt.Errorf("failed to get VirtualMachineSetResourcePolicy: %w", err)
}

if folderName := resourcePolicy.Spec.Folder; folderName != "" {
childFolder, err := GetChildFolder(vmCtx, folder, folderName)
if err != nil {
vmCtx.Logger.Error(err, "Failed to get VirtualMachineSetResourcePolicy child Folder",
"parentPath", folder.InventoryPath, "folderName", folderName, "policyName", policyName)
return nil, err
}

folder = childFolder
}
}

ref, err := object.NewSearchIndex(vimClient).FindChild(vmCtx, folder.Reference(), vmCtx.VM.Name)
if err != nil {
return nil, err
} else if ref == nil {
// VM does not exist.
return nil, nil
}

vm, ok := ref.(*object.VirtualMachine)
if !ok {
return nil, fmt.Errorf("found VM reference was not a VM but a %T", ref)
}

vmCtx.Logger.V(4).Info("Found VM via inventory",
"parentFolderMoID", folder.Reference().Value, "moID", vm.Reference().Value)
return vm, nil
}
89 changes: 2 additions & 87 deletions pkg/providers/vsphere/vcenter/getvm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ import (
. "github.com/onsi/gomega"

"github.com/vmware/govmomi/vim25/mo"
vimtypes "github.com/vmware/govmomi/vim25/types"
"k8s.io/apimachinery/pkg/types"

vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context"
"github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vcenter"
"github.com/vmware-tanzu/vm-operator/test/builder"
Expand Down Expand Up @@ -52,54 +50,6 @@ func getVM() {
}
})

Context("Gets VM by inventory", func() {
BeforeEach(func() {
vm, err := ctx.Finder.VirtualMachine(ctx, vcVMName)
Expect(err).ToNot(HaveOccurred())

task, err := vm.Clone(ctx, nsInfo.Folder, vmCtx.VM.Name, vimtypes.VirtualMachineCloneSpec{})
Expect(err).ToNot(HaveOccurred())
Expect(task.Wait(ctx)).To(Succeed())
})

It("returns success", func() {
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).ToNot(HaveOccurred())
Expect(vm).ToNot(BeNil())
})

It("returns nil if VM does not exist", func() {
vmCtx.VM.Name = "bogus"
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).ToNot(HaveOccurred())
Expect(vm).To(BeNil())
})

Context("Namespace Folder does not exist", func() {
BeforeEach(func() {
task, err := nsInfo.Folder.Destroy(vmCtx)
Expect(err).ToNot(HaveOccurred())
Expect(task.Wait(vmCtx)).To(Succeed())
})

It("returns error", func() {
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(HavePrefix("failed to get namespace Folder"))
Expect(vm).To(BeNil())
})
})

It("returns success when MoID is invalid", func() {
// Expect fallback to inventory.
vmCtx.VM.Status.UniqueID = "vm-bogus"

vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).ToNot(HaveOccurred())
Expect(vm).ToNot(BeNil())
})
})

Context("Gets VM when MoID is set", func() {
BeforeEach(func() {
vm, err := ctx.Finder.VirtualMachine(ctx, vcVMName)
Expand All @@ -108,7 +58,7 @@ func getVM() {
})

It("returns success", func() {
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).ToNot(HaveOccurred())
Expect(vm).ToNot(BeNil())
Expect(vm.Reference().Value).To(Equal(vmCtx.VM.Status.UniqueID))
Expand All @@ -126,44 +76,9 @@ func getVM() {
})

It("returns success", func() {
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).ToNot(HaveOccurred())
Expect(vm).ToNot(BeNil())
})
})

Context("Gets VM with ResourcePolicy by inventory", func() {
BeforeEach(func() {
resourcePolicy, folder := ctx.CreateVirtualMachineSetResourcePolicy("getvm-test", nsInfo)
if vmCtx.VM.Spec.Reserved == nil {
vmCtx.VM.Spec.Reserved = &vmopv1.VirtualMachineReservedSpec{}
}
vmCtx.VM.Spec.Reserved.ResourcePolicyName = resourcePolicy.Name

vm, err := ctx.Finder.VirtualMachine(ctx, vcVMName)
Expect(err).ToNot(HaveOccurred())

task, err := vm.Clone(ctx, folder, vmCtx.VM.Name, vimtypes.VirtualMachineCloneSpec{})
Expect(err).ToNot(HaveOccurred())
Expect(task.Wait(ctx)).To(Succeed())
})

It("returns success", func() {
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).ToNot(HaveOccurred())
Expect(vm).ToNot(BeNil())
})

It("returns error when ResourcePolicy does not exist", func() {
if vmCtx.VM.Spec.Reserved == nil {
vmCtx.VM.Spec.Reserved = &vmopv1.VirtualMachineReservedSpec{}
}
vmCtx.VM.Spec.Reserved.ResourcePolicyName = "bogus"

vm, err := vcenter.GetVirtualMachine(vmCtx, ctx.Client, ctx.VCClient.Client, ctx.Datacenter, ctx.Finder)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(HavePrefix("failed to get VirtualMachineSetResourcePolicy"))
Expect(vm).To(BeNil())
})
})
}
2 changes: 1 addition & 1 deletion pkg/providers/vsphere/vmprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (vs *vSphereVMProvider) getVM(
client *vcclient.Client,
notFoundReturnErr bool) (*object.VirtualMachine, error) {

vcVM, err := vcenter.GetVirtualMachine(vmCtx, vs.k8sClient, client.VimClient(), client.Datacenter(), client.Finder())
vcVM, err := vcenter.GetVirtualMachine(vmCtx, client.VimClient(), client.Datacenter(), client.Finder())
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 65fe835

Please sign in to comment.