diff --git a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go index 42c2b2d25..63ddd2036 100644 --- a/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go +++ b/controllers/virtualmachine/virtualmachine/virtualmachine_controller.go @@ -261,7 +261,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re mode = pkgcfg.FromContext(ctx).FastDeployMode } - reason := "unsupported mode" + var reasons []string + if !pkgcfg.FromContext(ctx).Features.BringYourOwnEncryptionKey { isEncStorClass, _, err := kubeutil.IsEncryptedStorageClass( ctx, r.Client, vm.Spec.StorageClass) @@ -272,15 +273,23 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re } if isEncStorClass { mode = "" - reason = "encrypted storage class sans byok" + reasons = append(reasons, "encrypted storage class sans byok") } } - if mode != "" { - if pkgcfg.FromContext(ctx).Features.InstanceStorage && vmopv1util.IsInstanceStoragePresent(vm) { - mode = "" - reason = "instance storage present" - } + if pkgcfg.FromContext(ctx).Features.InstanceStorage && + vmopv1util.IsInstanceStoragePresent(vm) { + + mode = "" + reasons = append(reasons, "instance storage present") + + } + + if !pkgcfg.FromContext(ctx).FastDeployExplicitDir && + strings.ToLower(mode) == pkgconst.FastDeployModeDirect { + + mode = "" + reasons = append(reasons, "direct mode not supported") } switch strings.ToLower(mode) { @@ -292,9 +301,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re cfg := pkgcfg.FromContext(ctx) cfg.Features.FastDeploy = false ctx = pkgcfg.WithContext(ctx, cfg) + + if len(reasons) == 0 { + reasons = []string{"unsupported mode"} + } + logger.Info( "Disabled fast-deploy for this VM", - "mode", mode, "reason", reason) + "mode", mode, "reasons", strings.Join(reasons, ",")) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index e941eed22..312ab065e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -146,9 +146,14 @@ type Config struct { // - the value is anything else, then fast deploy is not used to deploy // VMs. // - // Defaults to "direct". + // Defaults to "linked". FastDeployMode string + // FastDeployExplicitDir indicates whether or not we can explicitly create + // a VM directory with fast deploy. If false, direct mode is not supported. + // This is due to a bug with vSAN that is being corrected. + FastDeployExplicitDir bool + // VCCredsSecretName is the name of the secret in the pod namespace that // contains the VC credentials. // diff --git a/pkg/providers/vsphere/vmlifecycle/create.go b/pkg/providers/vsphere/vmlifecycle/create.go index d47ea8e5c..e4bb34c79 100644 --- a/pkg/providers/vsphere/vmlifecycle/create.go +++ b/pkg/providers/vsphere/vmlifecycle/create.go @@ -46,6 +46,8 @@ type DatastoreRef struct { // true if for a disk. DiskKey is only valid if ForDisk is true. ForDisk bool DiskKey int32 + + Capabilities vimtypes.DatastoreCapability } func CreateVirtualMachine( diff --git a/pkg/providers/vsphere/vmlifecycle/create_fastdeploy.go b/pkg/providers/vsphere/vmlifecycle/create_fastdeploy.go index e18a6ab88..e2002920d 100644 --- a/pkg/providers/vsphere/vmlifecycle/create_fastdeploy.go +++ b/pkg/providers/vsphere/vmlifecycle/create_fastdeploy.go @@ -25,6 +25,7 @@ import ( "github.com/vmware-tanzu/vm-operator/pkg/util/ptr" ) +// nolint:gocyclo func fastDeploy( vmCtx pkgctx.VirtualMachineContext, vimClient *vim25.Client, @@ -104,91 +105,128 @@ func fastDeploy( }) logger.Info("Got datacenter", "datacenter", datacenter.Reference()) - // Create the directory where the VM will be created. - fm := object.NewFileManager(vimClient) - if err := fm.MakeDirectory(vmCtx, vmDir, datacenter, true); err != nil { - return nil, fmt.Errorf("failed to create vm dir %q: %w", vmDir, err) - } + // Determine the type of fast deploy operation. + var fastDeployMode string - // Copy any non-disk files to the target directory. - for i := range dstFilePaths { - task, err := fm.CopyDatastoreFile( - vmCtx, - srcFilePaths[i], - datacenter, - dstFilePaths[i], - datacenter, false) - if err != nil { - return nil, err - } - if err = task.Wait(vmCtx); err != nil { - return nil, err + if createArgs.IsEncryptedStorageProfile { + fastDeployMode = pkgconst.FastDeployModeDirect + } else { + fastDeployMode = vmCtx.VM.Annotations[pkgconst.FastDeployAnnotationKey] + if fastDeployMode == "" { + fastDeployMode = pkgcfg.FromContext(vmCtx).FastDeployMode } } - // Update the config spec to know about a potential NVRAM file. - for i := range createArgs.ConfigSpec.ExtraConfig { - ov := createArgs.ConfigSpec.ExtraConfig[i].GetOptionValue() - - if ov != nil && ov.Key == "nvram" { - - if v, ok := ov.Value.(string); ok { + if fastDeployMode == pkgconst.FastDeployModeDirect && + !pkgcfg.FromContext(vmCtx).FastDeployExplicitDir { - oldNvramPath := v + return nil, errors.New("fast-deploy+direct not supported") - for j := range dstFilePaths { - if strings.EqualFold(".nvram", path.Ext(dstFilePaths[j])) { - newNvramPath := path.Base(dstFilePaths[j]) - logger.Info("Updated NVRAM in ExtraConfig", - "oldValue", oldNvramPath, - "newValue", newNvramPath) - createArgs.ConfigSpec.ExtraConfig[i] = &vimtypes.OptionValue{ - Key: ov.Key, - Value: newNvramPath, - } - } - } - } - } } - // If any error occurs after this point, the newly created VM directory and - // its contents need to be cleaned up. - defer func() { - if retErr == nil { - // Do not delete the VM directory if this function was successful. - return - } + if pkgcfg.FromContext(vmCtx).FastDeployExplicitDir { + fm := object.NewFileManager(vimClient) - // Use a new context to ensure cleanup happens even if the context - // is cancelled. - ctx := context.Background() + // Create the directory where the VM will be created for direct mode. + if err := fm.MakeDirectory(vmCtx, vmDir, datacenter, true); err != nil { + return nil, fmt.Errorf("failed to create vm dir %q: %w", vmDir, err) + } - // Delete the VM directory and its contents. - t, err := fm.DeleteDatastoreFile(ctx, vmDir, datacenter) - if err != nil { - err = fmt.Errorf( - "failed to call delete api for vm dir %q: %w", vmDir, err) + // If any error occurs after this point, the newly created VM directory and + // its contents need to be cleaned up. + defer func() { if retErr == nil { - retErr = err - } else { - retErr = fmt.Errorf("%w,%w", err, retErr) + // Do not delete the VM directory if this function was successful. + return } - return - } - // Wait for the delete call to return. - if err := t.Wait(ctx); err != nil { - if !fault.Is(err, &vimtypes.FileNotFound{}) { - err = fmt.Errorf("failed to delete vm dir %q: %w", vmDir, err) + // Use a new context to ensure cleanup happens even if the context + // is cancelled. + ctx := context.Background() + + // Delete the VM directory and its contents. + t, err := fm.DeleteDatastoreFile(ctx, vmDir, datacenter) + if err != nil { + err = fmt.Errorf( + "failed to call delete api for vm dir %q: %w", vmDir, err) if retErr == nil { retErr = err } else { retErr = fmt.Errorf("%w,%w", err, retErr) } + return + } + + // Wait for the delete call to return. + if err := t.Wait(ctx); err != nil { + if !fault.Is(err, &vimtypes.FileNotFound{}) { + err = fmt.Errorf("failed to delete vm dir %q: %w", vmDir, err) + if retErr == nil { + retErr = err + } else { + retErr = fmt.Errorf("%w,%w", err, retErr) + } + } + } + }() + + // Copy any non-disk files to the target directory. + for i := range dstFilePaths { + task, err := fm.CopyDatastoreFile( + vmCtx, + srcFilePaths[i], + datacenter, + dstFilePaths[i], + datacenter, false) + if err != nil { + return nil, err + } + if err = task.Wait(vmCtx); err != nil { + return nil, err } } - }() + + // Update the config spec to know about a potential NVRAM file. + for i := range createArgs.ConfigSpec.ExtraConfig { + ov := createArgs.ConfigSpec.ExtraConfig[i].GetOptionValue() + + if ov != nil && ov.Key == "nvram" { + + if v, ok := ov.Value.(string); ok { + + oldNvramPath := v + + for j := range dstFilePaths { + if strings.EqualFold(".nvram", path.Ext(dstFilePaths[j])) { + newNvramPath := path.Base(dstFilePaths[j]) + logger.Info("Updated NVRAM in ExtraConfig", + "oldValue", oldNvramPath, + "newValue", newNvramPath) + createArgs.ConfigSpec.ExtraConfig[i] = &vimtypes.OptionValue{ + Key: ov.Key, + Value: newNvramPath, + } + } + } + } + } + } + } else { //nolint:gocritic + + if len(createArgs.ConfigSpec.ExtraConfig) > 0 { + // ExplicitDir is disabled, so we must make sure to remove a + // potential nvram file definition from the ExtraConfig that may + // have come from the image. + newEC := []vimtypes.BaseOptionValue{} + for _, kvObj := range createArgs.ConfigSpec.ExtraConfig { + kv := kvObj.GetOptionValue() + if kv.Key != "nvram" { + newEC = append(newEC, kvObj) + } + } + createArgs.ConfigSpec.ExtraConfig = newEC + } + } folder := object.NewFolder(vimClient, vimtypes.ManagedObjectReference{ Type: "Folder", @@ -211,17 +249,6 @@ func fastDeploy( logger.Info("Got host", "host", host.Reference()) } - // Determine the type of fast deploy operation. - var fastDeployMode string - - if createArgs.IsEncryptedStorageProfile { - fastDeployMode = pkgconst.FastDeployModeDirect - } else { - fastDeployMode = vmCtx.VM.Annotations[pkgconst.FastDeployAnnotationKey] - if fastDeployMode == "" { - fastDeployMode = pkgcfg.FromContext(vmCtx).FastDeployMode - } - } logger.Info("Deploying VM with Fast Deploy", "mode", fastDeployMode) if strings.EqualFold(fastDeployMode, pkgconst.FastDeployModeLinked) { diff --git a/pkg/providers/vsphere/vmprovider_vm_test.go b/pkg/providers/vsphere/vmprovider_vm_test.go index a6f19391a..cd5a6dd2a 100644 --- a/pkg/providers/vsphere/vmprovider_vm_test.go +++ b/pkg/providers/vsphere/vmprovider_vm_test.go @@ -1742,6 +1742,7 @@ func vmTests() { Expect(enc.Encode(configSpec)).To(Succeed()) vmClass.Spec.ConfigSpec = w.Bytes() + vm.Spec.PowerState = vmopv1.VirtualMachinePowerStateOff }) JustBeforeEach(func() { @@ -1779,24 +1780,96 @@ func vmTests() { true)).To(Succeed()) }) - It("should succeed", func() { - vcVM, err := createOrUpdateAndGetVcVM(ctx, vmProvider, vm) - Expect(err).ToNot(HaveOccurred()) + assertSuccess := func( + expectedToHaveNVRAM bool, + expectKeepDisksKey bool) { + vcVM, err := createOrUpdateAndGetVcVM(ctx, vmProvider, vm) + ExpectWithOffset(1, err).ToNot(HaveOccurred()) var moVM mo.VirtualMachine - Expect(vcVM.Properties( + ExpectWithOffset(1, vcVM.Properties( ctx, vcVM.Reference(), []string{"config.extraConfig"}, &moVM)).To(Succeed()) ec := object.OptionValueList(moVM.Config.ExtraConfig) v1, _ := ec.GetString("hello") - Expect(v1).To(Equal("world")) + ExpectWithOffset(1, v1).To(Equal("world")) v2, _ := ec.GetString("fu") - Expect(v2).To(Equal("bar")) - v3, _ := ec.GetString(pkgconst.VMProvKeepDisksExtraConfigKey) - Expect(v3).To(Equal(path.Base(ctx.ContentLibraryItemDiskPath))) + ExpectWithOffset(1, v2).To(Equal("bar")) + + if expectKeepDisksKey { + v, _ := ec.GetString(pkgconst.VMProvKeepDisksExtraConfigKey) + ExpectWithOffset(1, v).To(Equal(path.Base(ctx.ContentLibraryItemDiskPath))) + } else { + v, ok := ec.GetString(pkgconst.VMProvKeepDisksExtraConfigKey) + ExpectWithOffset(1, ok).To(BeFalse(), v) + } + + nvramFile, hasNVRAM := ec.GetString("nvram") + ExpectWithOffset(1, hasNVRAM).To(Equal(expectedToHaveNVRAM), nvramFile) + } + + When("mode is direct", func() { + JustBeforeEach(func() { + if vm.Annotations == nil { + vm.Annotations = map[string]string{} + } + vm.Annotations[pkgconst.FastDeployAnnotationKey] = pkgconst.FastDeployModeDirect + }) + + When("ExplicitDir is supported", func() { + JustBeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.FastDeployExplicitDir = true + }) + }) + It("should succeed", func() { + assertSuccess(true, false) + }) + }) + When("ExplicitDir is not supported", func() { + JustBeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.FastDeployExplicitDir = false + }) + }) + It("should return an error", func() { + _, err := createOrUpdateAndGetVcVM(ctx, vmProvider, vm) + Expect(err).To(MatchError("fast-deploy+direct not supported")) + }) + }) + }) + + When("mode is linked", func() { + JustBeforeEach(func() { + if vm.Annotations == nil { + vm.Annotations = map[string]string{} + } + vm.Annotations[pkgconst.FastDeployAnnotationKey] = pkgconst.FastDeployModeLinked + }) + When("ExplicitDir is supported", func() { + JustBeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.FastDeployExplicitDir = true + }) + }) + It("should succeed", func() { + assertSuccess(true, true) + }) + }) + When("ExplicitDir is not supported", func() { + JustBeforeEach(func() { + pkgcfg.SetContext(ctx, func(config *pkgcfg.Config) { + config.FastDeployExplicitDir = false + }) + }) + It("should succeed", func() { + assertSuccess(false, true) + }) + }) }) + }) }) diff --git a/pkg/vmconfig/diskpromo/diskpromo_reconciler.go b/pkg/vmconfig/diskpromo/diskpromo_reconciler.go index 63f50748d..652f92366 100644 --- a/pkg/vmconfig/diskpromo/diskpromo_reconciler.go +++ b/pkg/vmconfig/diskpromo/diskpromo_reconciler.go @@ -141,11 +141,15 @@ func (r reconciler) Reconcile( } } - logger = logger.WithValues("mode", vm.Spec.PromoteDisksMode) + promoMode := vm.Spec.PromoteDisksMode + if promoMode == "" { + promoMode = vmopv1.VirtualMachinePromoteDisksModeOnline + } + logger = logger.WithValues("mode", promoMode) logger.V(4).Info("Finding candidates for disk promotion") - if vm.Spec.PromoteDisksMode == vmopv1.VirtualMachinePromoteDisksModeDisabled { + if promoMode == vmopv1.VirtualMachinePromoteDisksModeDisabled { // Skip VMs that do not request promotion. pkgcond.Delete(vm, vmopv1.VirtualMachineDiskPromotionSynced) return nil @@ -221,7 +225,7 @@ func (r reconciler) Reconcile( return nil } - switch vm.Spec.PromoteDisksMode { + switch promoMode { case vmopv1.VirtualMachinePromoteDisksModeOnline: if moVM.Snapshot != nil && moVM.Snapshot.CurrentSnapshot != nil { // Skip VMs that have snapshots.