Perennial properties

This patch does two things:

1. The VM object's namespace and name are stored in ExtraConfig
   as the keys vmservice.namespace and vmservice.name.

2. 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 managedBy property.

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"

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,23 +977,27 @@ 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 {
 		powerOff = vmCtx.VM.Spec.PowerOffMode == vmopv1.VirtualMachinePowerOpModeHard ||
 			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,14 +1018,26 @@ 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,
 			getResizeArgsFn)
 		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)
+}

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 != "" {

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,

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},