From 67dbe3580176ca498fe2241596cec0bdfffeb9ed Mon Sep 17 00:00:00 2001 From: akutz Date: Mon, 28 Oct 2024 15:06:49 -0500 Subject: [PATCH] Support async create VM This patch updates the CreateOrUpdateVirtualMachine implementation to be asynchronous, i.e. non-blocking. This also removes the need for "create threads," as the moment a create operation is enqueued, the reconcile thread is returned to the pool. --- .../virtualmachine_controller.go | 13 +- .../virtualmachine_controller_intg_test.go | 50 ++-- .../virtualmachine_controller_unit_test.go | 37 ++- ...lmachinereplicaset_controller_intg_test.go | 9 +- pkg/providers/fake/fake_vm_provider.go | 6 +- pkg/providers/vm_provider_interface.go | 2 +- pkg/providers/vsphere/vmprovider_vm.go | 254 +++++++++--------- pkg/providers/vsphere/vmprovider_vm2_test.go | 63 ++++- .../vsphere/vmprovider_vm_resize_test.go | 51 +++- pkg/providers/vsphere/vmprovider_vm_test.go | 176 +++++++----- 10 files changed, 426 insertions(+), 235 deletions(-) diff --git a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go index 80965b00e..92b6267f2 100644 --- a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go +++ b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go @@ -338,7 +338,7 @@ func requeueDelay(ctx *pkgctx.VirtualMachineContext) time.Duration { return pkgcfg.FromContext(ctx).CreateVMRequeueDelay } - if ctx.VM.Status.PowerState == vmopv1.VirtualMachinePowerStateOn { + if ctx.VM.Status.PowerState == "" || ctx.VM.Status.PowerState == vmopv1.VirtualMachinePowerStateOn { networkSpec := ctx.VM.Spec.Network if networkSpec != nil && !networkSpec.Disabled { networkStatus := ctx.VM.Status.Network @@ -439,11 +439,18 @@ func (r *Reconciler) ReconcileNormal(ctx *pkgctx.VirtualMachineContext) (reterr // Upgrade schema fields where needed upgradeSchema(ctx) - err := r.VMProvider.CreateOrUpdateVirtualMachine(ctx, ctx.VM) + chanErr, err := r.VMProvider.CreateOrUpdateVirtualMachine(ctx, ctx.VM) switch { case ctxop.IsCreate(ctx): - r.Recorder.EmitEvent(ctx.VM, "Create", err, false) + if err != nil { + r.Recorder.EmitEvent(ctx.VM, "Create", err, false) + } else if chanErr != nil { + go func(obj client.Object) { + err := <-chanErr + r.Recorder.EmitEvent(obj, "Create", err, false) + }(ctx.VM.DeepCopy()) + } case ctxop.IsUpdate(ctx): r.Recorder.EmitEvent(ctx.VM, "Update", err, false) case err != nil: diff --git a/controllers/virtualmachine/virtualmachine/virtualmachine_controller_intg_test.go b/controllers/virtualmachine/virtualmachine/virtualmachine_controller_intg_test.go index 36a606843..6b1aee8ac 100644 --- a/controllers/virtualmachine/virtualmachine/virtualmachine_controller_intg_test.go +++ b/controllers/virtualmachine/virtualmachine/virtualmachine_controller_intg_test.go @@ -106,8 +106,10 @@ func intgTestsReconcile() { BeforeEach(func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { - return nil + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) + return chanErr, nil } intgFakeVMProvider.Unlock() }) @@ -137,10 +139,12 @@ func intgTestsReconcile() { By("Set InstanceUUID in CreateOrUpdateVM", func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) // Just using InstanceUUID here for a field to update. vm.Status.InstanceUUID = dummyInstanceUUID - return nil + return chanErr, nil } intgFakeVMProvider.Unlock() }) @@ -156,10 +160,11 @@ func intgTestsReconcile() { By("Set Created condition in CreateOrUpdateVM", func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { - vm.Status.PowerState = vmopv1.VirtualMachinePowerStateOn + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) conditions.MarkTrue(vm, vmopv1.VirtualMachineConditionCreated) - return nil + return chanErr, nil } intgFakeVMProvider.Unlock() }) @@ -175,11 +180,13 @@ func intgTestsReconcile() { By("Set IP address in CreateOrUpdateVM", func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) vm.Status.Network = &vmopv1.VirtualMachineNetworkStatus{ PrimaryIP4: dummyIPAddress, } - return nil + return chanErr, nil } intgFakeVMProvider.Unlock() }) @@ -214,10 +221,13 @@ func intgTestsReconcile() { BeforeEach(func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) vm.Status.InstanceUUID = instanceUUID vm.Status.BiosUUID = biosUUID - return nil + + return chanErr, nil } intgFakeVMProvider.Unlock() }) @@ -262,10 +272,12 @@ func intgTestsReconcile() { It("Reconciles after VirtualMachineClass change", func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) // Set this so requeueDelay() returns 0. conditions.MarkTrue(vm, vmopv1.VirtualMachineConditionCreated) - return nil + return chanErr, nil } intgFakeVMProvider.Unlock() @@ -284,9 +296,11 @@ func intgTestsReconcile() { instanceUUID := uuid.NewString() intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) vm.Status.InstanceUUID = instanceUUID - return nil + return chanErr, nil } intgFakeVMProvider.Unlock() @@ -400,9 +414,11 @@ var _ = Describe( provider.VSphereClientFn = func(ctx context.Context) (*vsclient.Client, error) { return vsclient.NewClient(ctx, vcSimCtx.VCClientConfig) } - provider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, obj *vmopv1.VirtualMachine) error { + provider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) atomic.AddInt32(&numCreateOrUpdateCalls, 1) - return nil + return chanErr, nil } vcSimCtx = builder.NewIntegrationTestContextForVCSim( diff --git a/controllers/virtualmachine/virtualmachine/virtualmachine_controller_unit_test.go b/controllers/virtualmachine/virtualmachine/virtualmachine_controller_unit_test.go index 4de219cfc..81ad6332a 100644 --- a/controllers/virtualmachine/virtualmachine/virtualmachine_controller_unit_test.go +++ b/controllers/virtualmachine/virtualmachine/virtualmachine_controller_unit_test.go @@ -6,7 +6,6 @@ package virtualmachine_test import ( "context" "errors" - "fmt" "strings" . "github.com/onsi/ginkgo/v2" @@ -160,8 +159,10 @@ func unitTestsReconcile() { Context("ProberManager", func() { It("Should call add to Prober Manager if ReconcileNormal fails", func() { - fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { - return errors.New(providerError) + fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) + return chanErr, errors.New(providerError) } err := reconciler.ReconcileNormal(vmCtx) @@ -180,9 +181,11 @@ func unitTestsReconcile() { }) It("Should emit a CreateSuccess event if ReconcileNormal causes a successful VM creation", func() { - fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) ctxop.MarkCreate(ctx) - return nil + return chanErr, nil } Expect(reconciler.ReconcileNormal(vmCtx)).Should(Succeed()) @@ -190,9 +193,11 @@ func unitTestsReconcile() { }) It("Should emit CreateFailure event if ReconcileNormal causes a failed VM create", func() { - fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) ctxop.MarkCreate(ctx) - return fmt.Errorf("fake") + return chanErr, errors.New("fake") } Expect(reconciler.ReconcileNormal(vmCtx)).ShouldNot(Succeed()) @@ -200,9 +205,11 @@ func unitTestsReconcile() { }) It("Should emit UpdateSuccess event if ReconcileNormal causes a successful VM update", func() { - fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) ctxop.MarkUpdate(ctx) - return nil + return chanErr, nil } Expect(reconciler.ReconcileNormal(vmCtx)).Should(Succeed()) @@ -210,9 +217,11 @@ func unitTestsReconcile() { }) It("Should emit UpdateFailure event if ReconcileNormal causes a failed VM update", func() { - fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) ctxop.MarkUpdate(ctx) - return fmt.Errorf("fake") + return chanErr, errors.New("fake") } Expect(reconciler.ReconcileNormal(vmCtx)).ShouldNot(Succeed()) @@ -220,8 +229,10 @@ func unitTestsReconcile() { }) It("Should emit ReconcileNormalFailure if ReconcileNormal fails for neither create or update op", func() { - fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { - return fmt.Errorf("fake") + fakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) + return chanErr, errors.New("fake") } Expect(reconciler.ReconcileNormal(vmCtx)).ShouldNot(Succeed()) diff --git a/controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller_intg_test.go b/controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller_intg_test.go index b7a032f39..28668bdb5 100644 --- a/controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller_intg_test.go +++ b/controllers/virtualmachinereplicaset/virtualmachinereplicaset_controller_intg_test.go @@ -125,11 +125,16 @@ func intgTestsReconcile() { BeforeEach(func() { intgFakeVMProvider.Lock() - intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) error { + intgFakeVMProvider.CreateOrUpdateVirtualMachineFn = func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { + chanErr := make(chan error) + close(chanErr) + // Used below just to check for something in the Status is updated. vm.Status.InstanceUUID = dummyInstanceUUID - return nil + + return chanErr, nil } + intgFakeVMProvider.Unlock() }) diff --git a/pkg/providers/fake/fake_vm_provider.go b/pkg/providers/fake/fake_vm_provider.go index b30fcd6cc..3731e6e13 100644 --- a/pkg/providers/fake/fake_vm_provider.go +++ b/pkg/providers/fake/fake_vm_provider.go @@ -28,7 +28,7 @@ import ( // expected to evolve as more tests get added in the future. type funcs struct { - CreateOrUpdateVirtualMachineFn func(ctx context.Context, vm *vmopv1.VirtualMachine) error + CreateOrUpdateVirtualMachineFn func(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) DeleteVirtualMachineFn func(ctx context.Context, vm *vmopv1.VirtualMachine) error PublishVirtualMachineFn func(ctx context.Context, vm *vmopv1.VirtualMachine, vmPub *vmopv1.VirtualMachinePublishRequest, cl *imgregv1a1.ContentLibrary, actID string) (string, error) @@ -82,14 +82,14 @@ func (s *VMProvider) Reset() { s.isPublishVMCalled = false } -func (s *VMProvider) CreateOrUpdateVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine) error { +func (s *VMProvider) CreateOrUpdateVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) { s.Lock() defer s.Unlock() if s.CreateOrUpdateVirtualMachineFn != nil { return s.CreateOrUpdateVirtualMachineFn(ctx, vm) } s.addToVMMap(vm) - return nil + return nil, nil } func (s *VMProvider) DeleteVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine) error { diff --git a/pkg/providers/vm_provider_interface.go b/pkg/providers/vm_provider_interface.go index 0e2a9df09..3795028a3 100644 --- a/pkg/providers/vm_provider_interface.go +++ b/pkg/providers/vm_provider_interface.go @@ -18,7 +18,7 @@ import ( // VirtualMachineProviderInterface is a pluggable interface for VM Providers. type VirtualMachineProviderInterface interface { - CreateOrUpdateVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine) error + CreateOrUpdateVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine) (<-chan error, error) DeleteVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine) error PublishVirtualMachine(ctx context.Context, vm *vmopv1.VirtualMachine, vmPub *vmopv1.VirtualMachinePublishRequest, cl *imgregv1a1.ContentLibrary, actID string) (string, error) diff --git a/pkg/providers/vsphere/vmprovider_vm.go b/pkg/providers/vsphere/vmprovider_vm.go index a6f0ef6d0..262e95ecf 100644 --- a/pkg/providers/vsphere/vmprovider_vm.go +++ b/pkg/providers/vsphere/vmprovider_vm.go @@ -9,7 +9,6 @@ import ( "maps" "math/rand" "strings" - "sync" "text/template" "time" @@ -22,6 +21,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" apierrorsutil "k8s.io/apimachinery/pkg/util/errors" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" imgregv1a1 "github.com/vmware-tanzu/image-registry-operator-api/api/v1alpha1" @@ -30,6 +30,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" + ctxop "github.com/vmware-tanzu/vm-operator/pkg/context/operation" vcclient "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/client" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/clustermodules" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" @@ -73,9 +74,6 @@ type vmUpdateArgs = session.VMUpdateArgs type vmResizeArgs = session.VMResizeArgs var ( - createCountLock sync.Mutex - concurrentCreateCount int - // SkipVMImageCLProviderCheck skips the checks that a VM Image has a Content Library item provider // since a VirtualMachineImage created for a VM template won't have either. This has been broken for // a long time but was otherwise masked on how the tests used to be organized. @@ -84,7 +82,17 @@ var ( func (vs *vSphereVMProvider) CreateOrUpdateVirtualMachine( ctx context.Context, - vm *vmopv1.VirtualMachine) error { + vm *vmopv1.VirtualMachine) (<-chan error, error) { + + chanErr := make(chan error) + err := vs.createOrUpdateVirtualMachine(ctx, vm, chanErr) + return chanErr, err +} + +func (vs *vSphereVMProvider) createOrUpdateVirtualMachine( + ctx context.Context, + vm *vmopv1.VirtualMachine, + chanErr chan error) error { vmCtx := pkgctx.VirtualMachineContext{ Context: context.WithValue(ctx, vimtypes.ID{}, vs.getOpID(vm, "createOrUpdateVM")), @@ -111,22 +119,37 @@ func (vs *vSphereVMProvider) CreateOrUpdateVirtualMachine( } if vcVM == nil { - var createArgs *VMCreateArgs - - vcVM, createArgs, err = vs.createVirtualMachine(vmCtx, client) + createArgs, err := vs.getCreateArgs(vmCtx, client) if err != nil { return err } - if vcVM == nil { - // Creation was not ready or blocked for some reason. We depend on the controller - // to eventually retry the create. - return nil - } + // Create a copy of the context and replace its VM with a copy to ensure + // modifications in the goroutine below are not impacted or impact the + // operations above us in the call stack. + copyOfCtx := vmCtx + copyOfCtx.VM = vmCtx.VM.DeepCopy() + + // Record this as a create operation before entering the goroutine. + ctxop.MarkCreate(vmCtx) - return vs.createdVirtualMachineFallthroughUpdate(vmCtx, vcVM, client, createArgs) + // Start a goroutine to create the VM in the background. + go vs.createVirtualMachine( + copyOfCtx, + client, + createArgs, + chanErr) + + // Return with no error. The VM will be re-enqueued once the create + // completes with success or failure. + return nil } + defer close(chanErr) + + // Record this as an update operation. + ctxop.MarkUpdate(vmCtx) + return vs.updateVirtualMachine(vmCtx, vcVM, client, nil) } @@ -388,78 +411,84 @@ func (vs *vSphereVMProvider) vmCreatePathName( return nil } -func (vs *vSphereVMProvider) createVirtualMachine( +func (vs *vSphereVMProvider) getCreateArgs( vmCtx pkgctx.VirtualMachineContext, - vcClient *vcclient.Client) (*object.VirtualMachine, *VMCreateArgs, error) { + vcClient *vcclient.Client) (*VMCreateArgs, error) { createArgs, err := vs.vmCreateGetArgs(vmCtx, vcClient) if err != nil { - return nil, nil, err + return nil, err } - err = vs.vmCreateDoPlacement(vmCtx, vcClient, createArgs) - if err != nil { - return nil, nil, err + if err := vs.vmCreateDoPlacement(vmCtx, vcClient, createArgs); err != nil { + return nil, err } - err = vs.vmCreateGetFolderAndRPMoIDs(vmCtx, vcClient, createArgs) - if err != nil { - return nil, nil, err + if err := vs.vmCreateGetFolderAndRPMoIDs(vmCtx, vcClient, createArgs); err != nil { + return nil, err } - err = vs.vmCreatePathName(vmCtx, vcClient, createArgs) - if err != nil { - return nil, nil, err + if err := vs.vmCreatePathName(vmCtx, vcClient, createArgs); err != nil { + return nil, err } - err = vs.vmCreateFixupConfigSpec(vmCtx, vcClient, createArgs) - if err != nil { - return nil, nil, err + if err := vs.vmCreateFixupConfigSpec(vmCtx, vcClient, createArgs); err != nil { + return nil, err } - err = vs.vmCreateIsReady(vmCtx, vcClient, createArgs) - if err != nil { - return nil, nil, err + if err := vs.vmCreateIsReady(vmCtx, vcClient, createArgs); err != nil { + return nil, err } - // BMV: This is about where we used to do this check but it prb make more sense to do - // earlier, as to limit wasted work. Before DoPlacement() is likely the best place so - // the window between the placement decision and creating the VM on VC is small(ish). - allowed, createDeferFn := vs.vmCreateConcurrentAllowed(vmCtx) - if !allowed { - return nil, nil, nil - } - defer createDeferFn() + return createArgs, nil +} - moRef, err := vmlifecycle.CreateVirtualMachine( - vmCtx, +func (vs *vSphereVMProvider) createVirtualMachine( + ctx pkgctx.VirtualMachineContext, + vcClient *vcclient.Client, + args *VMCreateArgs, + chanErr chan error) { + + defer close(chanErr) + + moRef, vimErr := vmlifecycle.CreateVirtualMachine( + ctx, vcClient.RestClient(), vcClient.VimClient(), vcClient.Finder(), - &createArgs.CreateArgs) + &args.CreateArgs) - if err != nil { - vmCtx.Logger.Error(err, "CreateVirtualMachine failed") - conditions.MarkFalse(vmCtx.VM, vmopv1.VirtualMachineConditionCreated, "Error", err.Error()) - return nil, nil, err + if vimErr != nil { + chanErr <- vimErr + ctx.Logger.Error(vimErr, "CreateVirtualMachine failed") } - vmCtx.VM.Status.UniqueID = moRef.Reference().Value - conditions.MarkTrue(vmCtx.VM, vmopv1.VirtualMachineConditionCreated) - - return object.NewVirtualMachine(vcClient.VimClient(), *moRef), createArgs, nil -} + _, k8sErr := controllerutil.CreateOrPatch( + ctx, + vs.k8sClient, + ctx.VM, + func() error { + + if vimErr != nil { + conditions.MarkFalse( + ctx.VM, + vmopv1.VirtualMachineConditionCreated, + "Error", + vimErr.Error()) + return nil //nolint:nilerr + } -func (vs *vSphereVMProvider) createdVirtualMachineFallthroughUpdate( - vmCtx pkgctx.VirtualMachineContext, - vcVM *object.VirtualMachine, - vcClient *vcclient.Client, - createArgs *VMCreateArgs) error { + ctx.VM.Status.UniqueID = moRef.Reference().Value + conditions.MarkTrue(ctx.VM, vmopv1.VirtualMachineConditionCreated) - // TODO: In the common case, we'll call directly into update right after create succeeds, and - // can use the createArgs to avoid doing a bunch of lookup work again. + return nil + }, + ) - return vs.updateVirtualMachine(vmCtx, vcVM, vcClient, createArgs) + if k8sErr != nil { + chanErr <- k8sErr + ctx.Logger.Error(k8sErr, "Failed to patch VM status after create") + } } // VMUpdatePropertiesSelector is the set of VM properties fetched at the start @@ -484,53 +513,58 @@ func (vs *vSphereVMProvider) updateVirtualMachine( vmCtx.Logger.V(4).Info("Updating VirtualMachine") - { - // Hack - create just enough of the Session that's needed for update + // TODO(akutz) This is a hack to create just enough of the Session needed + // for the update. - if err := vcVM.Properties( - vmCtx, - vcVM.Reference(), - VMUpdatePropertiesSelector, - &vmCtx.MoVM); err != nil { + if err := vcVM.Properties( + vmCtx, + vcVM.Reference(), + VMUpdatePropertiesSelector, + &vmCtx.MoVM); err != nil { - return err - } + return err + } - if vmCtx.MoVM.ResourcePool == nil { - // Same error as govmomi VirtualMachine::ResourcePool(). - return fmt.Errorf("VM doesn't have a resourcePool") - } + // Check to see if the VM has any outstanding tasks associated with it. If + // there are, then we should return early from this reconciler thread. This + // is because active tasks prevent us from making changes, and the task's + // completion will enqueue a reconcile for the VM, sending it back into the + // reconcile workflow. - clusterMoRef, err := vcenter.GetResourcePoolOwnerMoRef( - vmCtx, - vcVM.Client(), - vmCtx.MoVM.ResourcePool.Value) - if err != nil { - return err - } + if vmCtx.MoVM.ResourcePool == nil { + // Same error as govmomi VirtualMachine::ResourcePool(). + return fmt.Errorf("VM doesn't have a resourcePool") + } - ses := &session.Session{ - K8sClient: vs.k8sClient, - Client: vcClient.Client, - Finder: vcClient.Finder(), - ClusterMoRef: clusterMoRef, - } + clusterMoRef, err := vcenter.GetResourcePoolOwnerMoRef( + vmCtx, + vcVM.Client(), + vmCtx.MoVM.ResourcePool.Value) + if err != nil { + return err + } - getUpdateArgsFn := func() (*vmUpdateArgs, error) { - // TODO: Use createArgs if we already got them, except for: - // - createArgs.ConfigSpec.Crypto - _ = createArgs - return vs.vmUpdateGetArgs(vmCtx) - } + ses := &session.Session{ + K8sClient: vs.k8sClient, + Client: vcClient.Client, + Finder: vcClient.Finder(), + ClusterMoRef: clusterMoRef, + } - getResizeArgsFn := func() (*vmResizeArgs, error) { - return vs.vmResizeGetArgs(vmCtx) - } + getUpdateArgsFn := func() (*vmUpdateArgs, error) { + // TODO: Use createArgs if we already got them, except for: + // - createArgs.ConfigSpec.Crypto + _ = createArgs + return vs.vmUpdateGetArgs(vmCtx) + } - err = ses.UpdateVirtualMachine(vmCtx, vcVM, getUpdateArgsFn, getResizeArgsFn) - if err != nil { - return err - } + getResizeArgsFn := func() (*vmResizeArgs, error) { + return vs.vmResizeGetArgs(vmCtx) + } + + err = ses.UpdateVirtualMachine(vmCtx, vcVM, getUpdateArgsFn, getResizeArgsFn) + if err != nil { + return err } // Back up the VM at the end after a successful update. TKG nodes are skipped @@ -771,28 +805,6 @@ func (vs *vSphereVMProvider) vmCreateIsReady( return nil } -func (vs *vSphereVMProvider) vmCreateConcurrentAllowed(vmCtx pkgctx.VirtualMachineContext) (bool, func()) { - maxDeployThreads := pkgcfg.FromContext(vmCtx).GetMaxDeployThreadsOnProvider() - - createCountLock.Lock() - if concurrentCreateCount >= maxDeployThreads { - createCountLock.Unlock() - vmCtx.Logger.Info("Too many create VirtualMachine already occurring. Re-queueing request") - return false, nil - } - - concurrentCreateCount++ - createCountLock.Unlock() - - decrementFn := func() { - createCountLock.Lock() - concurrentCreateCount-- - createCountLock.Unlock() - } - - return true, decrementFn -} - func (vs *vSphereVMProvider) vmCreateGetArgs( vmCtx pkgctx.VirtualMachineContext, vcClient *vcclient.Client) (*VMCreateArgs, error) { diff --git a/pkg/providers/vsphere/vmprovider_vm2_test.go b/pkg/providers/vsphere/vmprovider_vm2_test.go index e081a8837..70723b9af 100644 --- a/pkg/providers/vsphere/vmprovider_vm2_test.go +++ b/pkg/providers/vsphere/vmprovider_vm2_test.go @@ -4,6 +4,7 @@ package vsphere_test import ( + "fmt" "time" . "github.com/onsi/ginkgo/v2" @@ -23,6 +24,7 @@ import ( "github.com/vmware-tanzu/vm-operator/api/v1alpha3/common" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + ctxop "github.com/vmware-tanzu/vm-operator/pkg/context/operation" "github.com/vmware-tanzu/vm-operator/pkg/providers" vsphere "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/network" @@ -125,6 +127,51 @@ func vmE2ETests() { sysprepSecret = nil }) + createOrUpdateVM := func( + ctx *builder.TestContextForVCSim, + vm *vmopv1.VirtualMachine) error { + + chanErr, err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + if err != nil { + return err + } + + // Unlike the VM controller, this test helper blocks until the async + // parts of CreateOrUpdateVM are complete. This is to avoid a large + // refactor for now. + for err2 := range chanErr { + if err2 != nil { + if err == nil { + err = err2 + } else { + err = fmt.Errorf("%w,%w", err, err2) + } + } + } + if err != nil { + return err + } + + if ctxop.IsCreate(ctx) { + // The async create operation does not fall-through to the + // update logic, so we need to call CreateOrUpdateVirtualMachine + // a second time to cause the update. + ExpectWithOffset(1, ctx.Client.Get( + ctx, + client.ObjectKeyFromObject(vm), + vm)).To(Succeed()) + + if _, err := vmProvider.CreateOrUpdateVirtualMachine( + ctx, + vm); err != nil { + + return err + } + } + + return nil + } + Context("Nil fields in Spec", func() { BeforeEach(func() { @@ -137,10 +184,10 @@ func vmE2ETests() { }) It("DoIt without an NPE", func() { - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).ToNot(HaveOccurred()) - err = vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err = createOrUpdateVM(ctx, vm) Expect(err).ToNot(HaveOccurred()) Expect(vm.Status.UniqueID).ToNot(BeEmpty()) @@ -188,7 +235,7 @@ func vmE2ETests() { }) It("DoIt", func() { - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("network interface is not ready yet")) Expect(conditions.IsFalse(vm, vmopv1.VirtualMachineConditionNetworkReady)).To(BeTrue()) @@ -222,7 +269,7 @@ func vmE2ETests() { Expect(ctx.Client.Status().Update(ctx, netInterface)).To(Succeed()) }) - err = vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err = createOrUpdateVM(ctx, vm) Expect(err).ToNot(HaveOccurred()) By("has expected conditions", func() { @@ -296,7 +343,7 @@ func vmE2ETests() { }) It("DoIt", func() { - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("network interface is not ready yet")) Expect(conditions.IsFalse(vm, vmopv1.VirtualMachineConditionNetworkReady)).To(BeTrue()) @@ -331,7 +378,7 @@ func vmE2ETests() { Expect(ctx.Client.Status().Update(ctx, netInterface)).To(Succeed()) }) - err = vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err = createOrUpdateVM(ctx, vm) Expect(err).ToNot(HaveOccurred()) By("has expected conditions", func() { @@ -407,7 +454,7 @@ func vmE2ETests() { }) It("DoIt", func() { - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("subnetPort is not ready yet")) Expect(conditions.IsFalse(vm, vmopv1.VirtualMachineConditionNetworkReady)).To(BeTrue()) @@ -439,7 +486,7 @@ func vmE2ETests() { Expect(ctx.Client.Status().Update(ctx, subnetPort)).To(Succeed()) }) - err = vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err = createOrUpdateVM(ctx, vm) Expect(err).ToNot(HaveOccurred()) By("has expected conditions", func() { diff --git a/pkg/providers/vsphere/vmprovider_vm_resize_test.go b/pkg/providers/vsphere/vmprovider_vm_resize_test.go index ffeed3595..cffba4af2 100644 --- a/pkg/providers/vsphere/vmprovider_vm_resize_test.go +++ b/pkg/providers/vsphere/vmprovider_vm_resize_test.go @@ -20,6 +20,7 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" "github.com/vmware-tanzu/vm-operator/pkg/conditions" pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config" + ctxop "github.com/vmware-tanzu/vm-operator/pkg/context/operation" "github.com/vmware-tanzu/vm-operator/pkg/providers" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" @@ -68,12 +69,58 @@ func vmResizeTests() { return w.Bytes() } + createOrUpdateVM := func( + testCtx *builder.TestContextForVCSim, + vm *vmopv1.VirtualMachine) error { + + ctx := ctxop.WithContext(testCtx) + + chanErr, err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + if err != nil { + return err + } + + // Unlike the VM controller, this test helper blocks until the async + // parts of CreateOrUpdateVM are complete. This is to avoid a large + // refactor for now. + for err2 := range chanErr { + if err2 != nil { + if err == nil { + err = err2 + } else { + err = fmt.Errorf("%w,%w", err, err2) + } + } + } + if err != nil { + return err + } + + if ctxop.IsCreate(ctx) { + // The async create operation does not fall-through to the + // update logic, so we need to call CreateOrUpdateVirtualMachine + // a second time to cause the update. + ExpectWithOffset(1, testCtx.Client.Get( + ctx, + client.ObjectKeyFromObject(vm), + vm)).To(Succeed()) + + if _, err := vmProvider.CreateOrUpdateVirtualMachine( + ctx, + vm); err != nil { + + return err + } + } + + return nil + } + createOrUpdateAndGetVcVM := func( ctx *builder.TestContextForVCSim, vm *vmopv1.VirtualMachine) (*object.VirtualMachine, error) { - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) - if err != nil { + if err := createOrUpdateVM(ctx, vm); err != nil { return nil, err } diff --git a/pkg/providers/vsphere/vmprovider_vm_test.go b/pkg/providers/vsphere/vmprovider_vm_test.go index 899f59d3a..c8fa25d2c 100644 --- a/pkg/providers/vsphere/vmprovider_vm_test.go +++ b/pkg/providers/vsphere/vmprovider_vm_test.go @@ -158,12 +158,58 @@ func vmTests() { vsphere.SkipVMImageCLProviderCheck = false }) + createOrUpdateVM := func( + testCtx *builder.TestContextForVCSim, + vm *vmopv1.VirtualMachine) error { + + ctx := ctxop.WithContext(testCtx) + + chanErr, err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + if err != nil { + return err + } + + // Unlike the VM controller, this test helper blocks until the async + // parts of CreateOrUpdateVM are complete. This is to avoid a large + // refactor for now. + for err2 := range chanErr { + if err2 != nil { + if err == nil { + err = err2 + } else { + err = fmt.Errorf("%w,%w", err, err2) + } + } + } + if err != nil { + return err + } + + if ctxop.IsCreate(ctx) { + // The async create operation does not fall-through to the + // update logic, so we need to call CreateOrUpdateVirtualMachine + // a second time to cause the update. + ExpectWithOffset(1, testCtx.Client.Get( + ctx, + client.ObjectKeyFromObject(vm), + vm)).To(Succeed()) + + if _, err := vmProvider.CreateOrUpdateVirtualMachine( + ctx, + vm); err != nil { + + return err + } + } + + return nil + } + createOrUpdateAndGetVcVM := func( ctx *builder.TestContextForVCSim, vm *vmopv1.VirtualMachine) (*object.VirtualMachine, error) { - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) - if err != nil { + if err := createOrUpdateVM(ctx, vm); err != nil { return nil, err } @@ -2089,7 +2135,7 @@ func vmTests() { } Expect(ctx.Client.Update(ctx, vmClass)).To(Succeed()) - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + _, err := createOrUpdateAndGetVcVM(ctx, vm) Expect(err).To(MatchError("instance storage PVCs are not bound yet")) Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineConditionCreated)).To(BeFalse()) @@ -2111,7 +2157,7 @@ func vmTests() { // Simulate what would be set by volume controller. vm.Annotations[constants.InstanceStoragePVCsBoundAnnotationKey] = "" - err = vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + _, err := createOrUpdateAndGetVcVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("status update pending for persistent volume: %s on VM", isVol0.Name))) @@ -2125,7 +2171,7 @@ func vmTests() { }) By("VM is now created", func() { - _, err = createOrUpdateAndGetVcVM(ctx, vm) + _, err := createOrUpdateAndGetVcVM(ctx, vm) Expect(err).ToNot(HaveOccurred()) Expect(conditions.IsTrue(vm, vmopv1.VirtualMachineConditionCreated)).To(BeTrue()) }) @@ -2133,12 +2179,12 @@ func vmTests() { }) It("Powers VM off", func() { - vcVM, err := createOrUpdateAndGetVcVM(ctx, vm) - Expect(err).ToNot(HaveOccurred()) - + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) Expect(vm.Status.PowerState).To(Equal(vmopv1.VirtualMachinePowerStateOn)) + vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOff - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + vcVM, err := createOrUpdateAndGetVcVM(ctx, vm) + Expect(err).ToNot(HaveOccurred()) Expect(vm.Status.PowerState).To(Equal(vmopv1.VirtualMachinePowerStateOff)) state, err := vcVM.PowerState(ctx) @@ -2148,7 +2194,7 @@ func vmTests() { It("returns error when StorageClass is required but none specified", func() { vm.Spec.StorageClass = "" - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(MatchError("StorageClass is required but not specified")) c := conditions.Get(vm, vmopv1.VirtualMachineConditionStorageReady) @@ -2417,7 +2463,7 @@ func vmTests() { }, } - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("status update pending for persistent volume: %s on VM", cnsVolumeName))) Expect(vm.Status.PowerState).To(Equal(vmopv1.VirtualMachinePowerStateOff)) @@ -2434,7 +2480,7 @@ func vmTests() { }, } - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("persistent volume: %s not attached to VM", cnsVolumeName))) Expect(vm.Status.PowerState).To(Equal(vmopv1.VirtualMachinePowerStateOff)) @@ -2447,7 +2493,7 @@ func vmTests() { Attached: true, }, } - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) Expect(vm.Status.PowerState).To(Equal(vmopv1.VirtualMachinePowerStateOn)) }) }) @@ -2461,7 +2507,7 @@ func vmTests() { Expect(vm.Status.Zone).To(Equal(zoneName)) delete(vm.Labels, topology.KubernetesTopologyZoneLabelKey) - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) Expect(vm.Labels).To(HaveKeyWithValue(topology.KubernetesTopologyZoneLabelKey, zoneName)) Expect(vm.Status.Zone).To(Equal(zoneName)) }) @@ -2528,7 +2574,7 @@ func vmTests() { It("Returns error with non-existence cluster module", func() { vm.Annotations["vsphere-cluster-module-group"] = "bogusClusterMod" - err := vmProvider.CreateOrUpdateVirtualMachine(ctx, vm) + err := createOrUpdateVM(ctx, vm) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("ClusterModule bogusClusterMod not found")) }) @@ -2543,7 +2589,7 @@ func vmTests() { }) JustBeforeEach(func() { - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) }) Context("when the VM is off", func() { @@ -2604,7 +2650,7 @@ func vmTests() { Context("Guest Heartbeat", func() { JustBeforeEach(func() { - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) }) It("return guest heartbeat", func() { @@ -2617,7 +2663,7 @@ func vmTests() { Context("Web console ticket", func() { JustBeforeEach(func() { - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) }) It("return ticket", func() { @@ -2630,7 +2676,7 @@ func vmTests() { Context("VM hardware version", func() { JustBeforeEach(func() { - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) }) It("return version", func() { @@ -2639,59 +2685,59 @@ func vmTests() { Expect(version).To(Equal(vimtypes.VMX9)) }) }) - }) - - Context("Create/Update/Delete ISO backed VirtualMachine", func() { - var ( - vm *vmopv1.VirtualMachine - vmClass *vmopv1.VirtualMachineClass - ) - BeforeEach(func() { - vmClass = builder.DummyVirtualMachineClassGenName() - vm = builder.DummyBasicVirtualMachine("test-vm", "") - - // Reduce diff from old tests: by default don't create an NIC. - if vm.Spec.Network == nil { - vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{} - } - vm.Spec.Network.Disabled = true - }) - - JustBeforeEach(func() { - vmClass.Namespace = nsInfo.Namespace - Expect(ctx.Client.Create(ctx, vmClass)).To(Succeed()) + Context("Create/Update/Delete ISO backed VirtualMachine", func() { + var ( + vm *vmopv1.VirtualMachine + vmClass *vmopv1.VirtualMachineClass + ) - clusterVMImage := &vmopv1.ClusterVirtualMachineImage{} - Expect(ctx.Client.Get(ctx, client.ObjectKey{Name: ctx.ContentLibraryIsoImageName}, clusterVMImage)).To(Succeed()) + BeforeEach(func() { + vmClass = builder.DummyVirtualMachineClassGenName() + vm = builder.DummyBasicVirtualMachine("test-vm", "") - vm.Namespace = nsInfo.Namespace - vm.Spec.ClassName = vmClass.Name - vm.Spec.ImageName = clusterVMImage.Name - vm.Spec.Image.Kind = cvmiKind - vm.Spec.Image.Name = clusterVMImage.Name - vm.Spec.StorageClass = ctx.StorageClassName - vm.Spec.Cdrom = []vmopv1.VirtualMachineCdromSpec{{ - Name: "cdrom0", - Image: vmopv1.VirtualMachineImageRef{ - Name: cvmiKind, - Kind: clusterVMImage.Name, - }, - }} - }) + // Reduce diff from old tests: by default don't create an NIC. + if vm.Spec.Network == nil { + vm.Spec.Network = &vmopv1.VirtualMachineNetworkSpec{} + } + vm.Spec.Network.Disabled = true + }) - Context("return config", func() { JustBeforeEach(func() { - Expect(vmProvider.CreateOrUpdateVirtualMachine(ctx, vm)).To(Succeed()) + vmClass.Namespace = nsInfo.Namespace + Expect(ctx.Client.Create(ctx, vmClass)).To(Succeed()) + + clusterVMImage := &vmopv1.ClusterVirtualMachineImage{} + Expect(ctx.Client.Get(ctx, client.ObjectKey{Name: ctx.ContentLibraryIsoImageName}, clusterVMImage)).To(Succeed()) + + vm.Namespace = nsInfo.Namespace + vm.Spec.ClassName = vmClass.Name + vm.Spec.ImageName = clusterVMImage.Name + vm.Spec.Image.Kind = cvmiKind + vm.Spec.Image.Name = clusterVMImage.Name + vm.Spec.StorageClass = ctx.StorageClassName + vm.Spec.Cdrom = []vmopv1.VirtualMachineCdromSpec{{ + Name: "cdrom0", + Image: vmopv1.VirtualMachineImageRef{ + Name: cvmiKind, + Kind: clusterVMImage.Name, + }, + }} }) - It("return config.files", func() { - vmPathName := "config.files.vmPathName" - props, err := vmProvider.GetVirtualMachineProperties(ctx, vm, []string{vmPathName}) - Expect(err).NotTo(HaveOccurred()) - var path object.DatastorePath - path.FromString(props[vmPathName].(string)) - Expect(path.Datastore).NotTo(BeEmpty()) + Context("return config", func() { + JustBeforeEach(func() { + Expect(createOrUpdateVM(ctx, vm)).To(Succeed()) + }) + + It("return config.files", func() { + vmPathName := "config.files.vmPathName" + props, err := vmProvider.GetVirtualMachineProperties(ctx, vm, []string{vmPathName}) + Expect(err).NotTo(HaveOccurred()) + var path object.DatastorePath + path.FromString(props[vmPathName].(string)) + Expect(path.Datastore).NotTo(BeEmpty()) + }) }) }) })