Skip to content

Commit

Permalink
Wait on vAppConfig during async create
Browse files Browse the repository at this point in the history
This patch updates the behavior of the vAppConfig, LinuxPrep, and
Sysprep bootstrap providers to wait on the VM to have a non-nil
config.vAppConfig property when it is expected to exist.

Async create means we are no longer falling through to the update
logic directly after create, instead, relying on the reconciler to
be reentrant. That all works fine, but the problem is vpxd is sending
signal on the property collector that the VM is ready prior to its
config.vAppConfig property being set.

VM Op receives this signal and proceeds to reconcile the VM. Since
previously we did not wait on an expected vAppConfig, we proceeded
to configure and power on the VM. Once it is powered on, any customization
that dependended upon the vAppConfig properties being set prior to
first boot will no longer work.

This patch returns an error from GetOVFVAppConfigForConfigSpec if the
VM has a nil config.vAppConfig.
  • Loading branch information
akutz committed Dec 9, 2024
1 parent bfe5a9e commit 85fd7a4
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 15 deletions.
4 changes: 2 additions & 2 deletions pkg/providers/vsphere/vmlifecycle/bootstrap_linuxprep.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ func BootStrapLinuxPrep(
var configSpec *vimtypes.VirtualMachineConfigSpec
if vAppConfigSpec != nil {
configSpec = &vimtypes.VirtualMachineConfigSpec{}
configSpec.VAppConfig = GetOVFVAppConfigForConfigSpec(
configSpec.VAppConfig, err = GetOVFVAppConfigForConfigSpec(
config,
vAppConfigSpec,
bsArgs.BootstrapData.VAppData,
bsArgs.BootstrapData.VAppExData,
bsArgs.TemplateRenderFn)
}

return configSpec, customSpec, nil
return configSpec, customSpec, err
}
4 changes: 2 additions & 2 deletions pkg/providers/vsphere/vmlifecycle/bootstrap_sysprep.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ func BootstrapSysPrep(
var configSpec *vimtypes.VirtualMachineConfigSpec
if vAppConfigSpec != nil {
configSpec = &vimtypes.VirtualMachineConfigSpec{}
configSpec.VAppConfig = GetOVFVAppConfigForConfigSpec(
configSpec.VAppConfig, err = GetOVFVAppConfigForConfigSpec(
config,
vAppConfigSpec,
bsArgs.BootstrapData.VAppData,
bsArgs.BootstrapData.VAppExData,
bsArgs.TemplateRenderFn)
}

return configSpec, customSpec, nil
return configSpec, customSpec, err
}

func convertTo(from *vmopv1sysprep.Sysprep, bsArgs *BootstrapArgs) *vimtypes.CustomizationSysprep {
Expand Down
24 changes: 14 additions & 10 deletions pkg/providers/vsphere/vmlifecycle/bootstrap_vappconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package vmlifecycle

import (
"context"
"errors"

vimtypes "github.com/vmware/govmomi/vim25/types"

Expand All @@ -17,32 +18,35 @@ func BootstrapVAppConfig(
vAppConfigSpec *vmopv1.VirtualMachineBootstrapVAppConfigSpec,
bsArgs *BootstrapArgs) (*vimtypes.VirtualMachineConfigSpec, *vimtypes.CustomizationSpec, error) {

configSpec := &vimtypes.VirtualMachineConfigSpec{}
configSpec.VAppConfig = GetOVFVAppConfigForConfigSpec(
var (
err error
configSpec vimtypes.VirtualMachineConfigSpec
)

configSpec.VAppConfig, err = GetOVFVAppConfigForConfigSpec(
config,
vAppConfigSpec,
bsArgs.BootstrapData.VAppData,
bsArgs.BootstrapData.VAppExData,
bsArgs.TemplateRenderFn)

return configSpec, nil, nil
return &configSpec, nil, err
}

func GetOVFVAppConfigForConfigSpec(
config *vimtypes.VirtualMachineConfigInfo,
vAppConfigSpec *vmopv1.VirtualMachineBootstrapVAppConfigSpec,
vAppData map[string]string,
vAppExData map[string]map[string]string,
templateRenderFn TemplateRenderFunc) vimtypes.BaseVmConfigSpec {
templateRenderFn TemplateRenderFunc) (vimtypes.BaseVmConfigSpec, error) {

if config.VAppConfig == nil {
// BMV: Should we really silently return here and below?
return nil
var vAppConfigInfo *vimtypes.VmConfigInfo
if config.VAppConfig != nil {
vAppConfigInfo = config.VAppConfig.GetVmConfigInfo()
}

vAppConfigInfo := config.VAppConfig.GetVmConfigInfo()
if vAppConfigInfo == nil {
return nil
return nil, errors.New("vAppConfig is not yet available")
}

if len(vAppConfigSpec.Properties) > 0 {
Expand All @@ -65,7 +69,7 @@ func GetOVFVAppConfigForConfigSpec(
}
}

return GetMergedvAppConfigSpec(vAppData, vAppConfigInfo.Property)
return GetMergedvAppConfigSpec(vAppData, vAppConfigInfo.Property), nil
}

// GetMergedvAppConfigSpec prepares a vApp VmConfigSpec which will set the provided key/value fields.
Expand Down
13 changes: 12 additions & 1 deletion pkg/providers/vsphere/vmlifecycle/bootstrap_vappconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var _ = Describe("VAppConfig Bootstrap", func() {
const key, value = "fooKey", "fooValue"

var (
err error
configInfo *vimtypes.VirtualMachineConfigInfo
vAppConfigSpec *vmopv1.VirtualMachineBootstrapVAppConfigSpec
bsArgs vmlifecycle.BootstrapArgs
Expand Down Expand Up @@ -53,14 +54,24 @@ var _ = Describe("VAppConfig Bootstrap", func() {
Context("GetOVFVAppConfigForConfigSpec", func() {

JustBeforeEach(func() {
baseVMConfigSpec = vmlifecycle.GetOVFVAppConfigForConfigSpec(
baseVMConfigSpec, err = vmlifecycle.GetOVFVAppConfigForConfigSpec(
configInfo,
vAppConfigSpec,
bsArgs.VAppData,
bsArgs.VAppExData,
bsArgs.TemplateRenderFn)
})

When("config.vAppConfig is nil", func() {
BeforeEach(func() {
configInfo.VAppConfig = nil
})
It("Should return an error", func() {
Expect(err).To(MatchError("vAppConfig is not yet available"))
Expect(baseVMConfigSpec).To(BeNil())
})
})

Context("Empty input", func() {
It("No changes", func() {
Expect(baseVMConfigSpec).To(BeNil())
Expand Down

0 comments on commit 85fd7a4

Please sign in to comment.