From 09ac39ab164ec67a6b435caf73f05a7d728cced9 Mon Sep 17 00:00:00 2001 From: akutz Date: Fri, 9 Aug 2024 11:21:38 -0500 Subject: [PATCH] This patch does three things: 1. The VM object's namespace and name are stored in ExtraConfig as the keys vmservice.namespace and vmservice.name. 2. The VM's vSphere property config.managedBy is always set to reflect the VM is managed by VM Service if it is not set to this value. 3. There is now support for always ensuring certain properties are set during each reconcile, regardless of the VM's power state or whether or not the VM is paused by the admin. For example, the aforementioned namespace/name ExtraConfig keys are included, as well as the VM's config.managedBy property. --- pkg/providers/vsphere/constants/constants.go | 12 +- pkg/providers/vsphere/resources/vm.go | 17 +- pkg/providers/vsphere/session/session.go | 8 +- .../vsphere/session/session_vm_update.go | 264 +++++++++++++----- .../vsphere/session/session_vm_update_test.go | 122 ++++++++ .../vsphere/virtualmachine/configspec.go | 13 + pkg/util/resize/configspec.go | 13 - pkg/util/resize/configspec_test.go | 9 - pkg/util/vmopv1/resize_overwrite.go | 102 ++++++- pkg/util/vmopv1/resize_overwrite_test.go | 190 +++++++++++-- pkg/util/vsphere/vm/guest_id.go | 51 +--- pkg/util/vsphere/vm/guest_id_test.go | 175 ++++-------- 12 files changed, 685 insertions(+), 291 deletions(-) diff --git a/pkg/providers/vsphere/constants/constants.go b/pkg/providers/vsphere/constants/constants.go index 24a4d9e59..c13b8fb49 100644 --- a/pkg/providers/vsphere/constants/constants.go +++ b/pkg/providers/vsphere/constants/constants.go @@ -8,11 +8,13 @@ import ( ) const ( - ExtraConfigTrue = "TRUE" - ExtraConfigFalse = "FALSE" - ExtraConfigUnset = "" - ExtraConfigGuestInfoPrefix = "guestinfo." - ExtraConfigRunContainerKey = "RUN.container" + ExtraConfigTrue = "TRUE" + ExtraConfigFalse = "FALSE" + ExtraConfigUnset = "" + ExtraConfigGuestInfoPrefix = "guestinfo." + ExtraConfigRunContainerKey = "RUN.container" + ExtraConfigVMServiceName = "vmservice.name" + ExtraConfigVMServiceNamespace = "vmservice.namespace" // VCVMAnnotation Annotation placed on the VM. VCVMAnnotation = "Virtual Machine managed by the vSphere Virtual Machine service" diff --git a/pkg/providers/vsphere/resources/vm.go b/pkg/providers/vsphere/resources/vm.go index e9ec933a2..34b16f932 100644 --- a/pkg/providers/vsphere/resources/vm.go +++ b/pkg/providers/vsphere/resources/vm.go @@ -4,6 +4,7 @@ package resources import ( + "bytes" "context" "fmt" @@ -75,8 +76,20 @@ func (vm *VirtualMachine) Clone(ctx context.Context, folder *object.Folder, clon return &ref, nil } -func (vm *VirtualMachine) Reconfigure(ctx context.Context, configSpec *vimtypes.VirtualMachineConfigSpec) (*vimtypes.TaskInfo, error) { - vm.logger.V(5).Info("Reconfiguring VM", "configSpec", configSpec) +func (vm *VirtualMachine) Reconfigure( + ctx context.Context, + configSpec *vimtypes.VirtualMachineConfigSpec) (*vimtypes.TaskInfo, error) { + + logger := logr.FromContextOrDiscard(ctx) + + var w bytes.Buffer + enc := vimtypes.NewJSONEncoder(&w) + if err := enc.Encode(configSpec); err != nil { + logger.Error(err, "Failed to marshal ConfigSpec to JSON") + logger.Info("Reconfiguring VM", "configSpec", configSpec) + } else { + logger.Info("Reconfiguring VM", "configSpec", w.String()) + } reconfigureTask, err := vm.vcVirtualMachine.Reconfigure(ctx, *configSpec) if err != nil { diff --git a/pkg/providers/vsphere/session/session.go b/pkg/providers/vsphere/session/session.go index 7d8cc44a7..07a559a45 100644 --- a/pkg/providers/vsphere/session/session.go +++ b/pkg/providers/vsphere/session/session.go @@ -4,6 +4,8 @@ package session import ( + "fmt" + "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -28,13 +30,11 @@ func (s *Session) invokeFsrVirtualMachine(vmCtx pkgctx.VirtualMachineContext, re task, err := internal.VirtualMachineFSR(vmCtx, resVM.MoRef(), s.Client.VimClient()) if err != nil { - vmCtx.Logger.Error(err, "InvokeFSR call failed") - return err + return fmt.Errorf("failed to invoke FSR: %w", err) } if err = task.Wait(vmCtx); err != nil { - vmCtx.Logger.Error(err, "InvokeFSR task failed") - return err + return fmt.Errorf("failed to wait on FSR task: %w", err) } return nil diff --git a/pkg/providers/vsphere/session/session_vm_update.go b/pkg/providers/vsphere/session/session_vm_update.go index 82e3ea20c..6cd322935 100644 --- a/pkg/providers/vsphere/session/session_vm_update.go +++ b/pkg/providers/vsphere/session/session_vm_update.go @@ -4,7 +4,6 @@ package session import ( - "bytes" "context" "fmt" "maps" @@ -21,6 +20,7 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" vmopv1common "github.com/vmware-tanzu/vm-operator/api/v1alpha3/common" "github.com/vmware-tanzu/vm-operator/pkg" + "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" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/clustermodules" @@ -423,17 +423,6 @@ func UpdateConfigSpecAnnotation( } } -func UpdateConfigSpecManagedBy( - config *vimtypes.VirtualMachineConfigInfo, - configSpec *vimtypes.VirtualMachineConfigSpec) { - if config.ManagedBy == nil { - configSpec.ManagedBy = &vimtypes.ManagedByInfo{ - ExtensionKey: vmopv1.ManagedByExtensionKey, - Type: vmopv1.ManagedByExtensionType, - } - } -} - // UpdateConfigSpecGuestID sets the given vmSpecGuestID in the ConfigSpec if it // is not empty and different from the current GuestID in the VM's ConfigInfo. func UpdateConfigSpecGuestID( @@ -462,16 +451,24 @@ func UpdateConfigSpecFirmware( func updateConfigSpec( vmCtx pkgctx.VirtualMachineContext, config *vimtypes.VirtualMachineConfigInfo, - updateArgs *VMUpdateArgs) *vimtypes.VirtualMachineConfigSpec { + updateArgs *VMUpdateArgs) (*vimtypes.VirtualMachineConfigSpec, error) { configSpec := &vimtypes.VirtualMachineConfigSpec{} vmClassSpec := updateArgs.VMClass.Spec - UpdateConfigSpecAnnotation(config, configSpec) - UpdateConfigSpecManagedBy(config, configSpec) + if err := vmopv1util.OverwriteAlwaysResizeConfigSpec( + vmCtx, + *vmCtx.VM, + *config, + configSpec); err != nil { + + return nil, err + } + UpdateConfigSpecExtraConfig( vmCtx, config, configSpec, &updateArgs.ConfigSpec, &vmClassSpec, vmCtx.VM, updateArgs.ExtraConfig) + UpdateConfigSpecAnnotation(config, configSpec) UpdateConfigSpecChangeBlockTracking( vmCtx, config, configSpec, &updateArgs.ConfigSpec, vmCtx.VM.Spec) UpdateConfigSpecFirmware(config, configSpec, vmCtx.VM) @@ -483,7 +480,7 @@ func updateConfigSpec( resize.CompareMemoryAllocation(*config, updateArgs.ConfigSpec, configSpec) } - return configSpec + return configSpec, nil } func (s *Session) prePowerOnVMConfigSpec( @@ -491,7 +488,10 @@ func (s *Session) prePowerOnVMConfigSpec( config *vimtypes.VirtualMachineConfigInfo, updateArgs *VMUpdateArgs) (*vimtypes.VirtualMachineConfigSpec, error) { - configSpec := updateConfigSpec(vmCtx, config, updateArgs) + configSpec, err := updateConfigSpec(vmCtx, config, updateArgs) + if err != nil { + return nil, err + } virtualDevices := object.VirtualDeviceList(config.Hardware.Device) currentDisks := virtualDevices.SelectByType((*vimtypes.VirtualDisk)(nil)) @@ -557,23 +557,16 @@ func (s *Session) prePowerOnVMReconfigure( return err } - defaultConfigSpec := &vimtypes.VirtualMachineConfigSpec{} - if !apiEquality.Semantic.DeepEqual(configSpec, defaultConfigSpec) { - var w bytes.Buffer - enc := vimtypes.NewJSONEncoder(&w) - if err := enc.Encode(configSpec); err != nil { - vmCtx.Logger.Error(err, "Failed to marshal ConfigSpec to JSON") - vmCtx.Logger.Info("Pre PowerOn Reconfigure", "configSpec", configSpec) - } else { - vmCtx.Logger.Info("Pre PowerOn Reconfigure", "configSpec", w.String()) - } + if _, err := doReconfigure( + logr.NewContext( + vmCtx, + vmCtx.Logger.WithName("prePowerOnVMReconfigure"), + ), + vmCtx.VM, + resVM.VcVM(), + *configSpec); err != nil { - taskInfo, err := resVM.Reconfigure(vmCtx, configSpec) - vmutil.UpdateVMGuestIDReconfiguredCondition(vmCtx, *configSpec, taskInfo) - if err != nil { - vmCtx.Logger.Error(err, "pre power on reconfigure failed") - return err - } + return err } if features.VMResize || features.VMResizeCPUMemory { @@ -789,7 +782,16 @@ func (s *Session) poweredOnVMReconfigure( configSpec := &vimtypes.VirtualMachineConfigSpec{} - UpdateConfigSpecExtraConfig(vmCtx, config, configSpec, nil, nil, nil, nil) + if err := vmopv1util.OverwriteAlwaysResizeConfigSpec( + vmCtx, + *vmCtx.VM, + *config, + configSpec); err != nil { + + return false, err + } + + UpdateConfigSpecExtraConfig(vmCtx, config, configSpec, nil, nil, vmCtx.VM, nil) UpdateConfigSpecChangeBlockTracking(vmCtx, config, configSpec, nil, vmCtx.VM.Spec) if pkgcfg.FromContext(vmCtx).Features.IsoSupport { @@ -798,27 +800,26 @@ func (s *Session) poweredOnVMReconfigure( } } - var refetchProps bool + refetchProps, err := doReconfigure( + logr.NewContext( + vmCtx, + vmCtx.Logger.WithName("poweredOnVMReconfigure"), + ), + vmCtx.VM, + resVM.VcVM(), + *configSpec) - defaultConfigSpec := &vimtypes.VirtualMachineConfigSpec{} - if !apiEquality.Semantic.DeepEqual(configSpec, defaultConfigSpec) { - vmCtx.Logger.Info("PoweredOn Reconfigure", "configSpec", configSpec) - if _, err := resVM.Reconfigure(vmCtx, configSpec); err != nil { - vmCtx.Logger.Error(err, "powered on reconfigure failed") - return false, err - } + if err != nil { + return false, err + } - // Special case for CBT: in order for CBT change take effect for a powered on VM, - // a checkpoint save/restore is needed. tracks the implementation of - // this FSR internally to vSphere. - if configSpec.ChangeTrackingEnabled != nil { - if err := s.invokeFsrVirtualMachine(vmCtx, resVM); err != nil { - vmCtx.Logger.Error(err, "Failed to invoke FSR for CBT update") - return false, err - } + // Special case for CBT: in order for CBT change take effect for a powered + // on VM, a checkpoint save/restore is needed. The FSR call allows CBT to + // take effect for powered-on VMs. + if configSpec.ChangeTrackingEnabled != nil { + if err := s.invokeFsrVirtualMachine(vmCtx, resVM); err != nil { + return refetchProps, fmt.Errorf("failed to invoke FSR for CBT update") } - - refetchProps = true } return refetchProps, nil @@ -874,21 +875,34 @@ func (s *Session) resizeVMWhenPoweredStateOff( } if pkgcfg.FromContext(vmCtx).Features.VMResize { - err := vmopv1util.OverwriteResizeConfigSpec(vmCtx, *vmCtx.VM, *moVM.Config, &configSpec) - if err != nil { + if err := vmopv1util.OverwriteResizeConfigSpec( + vmCtx, + *vmCtx.VM, + *moVM.Config, + &configSpec); err != nil { + return false, err } + } else if err := vmopv1util.OverwriteAlwaysResizeConfigSpec( + vmCtx, + *vmCtx.VM, + *moVM.Config, + &configSpec); err != nil { + + return false, err } - refetchProps := false - if !reflect.DeepEqual(configSpec, vimtypes.VirtualMachineConfigSpec{}) { - vmCtx.Logger.Info("Powered off resizing", "configSpec", configSpec) - if _, err := res.NewVMFromObject(vcVM).Reconfigure(vmCtx, &configSpec); err != nil { - vmCtx.Logger.Error(err, "powered off resize reconfigure failed") - return false, err - } + refetchProps, err := doReconfigure( + logr.NewContext( + vmCtx, + vmCtx.Logger.WithName("resizeVMWhenPoweredStateOff"), + ), + vmCtx.VM, + vcVM, + configSpec) - refetchProps = true + if err != nil { + return false, err } if needsResize { @@ -934,9 +948,13 @@ func (s *Session) updateVMDesiredPowerStateOff( vmCtx pkgctx.VirtualMachineContext, vcVM *object.VirtualMachine, getResizeArgsFn func() (*VMResizeArgs, error), - existingPowerState vmopv1.VirtualMachinePowerState) (refetchProps bool, err error) { + existingPowerState vmopv1.VirtualMachinePowerState) (bool, error) { + + var ( + powerOff bool + refetchProps bool + ) - var powerOff bool if existingPowerState == vmopv1.VirtualMachinePowerStateOn { powerOff = true } else if existingPowerState == vmopv1.VirtualMachinePowerStateSuspended { @@ -944,13 +962,13 @@ func (s *Session) updateVMDesiredPowerStateOff( vmCtx.VM.Spec.PowerOffMode == vmopv1.VirtualMachinePowerOpModeTrySoft } if powerOff { - err = res.NewVMFromObject(vcVM).SetPowerState( + if err := res.NewVMFromObject(vcVM).SetPowerState( logr.NewContext(vmCtx, vmCtx.Logger), existingPowerState, vmCtx.VM.Spec.PowerState, - vmCtx.VM.Spec.PowerOffMode) - if err != nil { - return refetchProps, err + vmCtx.VM.Spec.PowerOffMode); err != nil { + + return false, err } refetchProps = true @@ -971,7 +989,7 @@ func (s *Session) updateVMDesiredPowerStateOff( } if f := pkgcfg.FromContext(vmCtx).Features; f.VMResize || f.VMResizeCPUMemory { - refetchProps, err = s.resizeVMWhenPoweredStateOff( + refetch, err := s.resizeVMWhenPoweredStateOff( vmCtx, vcVM, vmCtx.MoVM, @@ -979,6 +997,17 @@ func (s *Session) updateVMDesiredPowerStateOff( if err != nil { return refetchProps, err } + if refetch { + refetchProps = true + } + } else { + refetch, err := defaultReconfigure(vmCtx, vcVM, *vmCtx.MoVM.Config) + if err != nil { + return refetchProps, err + } + if refetch { + refetchProps = true + } } return refetchProps, err @@ -987,22 +1016,33 @@ func (s *Session) updateVMDesiredPowerStateOff( func (s *Session) updateVMDesiredPowerStateSuspended( vmCtx pkgctx.VirtualMachineContext, vcVM *object.VirtualMachine, - existingPowerState vmopv1.VirtualMachinePowerState) (refetchProps bool, err error) { + existingPowerState vmopv1.VirtualMachinePowerState) (bool, error) { + + var ( + refetchProps bool + ) if existingPowerState == vmopv1.VirtualMachinePowerStateOn { - err = res.NewVMFromObject(vcVM).SetPowerState( + if err := res.NewVMFromObject(vcVM).SetPowerState( logr.NewContext(vmCtx, vmCtx.Logger), existingPowerState, vmCtx.VM.Spec.PowerState, - vmCtx.VM.Spec.SuspendMode) - if err != nil { - return refetchProps, err + vmCtx.VM.Spec.SuspendMode); err != nil { + return false, err } refetchProps = true } - return + refetch, err := defaultReconfigure(vmCtx, vcVM, *vmCtx.MoVM.Config) + if err != nil { + return refetchProps, err + } + if refetch { + refetchProps = true + } + + return refetchProps, nil } func (s *Session) updateVMDesiredPowerStateOn( @@ -1168,6 +1208,7 @@ func (s *Session) UpdateVirtualMachine( } } else { vmCtx.Logger.Info("VirtualMachine is paused. PowerState is not updated.") + refetchProps, err = defaultReconfigure(vmCtx, vcVM, *vmCtx.MoVM.Config) } if refetchProps { @@ -1215,3 +1256,76 @@ func isVMPaused(vmCtx pkgctx.VirtualMachineContext) bool { delete(vm.Labels, vmopv1.PausedVMLabelKey) return false } + +func defaultReconfigure( + vmCtx pkgctx.VirtualMachineContext, + vcVM *object.VirtualMachine, + configInfo vimtypes.VirtualMachineConfigInfo) (bool, error) { + + var configSpec vimtypes.VirtualMachineConfigSpec + if err := vmopv1util.OverwriteAlwaysResizeConfigSpec( + vmCtx, + *vmCtx.VM, + configInfo, + &configSpec); err != nil { + + return false, err + } + + return doReconfigure( + logr.NewContext( + vmCtx, + vmCtx.Logger.WithName("defaultReconfigure"), + ), + vmCtx.VM, + vcVM, + configSpec) +} + +func doReconfigure( + ctx context.Context, + vm *vmopv1.VirtualMachine, + vcVM *object.VirtualMachine, + configSpec vimtypes.VirtualMachineConfigSpec) (bool, error) { + + var defaultConfigSpec vimtypes.VirtualMachineConfigSpec + if apiEquality.Semantic.DeepEqual(configSpec, defaultConfigSpec) { + return false, nil + } + + resVM := res.NewVMFromObject(vcVM) + taskInfo, err := resVM.Reconfigure(ctx, &configSpec) + + UpdateVMGuestIDReconfiguredCondition(vm, configSpec, taskInfo) + + if err != nil { + return false, err + } + + return true, err +} + +// UpdateVMGuestIDReconfiguredCondition deletes the VM's GuestIDReconfigured +// condition if the configSpec doesn't contain a guestID, or if the taskInfo +// does not contain an invalid guestID property error. Otherwise, it sets the +// condition to false with the invalid guest ID property value in the reason. +func UpdateVMGuestIDReconfiguredCondition( + vm *vmopv1.VirtualMachine, + configSpec vimtypes.VirtualMachineConfigSpec, + taskInfo *vimtypes.TaskInfo) { + + if configSpec.GuestId != "" && vmutil.IsTaskInfoErrorInvalidGuestID(taskInfo) { + conditions.MarkFalse( + vm, + vmopv1.GuestIDReconfiguredCondition, + "Invalid", + fmt.Sprintf( + "The specified guest ID value is not supported: %s", + configSpec.GuestId, + ), + ) + return + } + + conditions.Delete(vm, vmopv1.GuestIDReconfiguredCondition) +} diff --git a/pkg/providers/vsphere/session/session_vm_update_test.go b/pkg/providers/vsphere/session/session_vm_update_test.go index b59b6eb5a..ac0f49534 100644 --- a/pkg/providers/vsphere/session/session_vm_update_test.go +++ b/pkg/providers/vsphere/session/session_vm_update_test.go @@ -1709,3 +1709,125 @@ var _ = Describe("UpdateVirtualMachine", func() { }) }) }) + +var _ = Describe("UpdateVMGuestIDReconfiguredCondition", func() { + + var ( + vmCtx pkgctx.VirtualMachineContext + vmopVM vmopv1.VirtualMachine + configSpec vimtypes.VirtualMachineConfigSpec + taskInfo *vimtypes.TaskInfo + ) + + BeforeEach(func() { + // Init VM with the condition set to verify it's actually deleted. + vmopVM = vmopv1.VirtualMachine{ + Status: vmopv1.VirtualMachineStatus{ + Conditions: []metav1.Condition{ + { + Type: vmopv1.GuestIDReconfiguredCondition, + Status: metav1.ConditionFalse, + }, + }, + }, + } + vmCtx = pkgctx.VirtualMachineContext{ + VM: &vmopVM, + } + configSpec = vimtypes.VirtualMachineConfigSpec{} + taskInfo = &vimtypes.TaskInfo{} + }) + + JustBeforeEach(func() { + session.UpdateVMGuestIDReconfiguredCondition(vmCtx.VM, configSpec, taskInfo) + }) + + Context("ConfigSpec doesn't have a guest ID", func() { + + BeforeEach(func() { + configSpec.GuestId = "" + }) + + It("should delete the existing VM's guest ID condition", func() { + Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) + }) + }) + + Context("ConfigSpec has a guest ID", func() { + + BeforeEach(func() { + configSpec.GuestId = "test-guest-id-value" + }) + + When("TaskInfo is nil", func() { + + BeforeEach(func() { + taskInfo = nil + }) + + It("should delete the VM's guest ID condition", func() { + Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) + }) + }) + + When("TaskInfo.Error is nil", func() { + + BeforeEach(func() { + taskInfo.Error = nil + }) + + It("should delete the VM's guest ID condition", func() { + Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) + }) + }) + + When("TaskInfo.Error.Fault is not an InvalidPropertyFault", func() { + + BeforeEach(func() { + taskInfo.Error = &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.InvalidName{ + Name: "some-invalid-name", + }, + } + }) + + It("should delete the VM's guest ID condition", func() { + Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) + }) + }) + + When("TaskInfo.Error contains an invalid property error NOT about guestID", func() { + + BeforeEach(func() { + taskInfo.Error = &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.InvalidArgument{ + InvalidProperty: "config.version", + }, + } + }) + + It("should delete the VM's guest ID condition", func() { + Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) + }) + }) + + When("TaskInfo.Error contains an invalid property error of guestID", func() { + + BeforeEach(func() { + taskInfo.Error = &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.InvalidArgument{ + InvalidProperty: "configSpec.guestId", + }, + } + }) + + It("should set the VM's guest ID condition to false with the invalid value in the reason", func() { + c := conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition) + Expect(c).NotTo(BeNil()) + Expect(c.Status).To(Equal(metav1.ConditionFalse)) + Expect(c.Reason).To(Equal("Invalid")) + Expect(c.Message).To(Equal("The specified guest ID value is not supported: test-guest-id-value")) + }) + }) + }) +}) diff --git a/pkg/providers/vsphere/virtualmachine/configspec.go b/pkg/providers/vsphere/virtualmachine/configspec.go index 6d37766d6..efa9c10dc 100644 --- a/pkg/providers/vsphere/virtualmachine/configspec.go +++ b/pkg/providers/vsphere/virtualmachine/configspec.go @@ -42,6 +42,19 @@ func CreateConfigSpec( Type: vmopv1.ManagedByExtensionType, } + // Ensure ExtraConfig contains the name/namespace of the VM's Kubernetes + // resource. + configSpec.ExtraConfig = util.OptionValues(configSpec.ExtraConfig).Merge( + &vimtypes.OptionValue{ + Key: constants.ExtraConfigVMServiceName, + Value: vmCtx.VM.Name, + }, + &vimtypes.OptionValue{ + Key: constants.ExtraConfigVMServiceNamespace, + Value: vmCtx.VM.Namespace, + }, + ) + // spec.biosUUID is only set when creating a VM and is immutable. // This field should not be updated for existing VMs. if id := vmCtx.VM.Spec.BiosUUID; id != "" { diff --git a/pkg/util/resize/configspec.go b/pkg/util/resize/configspec.go index 555803a6f..ed8e1f5b4 100644 --- a/pkg/util/resize/configspec.go +++ b/pkg/util/resize/configspec.go @@ -25,7 +25,6 @@ func CreateResizeConfigSpec( outCS := vimtypes.VirtualMachineConfigSpec{} compareAnnotation(ci, cs, &outCS) - compareManagedBy(ci, cs, &outCS) compareHardware(ci, cs, &outCS) CompareCPUAllocation(ci, cs, &outCS) compareCPUHotAddOrRemove(ci, cs, &outCS) @@ -82,18 +81,6 @@ func compareAnnotation( } } -// compareManagedBy compares the ConfigInfo.ManagedBy. -func compareManagedBy( - ci vimtypes.VirtualMachineConfigInfo, - cs vimtypes.VirtualMachineConfigSpec, - outCS *vimtypes.VirtualMachineConfigSpec) { - - if ci.ManagedBy == nil { - // Only change the ManagedBy if it is currently unset. - outCS.ManagedBy = cs.ManagedBy - } -} - // compareHardware compares the ConfigInfo.Hardware. func compareHardware( ci vimtypes.VirtualMachineConfigInfo, diff --git a/pkg/util/resize/configspec_test.go b/pkg/util/resize/configspec_test.go index fee61b1ae..41d39d6d6 100644 --- a/pkg/util/resize/configspec_test.go +++ b/pkg/util/resize/configspec_test.go @@ -49,15 +49,6 @@ var _ = Describe("CreateResizeConfigSpec", func() { ConfigSpec{Annotation: "my-annotation"}, ConfigSpec{Annotation: "my-annotation"}), - Entry("ManagedBy is currently set", - ConfigInfo{ManagedBy: &vimtypes.ManagedByInfo{Type: "my-managed-by"}}, - ConfigSpec{}, - ConfigSpec{}), - Entry("ManagedBy is currently unset", - ConfigInfo{}, - ConfigSpec{ManagedBy: &vimtypes.ManagedByInfo{Type: "my-managed-by"}}, - ConfigSpec{ManagedBy: &vimtypes.ManagedByInfo{Type: "my-managed-by"}}), - Entry("NumCPUs needs updating", ConfigInfo{Hardware: vimtypes.VirtualHardware{NumCPU: 2}}, ConfigSpec{NumCPUs: 4}, diff --git a/pkg/util/vmopv1/resize_overwrite.go b/pkg/util/vmopv1/resize_overwrite.go index 6431efbb5..d2fe20578 100644 --- a/pkg/util/vmopv1/resize_overwrite.go +++ b/pkg/util/vmopv1/resize_overwrite.go @@ -18,11 +18,15 @@ import ( // the current VM state to the ConfigSpec. These are fields that we can change without the // VM Class. func OverwriteResizeConfigSpec( - _ context.Context, + ctx context.Context, vm vmopv1.VirtualMachine, ci vimtypes.VirtualMachineConfigInfo, cs *vimtypes.VirtualMachineConfigSpec) error { + if err := OverwriteAlwaysResizeConfigSpec(ctx, vm, ci, cs); err != nil { + return err + } + if adv := vm.Spec.Advanced; adv != nil { ptr.OverwriteWithUser(&cs.ChangeTrackingEnabled, adv.ChangeBlockTracking, ci.ChangeTrackingEnabled) } @@ -33,6 +37,21 @@ func OverwriteResizeConfigSpec( return nil } +// OverwriteAlwaysResizeConfigSpec applies any set fields in the VM Spec or +// changes required from the current VM state to the ConfigSpec. These are +// fields that change without the VM Class. +func OverwriteAlwaysResizeConfigSpec( + _ context.Context, + vm vmopv1.VirtualMachine, + ci vimtypes.VirtualMachineConfigInfo, + cs *vimtypes.VirtualMachineConfigSpec) error { + + overwriteManagedBy(vm, ci, cs) + overwriteExtraConfigNamespaceName(vm, ci, cs) + + return nil +} + func overwriteGuestID( vm vmopv1.VirtualMachine, ci vimtypes.VirtualMachineConfigInfo, @@ -53,6 +72,7 @@ func overwriteExtraConfig( var toMerge []vimtypes.BaseOptionValue + toMerge = append(toMerge, ensureNamespaceName(vm, ci, cs)...) toMerge = append(toMerge, overrideMMIOSize(vm, ci, cs)...) toMerge = append(toMerge, clearMMPowerOffEC(vm, ci, cs)...) toMerge = append(toMerge, updateV1Alpha1CompatibleEC(vm, ci, cs)...) @@ -60,6 +80,86 @@ func overwriteExtraConfig( cs.ExtraConfig = util.OptionValues(cs.ExtraConfig).Merge(toMerge...) } +func overwriteExtraConfigNamespaceName( + vm vmopv1.VirtualMachine, + ci vimtypes.VirtualMachineConfigInfo, + cs *vimtypes.VirtualMachineConfigSpec) { + + var toMerge []vimtypes.BaseOptionValue + + toMerge = append(toMerge, ensureNamespaceName(vm, ci, cs)...) + + cs.ExtraConfig = util.OptionValues(cs.ExtraConfig).Merge(toMerge...) +} + +func overwriteManagedBy( + _ vmopv1.VirtualMachine, + ci vimtypes.VirtualMachineConfigInfo, + cs *vimtypes.VirtualMachineConfigSpec) { + + var current vimtypes.ManagedByInfo + if ci.ManagedBy != nil { + current = *ci.ManagedBy + } + + if cs.ManagedBy == nil { + cs.ManagedBy = &vimtypes.ManagedByInfo{} + } + + user := vimtypes.ManagedByInfo{ + ExtensionKey: vmopv1.ManagedByExtensionKey, + Type: vmopv1.ManagedByExtensionType, + } + + overwrite(cs.ManagedBy, user, current) + + var empty vimtypes.ManagedByInfo + if *cs.ManagedBy == empty { + cs.ManagedBy = nil + } +} + +func ensureNamespaceName( + vm vmopv1.VirtualMachine, + ci vimtypes.VirtualMachineConfigInfo, + cs *vimtypes.VirtualMachineConfigSpec) []vimtypes.BaseOptionValue { + + outEC := []vimtypes.BaseOptionValue{} + curEC := util.OptionValues(ci.ExtraConfig).StringMap() + inEC := util.OptionValues(cs.ExtraConfig).StringMap() + + fn := func(key string, expectedVal string) { + // Does the VM have the key set in EC? + if v, ok := curEC[key]; ok { + if v == expectedVal { + // The key is present and correct; is the ConfigSpec trying to + // set it again? + if _, ok := inEC[key]; ok { + // Remove the entry from the ConfigSpec. + cs.ExtraConfig = util.OptionValues(cs.ExtraConfig).Delete(key) + } + } else { + // The key is present but incorrect. + outEC = append(outEC, &vimtypes.OptionValue{ + Key: key, + Value: expectedVal, + }) + } + } else { + // The key is not present. + outEC = append(outEC, &vimtypes.OptionValue{ + Key: key, + Value: expectedVal, + }) + } + } + + fn(constants.ExtraConfigVMServiceNamespace, vm.Namespace) + fn(constants.ExtraConfigVMServiceName, vm.Name) + + return outEC +} + func overrideMMIOSize( vm vmopv1.VirtualMachine, ci vimtypes.VirtualMachineConfigInfo, diff --git a/pkg/util/vmopv1/resize_overwrite_test.go b/pkg/util/vmopv1/resize_overwrite_test.go index c7c84e969..33f11ed38 100644 --- a/pkg/util/vmopv1/resize_overwrite_test.go +++ b/pkg/util/vmopv1/resize_overwrite_test.go @@ -15,15 +15,92 @@ import ( vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" "github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/constants" + pkgutil "github.com/vmware-tanzu/vm-operator/pkg/util" vmopv1util "github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1" "github.com/vmware-tanzu/vm-operator/test/builder" ) -type ConfigSpec = vimtypes.VirtualMachineConfigSpec -type ConfigInfo = vimtypes.VirtualMachineConfigInfo - var _ = Describe("OverwriteResizeConfigSpec", func() { + type ConfigSpec = vimtypes.VirtualMachineConfigSpec + type ConfigInfo = vimtypes.VirtualMachineConfigInfo + + configInfoManagedBy := func(ci ConfigInfo, args ...string) ConfigInfo { + var ( + exKey = vmopv1.ManagedByExtensionKey + exType = vmopv1.ManagedByExtensionType + ) + + if len(args) > 0 { + exKey = args[0] + } + if len(args) > 1 { + exType = args[1] + } + + ci.ManagedBy = &vimtypes.ManagedByInfo{ + ExtensionKey: exKey, + Type: exType, + } + + return ci + } + + //nolint:unparam + configSpecManagedBy := func(cs ConfigSpec, args ...string) ConfigSpec { + var ( + exKey = vmopv1.ManagedByExtensionKey + exType = vmopv1.ManagedByExtensionType + ) + + if len(args) > 0 { + exKey = args[0] + } + if len(args) > 1 { + exType = args[1] + } + + cs.ManagedBy = &vimtypes.ManagedByInfo{ + ExtensionKey: exKey, + Type: exType, + } + + return cs + } + + configInfoNamespaceName := func(ci ConfigInfo, args ...string) ConfigInfo { + var ( + namespace string + name string + ) + + if len(args) > 0 { + namespace = args[0] + } + if len(args) > 1 { + name = args[1] + } + + ci.ExtraConfig = pkgutil.OptionValues(ci.ExtraConfig).Merge( + &vimtypes.OptionValue{ + Key: constants.ExtraConfigVMServiceNamespace, + Value: namespace, + }, + &vimtypes.OptionValue{ + Key: constants.ExtraConfigVMServiceName, + Value: name, + }, + ) + return ci + } + + configInfoWithNamespaceName := func() ConfigInfo { + return configInfoNamespaceName(ConfigInfo{}) + } + configInfoWithManagedByAndNamespaceName := func() ConfigInfo { + return configInfoManagedBy(configInfoWithNamespaceName()) + } + ctx := context.Background() truePtr, falsePtr := vimtypes.NewBool(true), vimtypes.NewBool(false) @@ -51,71 +128,96 @@ var _ = Describe("OverwriteResizeConfigSpec", func() { Entry("Empty VM", vmopv1.VirtualMachine{}, - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{}, ConfigSpec{}), - Entry("CBT not set in VM Spec but in ConfigSpec", vmAdvSpec(vmopv1.VirtualMachineAdvancedSpec{}), - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{ChangeTrackingEnabled: truePtr}, ConfigSpec{ChangeTrackingEnabled: truePtr}), Entry("CBT set in VM Spec takes precedence over ConfigSpec", vmAdvSpec(vmopv1.VirtualMachineAdvancedSpec{ChangeBlockTracking: falsePtr}), - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{ChangeTrackingEnabled: truePtr}, ConfigSpec{ChangeTrackingEnabled: falsePtr}), Entry("CBT set in VM Spec but not in ConfigSpec", vmAdvSpec(vmopv1.VirtualMachineAdvancedSpec{ChangeBlockTracking: truePtr}), - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{}, ConfigSpec{ChangeTrackingEnabled: truePtr}), Entry("CBT set in VM Spec with same value in ConfigInfo", vmAdvSpec(vmopv1.VirtualMachineAdvancedSpec{ChangeBlockTracking: truePtr}), - ConfigInfo{ChangeTrackingEnabled: truePtr}, + configInfoManagedBy(configInfoNamespaceName(ConfigInfo{ChangeTrackingEnabled: truePtr})), ConfigSpec{}, ConfigSpec{}), Entry("CBT set in ConfigSpec with same value in ConfigInfo", vmAdvSpec(vmopv1.VirtualMachineAdvancedSpec{}), - ConfigInfo{ChangeTrackingEnabled: truePtr}, + configInfoManagedBy(configInfoNamespaceName(ConfigInfo{ChangeTrackingEnabled: truePtr})), ConfigSpec{ChangeTrackingEnabled: truePtr}, ConfigSpec{}), Entry("CBT set in VM Spec with same value in ConfigInfo but different value in ConfigSpec", vmAdvSpec(vmopv1.VirtualMachineAdvancedSpec{ChangeBlockTracking: truePtr}), - ConfigInfo{ChangeTrackingEnabled: truePtr}, + configInfoManagedBy(configInfoNamespaceName(ConfigInfo{ChangeTrackingEnabled: truePtr})), ConfigSpec{ChangeTrackingEnabled: falsePtr}, ConfigSpec{}), Entry("Guest not set in VM Spec but in ConfigSpec is ignored", vmGuestID(""), - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{GuestId: "foo"}, ConfigSpec{}), Entry("GuestID set in VM Spec takes precedence over ConfigSpec", vmGuestID("foo"), - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{GuestId: "bar"}, ConfigSpec{GuestId: "foo"}), Entry("GuestID set in VM Spec but not in ConfigSpec", vmGuestID("foo"), - ConfigInfo{}, + configInfoWithManagedByAndNamespaceName(), ConfigSpec{}, ConfigSpec{GuestId: "foo"}), Entry("GuestID set in VM Spec with same value in ConfigInfo", vmGuestID("foo"), - ConfigInfo{GuestId: "foo"}, + configInfoManagedBy(configInfoNamespaceName(ConfigInfo{GuestId: "foo"})), ConfigSpec{}, ConfigSpec{}), Entry("GuestID set in ConfigSpec with same value in ConfigInfo", vmGuestID(""), - ConfigInfo{GuestId: "foo"}, + configInfoManagedBy(configInfoNamespaceName(ConfigInfo{GuestId: "foo"})), ConfigSpec{GuestId: "foo"}, ConfigSpec{}), Entry("GuestID set in VM Spec with same value in ConfigInfo but different value in ConfigSpec", vmGuestID("foo"), - ConfigInfo{GuestId: "foo"}, + configInfoManagedBy(configInfoNamespaceName(ConfigInfo{GuestId: "foo"})), ConfigSpec{GuestId: "bar"}, ConfigSpec{}), + + Entry("ManagedBy not set in ConfigInfo or ConfigSpec", + vmopv1.VirtualMachine{}, + configInfoWithNamespaceName(), + ConfigSpec{}, + configSpecManagedBy(ConfigSpec{})), + Entry("ManagedBy set in ConfigInfo and not in ConfigSpec", + vmopv1.VirtualMachine{}, + configInfoWithManagedByAndNamespaceName(), + ConfigSpec{}, + ConfigSpec{}), + Entry("ManagedBy set in ConfigInfo with same value in ConfigSpec", + vmopv1.VirtualMachine{}, + configInfoWithManagedByAndNamespaceName(), + configSpecManagedBy(ConfigSpec{}), + ConfigSpec{}), + Entry("ManagedBy set in ConfigInfo with wrong value in ConfigSpec", + vmopv1.VirtualMachine{}, + configInfoWithManagedByAndNamespaceName(), + configSpecManagedBy(ConfigSpec{}, "fake", "fake"), + ConfigSpec{}), + Entry("ManagedBy not set in ConfigInfo with wrong value in ConfigSpec", + vmopv1.VirtualMachine{}, + configInfoWithNamespaceName(), + configSpecManagedBy(ConfigSpec{}, "fake", "fake"), + configSpecManagedBy(ConfigSpec{})), ) Context("ExtraConfig", func() { @@ -123,12 +225,15 @@ var _ = Describe("OverwriteResizeConfigSpec", func() { vm vmopv1.VirtualMachine ci ConfigInfo cs ConfigSpec + + nameVal = &vimtypes.OptionValue{Key: constants.ExtraConfigVMServiceName, Value: vm.Name} + namespaceVal = &vimtypes.OptionValue{Key: constants.ExtraConfigVMServiceNamespace, Value: vm.Namespace} ) BeforeEach(func() { vm = *builder.DummyVirtualMachine() - ci = ConfigInfo{} + ci = configInfoWithNamespaceName() cs = ConfigSpec{} }) @@ -137,6 +242,55 @@ var _ = Describe("OverwriteResizeConfigSpec", func() { Expect(err).ToNot(HaveOccurred()) }) + Context("Namespace and name", func() { + BeforeEach(func() { + ci.ExtraConfig = nil + }) + + When("VM already has expected EC values", func() { + BeforeEach(func() { + ci.ExtraConfig = append(ci.ExtraConfig, nameVal, namespaceVal) + }) + It("no updates", func() { + Expect(cs.ExtraConfig).To(BeEmpty()) + }) + }) + When("VM ConfigSpec already has expected EC values", func() { + BeforeEach(func() { + cs.ExtraConfig = append(cs.ExtraConfig, nameVal, namespaceVal) + }) + It("same changes", func() { + Expect(cs.ExtraConfig).To(ConsistOf(nameVal, namespaceVal)) + }) + }) + When("VM has none of expected EC values", func() { + It("adds it", func() { + Expect(cs.ExtraConfig).To(ConsistOf(nameVal, namespaceVal)) + }) + }) + When("VM has different than expected EC values", func() { + BeforeEach(func() { + ov1, ov2 := *nameVal, *namespaceVal + ov1.Value = "fake" + ov2.Value = "fake" + ci.ExtraConfig = append(ci.ExtraConfig, &ov1, &ov2) + }) + It("updates it", func() { + Expect(cs.ExtraConfig).To(ConsistOf(nameVal, namespaceVal)) + }) + }) + Context("VM and ConfigSpec already have expected values", func() { + BeforeEach(func() { + ci.ExtraConfig = append(ci.ExtraConfig, nameVal, namespaceVal) + cs.ExtraConfig = append(cs.ExtraConfig, nameVal, namespaceVal) + }) + + It("removes updates", func() { + Expect(cs.ExtraConfig).To(BeEmpty()) + }) + }) + }) + Context("MMIO ExtraConfig", func() { const mmIOValue = "42" diff --git a/pkg/util/vsphere/vm/guest_id.go b/pkg/util/vsphere/vm/guest_id.go index 6a4f99197..718148783 100644 --- a/pkg/util/vsphere/vm/guest_id.go +++ b/pkg/util/vsphere/vm/guest_id.go @@ -4,55 +4,26 @@ package vm import ( - "fmt" "strings" vimtypes "github.com/vmware/govmomi/vim25/types" - - vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" - "github.com/vmware-tanzu/vm-operator/pkg/conditions" - pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" ) // GuestIDProperty is the property name for the guest ID in the config spec. const GuestIDProperty = "configSpec.guestId" -// UpdateVMGuestIDReconfiguredCondition deletes the VM's GuestIDReconfigured -// condition if the configSpec doesn't contain a guestID, or if the taskInfo -// does not contain an invalid guestID property error. Otherwise, it sets the -// condition to false with the invalid guest ID property value in the reason. -func UpdateVMGuestIDReconfiguredCondition( - vmctx pkgctx.VirtualMachineContext, - configSpec vimtypes.VirtualMachineConfigSpec, - taskInfo *vimtypes.TaskInfo) { - if configSpec.GuestId == "" { - conditions.Delete(vmctx.VM, vmopv1.GuestIDReconfiguredCondition) - return +// IsTaskInfoErrorInvalidGuestID returns true if the provided taskInfo contains +// an error with a fault that indicates the underlying cause is an invalid guest +// ID. +func IsTaskInfoErrorInvalidGuestID(taskInfo *vimtypes.TaskInfo) bool { + if taskInfo == nil { + return false } - - var invalidGuestID bool - - defer func() { - if invalidGuestID { - conditions.MarkFalse( - vmctx.VM, - vmopv1.GuestIDReconfiguredCondition, - "Invalid", - fmt.Sprintf("The specified guest ID value is not supported: %s", - configSpec.GuestId)) - } else { - conditions.Delete(vmctx.VM, vmopv1.GuestIDReconfiguredCondition) - } - }() - - if taskInfo == nil || taskInfo.Error == nil { - return + if taskInfo.Error == nil { + return false } - - fault, ok := taskInfo.Error.Fault.(*vimtypes.InvalidArgument) - if !ok { - return + if f, ok := taskInfo.Error.Fault.(*vimtypes.InvalidArgument); ok { + return strings.Contains(f.InvalidProperty, GuestIDProperty) } - - invalidGuestID = strings.Contains(fault.InvalidProperty, GuestIDProperty) + return false } diff --git a/pkg/util/vsphere/vm/guest_id_test.go b/pkg/util/vsphere/vm/guest_id_test.go index a7915aa47..ab095d83c 100644 --- a/pkg/util/vsphere/vm/guest_id_test.go +++ b/pkg/util/vsphere/vm/guest_id_test.go @@ -8,135 +8,62 @@ import ( . "github.com/onsi/gomega" vimtypes "github.com/vmware/govmomi/vim25/types" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3" - "github.com/vmware-tanzu/vm-operator/pkg/conditions" - pkgctx "github.com/vmware-tanzu/vm-operator/pkg/context" vmutil "github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm" ) func guestIDTests() { - - Context("UpdateVMGuestIDReconfiguredCondition", func() { - - var ( - vmCtx pkgctx.VirtualMachineContext - vmopVM vmopv1.VirtualMachine - configSpec vimtypes.VirtualMachineConfigSpec - taskInfo *vimtypes.TaskInfo - ) - - BeforeEach(func() { - // Init VM with the condition set to verify it's actually deleted. - vmopVM = vmopv1.VirtualMachine{ - Status: vmopv1.VirtualMachineStatus{ - Conditions: []metav1.Condition{ - { - Type: vmopv1.GuestIDReconfiguredCondition, - Status: metav1.ConditionFalse, - }, + DescribeTable("IsTaskInfoErrorInvalidGuestID", + func(taskInfo *vimtypes.TaskInfo, expected bool) { + Expect(vmutil.IsTaskInfoErrorInvalidGuestID(taskInfo)).To(Equal(expected)) + }, + Entry( + "taskInfo is nil", + nil, + false, + ), + Entry( + "taskInfo.Error is nil", + &vimtypes.TaskInfo{}, + false, + ), + Entry( + "taskInfo.Error.Fault is nil", + &vimtypes.TaskInfo{ + Error: &vimtypes.LocalizedMethodFault{}, + }, + false, + ), + Entry( + "taskInfo.Error.Fault is not InvalidArgument", + &vimtypes.TaskInfo{ + Error: &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.AdminDisabled{}, + }, + }, + false, + ), + Entry( + "taskInfo.Error.Fault is InvalidArgument with wrong property", + &vimtypes.TaskInfo{ + Error: &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.InvalidArgument{ + InvalidProperty: "configSpec.name", }, }, - } - vmCtx = pkgctx.VirtualMachineContext{ - VM: &vmopVM, - } - configSpec = vimtypes.VirtualMachineConfigSpec{} - taskInfo = &vimtypes.TaskInfo{} - }) - - JustBeforeEach(func() { - vmutil.UpdateVMGuestIDReconfiguredCondition(vmCtx, configSpec, taskInfo) - }) - - Context("ConfigSpec doesn't have a guest ID", func() { - - BeforeEach(func() { - configSpec.GuestId = "" - }) - - It("should delete the existing VM's guest ID condition", func() { - Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) - }) - }) - - Context("ConfigSpec has a guest ID", func() { - - BeforeEach(func() { - configSpec.GuestId = "test-guest-id-value" - }) - - When("TaskInfo is nil", func() { - - BeforeEach(func() { - taskInfo = nil - }) - - It("should delete the VM's guest ID condition", func() { - Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) - }) - }) - - When("TaskInfo.Error is nil", func() { - - BeforeEach(func() { - taskInfo.Error = nil - }) - - It("should delete the VM's guest ID condition", func() { - Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) - }) - }) - - When("TaskInfo.Error.Fault is not an InvalidPropertyFault", func() { - - BeforeEach(func() { - taskInfo.Error = &vimtypes.LocalizedMethodFault{ - Fault: &vimtypes.InvalidName{ - Name: "some-invalid-name", - }, - } - }) - - It("should delete the VM's guest ID condition", func() { - Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) - }) - }) - - When("TaskInfo.Error contains an invalid property error NOT about guestID", func() { - - BeforeEach(func() { - taskInfo.Error = &vimtypes.LocalizedMethodFault{ - Fault: &vimtypes.InvalidArgument{ - InvalidProperty: "config.version", - }, - } - }) - - It("should delete the VM's guest ID condition", func() { - Expect(conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition)).To(BeNil()) - }) - }) - - When("TaskInfo.Error contains an invalid property error of guestID", func() { - - BeforeEach(func() { - taskInfo.Error = &vimtypes.LocalizedMethodFault{ - Fault: &vimtypes.InvalidArgument{ - InvalidProperty: "configSpec.guestId", - }, - } - }) - - It("should set the VM's guest ID condition to false with the invalid value in the reason", func() { - c := conditions.Get(&vmopVM, vmopv1.GuestIDReconfiguredCondition) - Expect(c).NotTo(BeNil()) - Expect(c.Status).To(Equal(metav1.ConditionFalse)) - Expect(c.Reason).To(Equal("Invalid")) - Expect(c.Message).To(Equal("The specified guest ID value is not supported: test-guest-id-value")) - }) - }) - }) - }) + }, + false, + ), + Entry( + "taskInfo.Error.Fault is InvalidArgument with guestID", + &vimtypes.TaskInfo{ + Error: &vimtypes.LocalizedMethodFault{ + Fault: &vimtypes.InvalidArgument{ + InvalidProperty: vmutil.GuestIDProperty, + }, + }, + }, + true, + ), + ) }