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/session/session_vm_update.go b/pkg/providers/vsphere/session/session_vm_update.go index e88f039f8..9de412ada 100644 --- a/pkg/providers/vsphere/session/session_vm_update.go +++ b/pkg/providers/vsphere/session/session_vm_update.go @@ -231,6 +231,40 @@ func UpdatePCIDeviceChanges( return append(removeDeviceChanges, deviceChanges...), nil } +// UpdatePerennialConfigSpecExtraConfig updates the ExtraConfig of the given +// ConfigSpec with the properties that should always be set. +func UpdatePerennialConfigSpecExtraConfig( + ctx context.Context, + configInfo vimtypes.VirtualMachineConfigInfo, + configSpec *vimtypes.VirtualMachineConfigSpec, + vm vmopv1.VirtualMachine) { + + extraConfig := map[string]string{} + + // Translate the VM's current extra config data into a map to simplify + // looking up existing values. + curExtraConfig := pkgutil.OptionValues(configInfo.ExtraConfig) + curExtraConfigMap := curExtraConfig.StringMap() + + // Ensure the VM's extra config contains a reference to the name/namespace + // of the VM's Kubernetes object. + { + const k = constants.ExtraConfigVMServiceName + if v, ok := curExtraConfigMap[k]; !ok || v != vm.Name { + extraConfig[k] = vm.Name + } + } + { + const k = constants.ExtraConfigVMServiceNamespace + if v, ok := curExtraConfigMap[k]; !ok || v != vm.Namespace { + extraConfig[k] = vm.Namespace + } + } + + // Update the ConfigSpec's ExtraConfig property with the results from above. + configSpec.ExtraConfig = curExtraConfig.Diff(pkgutil.OptionValuesFromMap(extraConfig)...) +} + // UpdateConfigSpecExtraConfig updates the ExtraConfig of the given ConfigSpec. // At a minimum, config and configSpec must be non-nil, in which case it will // just ensure MMPowerOffVMExtraConfigKey is no longer part of ExtraConfig. @@ -424,13 +458,19 @@ func UpdateConfigSpecAnnotation( } func UpdateConfigSpecManagedBy( - config *vimtypes.VirtualMachineConfigInfo, + configInfo vimtypes.VirtualMachineConfigInfo, configSpec *vimtypes.VirtualMachineConfigSpec) { - if config.ManagedBy == nil { - configSpec.ManagedBy = &vimtypes.ManagedByInfo{ - ExtensionKey: vmopv1.ManagedByExtensionKey, - Type: vmopv1.ManagedByExtensionType, - } + + in := configInfo.ManagedBy + if configSpec.ManagedBy == nil { + configSpec.ManagedBy = &vimtypes.ManagedByInfo{} + } + + if v := vmopv1.ManagedByExtensionKey; in == nil || in.ExtensionKey != v { + configSpec.ManagedBy.ExtensionKey = v + } + if v := vmopv1.ManagedByExtensionType; in == nil || in.Type != v { + configSpec.ManagedBy.Type = v } } @@ -467,11 +507,11 @@ func updateConfigSpec( configSpec := &vimtypes.VirtualMachineConfigSpec{} vmClassSpec := updateArgs.VMClass.Spec - UpdateConfigSpecAnnotation(config, configSpec) - UpdateConfigSpecManagedBy(config, configSpec) + UpdatePerennialConfigSpec(vmCtx, *config, configSpec, *vmCtx.VM) 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) @@ -789,7 +829,8 @@ func (s *Session) poweredOnVMReconfigure( configSpec := &vimtypes.VirtualMachineConfigSpec{} - UpdateConfigSpecExtraConfig(vmCtx, config, configSpec, nil, nil, nil, nil) + UpdatePerennialConfigSpec(vmCtx, *config, configSpec, *vmCtx.VM) + UpdateConfigSpecExtraConfig(vmCtx, config, configSpec, nil, nil, vmCtx.VM, nil) UpdateConfigSpecChangeBlockTracking(vmCtx, config, configSpec, nil, vmCtx.VM.Spec) if pkgcfg.FromContext(vmCtx).Features.IsoSupport { @@ -878,6 +919,8 @@ func (s *Session) resizeVMWhenPoweredStateOff( if err != nil { return false, err } + } else { + UpdatePerennialConfigSpec(vmCtx, *moVM.Config, &configSpec, *vmCtx.VM) } refetchProps := false @@ -934,9 +977,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 +991,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 +1018,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 +1026,18 @@ func (s *Session) updateVMDesiredPowerStateOff( if err != nil { return refetchProps, err } + if refetch { + refetchProps = true + } + } else { + refetch, err := reconfigurePerennialConfigSpec( + vmCtx, vcVM, *vmCtx.MoVM.Config) + if err != nil { + return refetchProps, err + } + if refetch { + refetchProps = true + } } return refetchProps, err @@ -987,22 +1046,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 := reconfigurePerennialConfigSpec(vmCtx, vcVM, *vmCtx.MoVM.Config) + if err != nil { + return refetchProps, err + } + if refetch { + refetchProps = true + } + + return refetchProps, nil } func (s *Session) updateVMDesiredPowerStateOn( @@ -1168,6 +1238,10 @@ func (s *Session) UpdateVirtualMachine( } } else { vmCtx.Logger.Info("VirtualMachine is paused. PowerState is not updated.") + refetchProps, err = reconfigurePerennialConfigSpec( + vmCtx, + vcVM, + *vmCtx.MoVM.Config) } if refetchProps { @@ -1215,3 +1289,40 @@ func isVMPaused(vmCtx pkgctx.VirtualMachineContext) bool { delete(vm.Labels, vmopv1.PausedVMLabelKey) return false } + +func reconfigurePerennialConfigSpec( + vmCtx pkgctx.VirtualMachineContext, + vcVM *object.VirtualMachine, + configInfo vimtypes.VirtualMachineConfigInfo) (bool, error) { + + var configSpec vimtypes.VirtualMachineConfigSpec + + UpdatePerennialConfigSpec(vmCtx, configInfo, &configSpec, *vmCtx.VM) + + resVM := res.NewVMFromObject(vcVM) + if _, err := resVM.Reconfigure(vmCtx, &configSpec); err != nil { + return false, err + } + + return true, nil +} + +// UpdatePerennialConfigSpec updates the ConfigSpec with changes that should +// *always* happen based on the current state of the VM, regardless of whether +// the VM is paused or its power state. +func UpdatePerennialConfigSpec( + ctx context.Context, + configInfo vimtypes.VirtualMachineConfigInfo, + configSpec *vimtypes.VirtualMachineConfigSpec, + vm vmopv1.VirtualMachine) { + + if ctx == nil { + panic("ctx is nil") + } + if configSpec == nil { + panic("configSpec is nil") + } + + UpdatePerennialConfigSpecExtraConfig(ctx, configInfo, configSpec, vm) + UpdateConfigSpecManagedBy(configInfo, configSpec) +} diff --git a/pkg/providers/vsphere/virtualmachine/configspec.go b/pkg/providers/vsphere/virtualmachine/configspec.go index 73a6596be..a54f3c86e 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..068b2ad4a 100644 --- a/pkg/util/vmopv1/resize_overwrite.go +++ b/pkg/util/vmopv1/resize_overwrite.go @@ -29,6 +29,7 @@ func OverwriteResizeConfigSpec( overwriteGuestID(vm, ci, cs) overwriteExtraConfig(vm, ci, cs) + overwriteManagedBy(vm, ci, cs) return nil } @@ -53,6 +54,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 +62,63 @@ func overwriteExtraConfig( 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) +} + +func ensureNamespaceName( + vm vmopv1.VirtualMachine, + ci vimtypes.VirtualMachineConfigInfo, + cs *vimtypes.VirtualMachineConfigSpec) []vimtypes.BaseOptionValue { + + outEC := map[string]string{} + 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[key] = expectedVal + } + } else { + // The key is not present. + outEC[key] = expectedVal + } + } + + fn(constants.ExtraConfigVMServiceName, vm.Name) + fn(constants.ExtraConfigVMServiceNamespace, vm.Namespace) + + return util.OptionValuesFromMap(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..44efdc030 100644 --- a/pkg/util/vmopv1/resize_overwrite_test.go +++ b/pkg/util/vmopv1/resize_overwrite_test.go @@ -137,6 +137,56 @@ var _ = Describe("OverwriteResizeConfigSpec", func() { Expect(err).ToNot(HaveOccurred()) }) + Context("Namespace and name", func() { + var ( + nameVal = &vimtypes.OptionValue{Key: constants.ExtraConfigVMServiceName, Value: vm.Name} + namespaceVal = &vimtypes.OptionValue{Key: constants.ExtraConfigVMServiceNamespace, Value: vm.Namespace} + ) + + 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"