diff --git a/.web-docs/components/builder/vsphere-clone/README.md b/.web-docs/components/builder/vsphere-clone/README.md index af9216d1..b7a8ce15 100644 --- a/.web-docs/components/builder/vsphere-clone/README.md +++ b/.web-docs/components/builder/vsphere-clone/README.md @@ -233,6 +233,10 @@ JSON Example: - `disk_controller_index` (int) - The assigned disk controller for the disk. Defaults to the first controller, `(0)`. +- `storage_policy` (string) - The name of the storage policy to apply to the disk. The storage policy + must already exist on the vCenter Server. If not specified, the default + storage policy of the target datastore is used. + diff --git a/.web-docs/components/builder/vsphere-iso/README.md b/.web-docs/components/builder/vsphere-iso/README.md index c3892261..e83f0645 100644 --- a/.web-docs/components/builder/vsphere-iso/README.md +++ b/.web-docs/components/builder/vsphere-iso/README.md @@ -877,6 +877,10 @@ JSON Example: - `disk_controller_index` (int) - The assigned disk controller for the disk. Defaults to the first controller, `(0)`. +- `storage_policy` (string) - The name of the storage policy to apply to the disk. The storage policy + must already exist on the vCenter Server. If not specified, the default + storage policy of the target datastore is used. + diff --git a/builder/vsphere/common/storage_config.go b/builder/vsphere/common/storage_config.go index bf819862..2760ae1f 100644 --- a/builder/vsphere/common/storage_config.go +++ b/builder/vsphere/common/storage_config.go @@ -108,6 +108,10 @@ type DiskConfig struct { // The assigned disk controller for the disk. // Defaults to the first controller, `(0)`. DiskControllerIndex int `mapstructure:"disk_controller_index"` + // The name of the storage policy to apply to the disk. The storage policy + // must already exist on the vCenter Server. If not specified, the default + // storage policy of the target datastore is used. + StoragePolicyName string `mapstructure:"storage_policy"` } type StorageConfig struct { diff --git a/builder/vsphere/common/storage_config.hcl2spec.go b/builder/vsphere/common/storage_config.hcl2spec.go index ad57d92e..f79ae03f 100644 --- a/builder/vsphere/common/storage_config.hcl2spec.go +++ b/builder/vsphere/common/storage_config.hcl2spec.go @@ -10,10 +10,11 @@ import ( // FlatDiskConfig is an auto-generated flat version of DiskConfig. // Where the contents of a field with a `mapstructure:,squash` tag are bubbled up. type FlatDiskConfig struct { - DiskSize *int64 `mapstructure:"disk_size" required:"true" cty:"disk_size" hcl:"disk_size"` - DiskThinProvisioned *bool `mapstructure:"disk_thin_provisioned" cty:"disk_thin_provisioned" hcl:"disk_thin_provisioned"` - DiskEagerlyScrub *bool `mapstructure:"disk_eagerly_scrub" cty:"disk_eagerly_scrub" hcl:"disk_eagerly_scrub"` - DiskControllerIndex *int `mapstructure:"disk_controller_index" cty:"disk_controller_index" hcl:"disk_controller_index"` + DiskSize *int64 `mapstructure:"disk_size" required:"true" cty:"disk_size" hcl:"disk_size"` + DiskThinProvisioned *bool `mapstructure:"disk_thin_provisioned" cty:"disk_thin_provisioned" hcl:"disk_thin_provisioned"` + DiskEagerlyScrub *bool `mapstructure:"disk_eagerly_scrub" cty:"disk_eagerly_scrub" hcl:"disk_eagerly_scrub"` + DiskControllerIndex *int `mapstructure:"disk_controller_index" cty:"disk_controller_index" hcl:"disk_controller_index"` + StoragePolicyName *string `mapstructure:"storage_policy" cty:"storage_policy" hcl:"storage_policy"` } // FlatMapstructure returns a new FlatDiskConfig. @@ -32,6 +33,7 @@ func (*FlatDiskConfig) HCL2Spec() map[string]hcldec.Spec { "disk_thin_provisioned": &hcldec.AttrSpec{Name: "disk_thin_provisioned", Type: cty.Bool, Required: false}, "disk_eagerly_scrub": &hcldec.AttrSpec{Name: "disk_eagerly_scrub", Type: cty.Bool, Required: false}, "disk_controller_index": &hcldec.AttrSpec{Name: "disk_controller_index", Type: cty.Number, Required: false}, + "storage_policy": &hcldec.AttrSpec{Name: "storage_policy", Type: cty.String, Required: false}, } return s } diff --git a/builder/vsphere/driver/disk.go b/builder/vsphere/driver/disk.go index 14d1cb88..25a86038 100644 --- a/builder/vsphere/driver/disk.go +++ b/builder/vsphere/driver/disk.go @@ -16,6 +16,9 @@ type Disk struct { DiskEagerlyScrub bool DiskThinProvisioned bool ControllerIndex int + // StoragePolicyID is the UUID of the vSphere storage policy to associate + // with this disk. Empty means no explicit policy; the datastore default applies. + StoragePolicyID string } type StorageConfig struct { @@ -28,10 +31,12 @@ type StorageConfig struct { // It creates a controller for each type specified in DiskControllerType and attaches // virtual disks to the controllers. If DatastoreRefs is provided, each disk is placed // on the corresponding datastore; otherwise, disks inherit the VM's datastore. +// When a disk has a StoragePolicyID, a VirtualMachineDefinedProfileSpec is attached +// to that disk's device config spec. func (c *StorageConfig) AddStorageDevices(existingDevices object.VirtualDeviceList) ([]types.BaseVirtualDeviceConfigSpec, error) { - newDevices := object.VirtualDeviceList{} - + var controllerDevices object.VirtualDeviceList var controllers []types.BaseVirtualController + for _, controllerType := range c.DiskControllerType { var device types.BaseVirtualDevice var err error @@ -47,7 +52,7 @@ func (c *StorageConfig) AddStorageDevices(existingDevices object.VirtualDeviceLi return nil, err } existingDevices = append(existingDevices, device) - newDevices = append(newDevices, device) + controllerDevices = append(controllerDevices, device) controller, err := existingDevices.FindDiskController(existingDevices.Name(device)) if err != nil { return nil, err @@ -55,6 +60,11 @@ func (c *StorageConfig) AddStorageDevices(existingDevices object.VirtualDeviceLi controllers = append(controllers, controller) } + allSpecs, err := controllerDevices.ConfigSpec(types.VirtualDeviceConfigSpecOperationAdd) + if err != nil { + return nil, err + } + for i, dc := range c.Storage { backing := &types.VirtualDiskFlatVer2BackingInfo{ DiskMode: string(types.VirtualDiskModePersistent), @@ -76,10 +86,24 @@ func (c *StorageConfig) AddStorageDevices(existingDevices object.VirtualDeviceLi existingDevices.AssignController(disk, controllers[dc.ControllerIndex]) existingDevices = append(existingDevices, disk) - newDevices = append(newDevices, disk) + + diskSpecs, err := object.VirtualDeviceList{disk}.ConfigSpec(types.VirtualDeviceConfigSpecOperationAdd) + if err != nil { + return nil, err + } + diskSpec := diskSpecs[0] + if dc.StoragePolicyID != "" { + vdcs := diskSpec.(*types.VirtualDeviceConfigSpec) + vdcs.Profile = []types.BaseVirtualMachineProfileSpec{ + &types.VirtualMachineDefinedProfileSpec{ + ProfileId: dc.StoragePolicyID, + }, + } + } + allSpecs = append(allSpecs, diskSpec) } - return newDevices.ConfigSpec(types.VirtualDeviceConfigSpecOperationAdd) + return allSpecs, nil } // findDisk scans a list of virtual devices and retrieves a single virtual disk. diff --git a/builder/vsphere/driver/disk_test.go b/builder/vsphere/driver/disk_test.go index 90ef524d..f89cfa9c 100644 --- a/builder/vsphere/driver/disk_test.go +++ b/builder/vsphere/driver/disk_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25/types" ) func TestAddStorageDevices(t *testing.T) { @@ -51,3 +52,47 @@ func TestAddStorageDevices(t *testing.T) { t.Fatalf("unexpected result: expected '3', but returned '%d'", len(storageConfigSpec)) } } + +// TestAddStorageDevices_WithStoragePolicy verifies that a disk with a +// StoragePolicyID gets a VirtualMachineDefinedProfileSpec in its config spec, +// while a disk without a policy gets no profile entry. +func TestAddStorageDevices_WithStoragePolicy(t *testing.T) { + const policyUUID = "aaaabbbb-cccc-dddd-eeee-ffffffffffff" + + config := &StorageConfig{ + DiskControllerType: []string{"pvscsi"}, + Storage: []Disk{ + {DiskSize: 10240, DiskThinProvisioned: true, ControllerIndex: 0, StoragePolicyID: policyUUID}, + {DiskSize: 20480, DiskThinProvisioned: true, ControllerIndex: 0}, + }, + } + + specs, err := config.AddStorageDevices(object.VirtualDeviceList{}) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + // 1 controller + 2 disks + if len(specs) != 3 { + t.Fatalf("expected 3 specs, got %d", len(specs)) + } + + // specs[0] = controller (no profile expected) + // specs[1] = first disk (policy set) + // specs[2] = second disk (no policy) + disk0spec := specs[1].(*types.VirtualDeviceConfigSpec) + if len(disk0spec.Profile) != 1 { + t.Fatalf("expected 1 profile on disk 0, got %d", len(disk0spec.Profile)) + } + profileSpec, ok := disk0spec.Profile[0].(*types.VirtualMachineDefinedProfileSpec) + if !ok { + t.Fatal("expected VirtualMachineDefinedProfileSpec on disk 0") + } + if profileSpec.ProfileId != policyUUID { + t.Fatalf("expected profile ID %q, got %q", policyUUID, profileSpec.ProfileId) + } + + disk1spec := specs[2].(*types.VirtualDeviceConfigSpec) + if len(disk1spec.Profile) != 0 { + t.Fatalf("expected no profile on disk 1, got %d", len(disk1spec.Profile)) + } +} diff --git a/builder/vsphere/driver/driver.go b/builder/vsphere/driver/driver.go index 5fc5dec1..bf84e7d2 100644 --- a/builder/vsphere/driver/driver.go +++ b/builder/vsphere/driver/driver.go @@ -14,6 +14,7 @@ import ( "github.com/vmware/govmomi" "github.com/vmware/govmomi/find" "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/pbm" "github.com/vmware/govmomi/session" "github.com/vmware/govmomi/vapi/library" "github.com/vmware/govmomi/vapi/rest" @@ -48,6 +49,11 @@ type Driver interface { FindContentLibraryItem(libraryId string, name string) (*library.Item, error) FindContentLibraryFileDatastorePath(isoPath string) (string, error) UpdateContentLibraryItem(item *library.Item, name string, description string) error + + // FindStoragePolicyID resolves a storage policy name to its profile UUID. + // Returns an error if the policy does not exist on vCenter. + FindStoragePolicyID(name string) (string, error) + Cleanup() (error, error) } @@ -58,6 +64,7 @@ type VCenterDriver struct { RestClient *RestClient Finder *find.Finder Datacenter *object.Datacenter + pbmClient *pbm.Client } func NewVCenterDriver(ctx context.Context, client *govmomi.Client, vimClient *vim25.Client, user *url.Userinfo, finder *find.Finder, datacenter *object.Datacenter) *VCenterDriver { @@ -131,6 +138,24 @@ func NewDriver(config *ConnectConfig) (Driver, error) { return d, nil } +// FindStoragePolicyID resolves a storage policy name to its profile UUID using +// the vSphere Policy-Based Management (PBM) API. The PBM client is initialized +// lazily on first use; it shares the existing VIM25 session. +func (d *VCenterDriver) FindStoragePolicyID(name string) (string, error) { + if d.pbmClient == nil { + c, err := pbm.NewClient(d.Ctx, d.VimClient) + if err != nil { + return "", fmt.Errorf("error initializing PBM client: %v", err) + } + d.pbmClient = c + } + id, err := d.pbmClient.ProfileIDByName(d.Ctx, name) + if err != nil { + return "", fmt.Errorf("storage policy %q not found: %v", name, err) + } + return id, nil +} + func (d *VCenterDriver) Cleanup() (error, error) { return d.RestClient.client.Logout(d.Ctx), d.Client.SessionManager.Logout(d.Ctx) } diff --git a/builder/vsphere/driver/driver_mock.go b/builder/vsphere/driver/driver_mock.go index 9d5bff63..3fe16ec5 100644 --- a/builder/vsphere/driver/driver_mock.go +++ b/builder/vsphere/driver/driver_mock.go @@ -31,6 +31,11 @@ type DriverMock struct { FindVMCalled bool FindVMName string + + FindStoragePolicyIDCalled bool + FindStoragePolicyIDName string + FindStoragePolicyIDResult string + FindStoragePolicyIDErr error } func NewDriverMock() *DriverMock { @@ -126,6 +131,12 @@ func (d *DriverMock) UpdateContentLibraryItem(item *library.Item, name string, d return nil } +func (d *DriverMock) FindStoragePolicyID(name string) (string, error) { + d.FindStoragePolicyIDCalled = true + d.FindStoragePolicyIDName = name + return d.FindStoragePolicyIDResult, d.FindStoragePolicyIDErr +} + func (d *DriverMock) Cleanup() (error, error) { return nil, nil } diff --git a/builder/vsphere/driver/vm.go b/builder/vsphere/driver/vm.go index 7619b302..752b9c2a 100644 --- a/builder/vsphere/driver/vm.go +++ b/builder/vsphere/driver/vm.go @@ -264,6 +264,21 @@ func (d *VCenterDriver) CreateVM(config *CreateConfig) (VirtualMachine, error) { } createSpec.DeviceChange = append(createSpec.DeviceChange, storageConfigSpec...) + // vSphere requires VmProfile on the top-level config spec (which governs + // the VM home files: .vmx, .nvram, etc.) whenever any disk carries a + // per-disk storage policy profile. Use the first disk's policy for the + // VM home so the entire VM lives on compatible storage. + for _, disk := range config.StorageConfig.Storage { + if disk.StoragePolicyID != "" { + createSpec.VmProfile = []types.BaseVirtualMachineProfileSpec{ + &types.VirtualMachineDefinedProfileSpec{ + ProfileId: disk.StoragePolicyID, + }, + } + break + } + } + devices, err = addNetwork(d, devices, config) if err != nil { return nil, err diff --git a/builder/vsphere/driver/vm_test.go b/builder/vsphere/driver/vm_test.go index a8aa72e0..fbd41446 100644 --- a/builder/vsphere/driver/vm_test.go +++ b/builder/vsphere/driver/vm_test.go @@ -99,6 +99,56 @@ func TestVirtualMachineDriver_CreateVMWithMultipleDisks(t *testing.T) { } } +// TestVirtualMachineDriver_CreateVM_WithStoragePolicy verifies that CreateVM +// succeeds when a disk carries a StoragePolicyID, exercising the VmProfile +// code path in the VM config spec. +func TestVirtualMachineDriver_CreateVM_WithStoragePolicy(t *testing.T) { + sim, err := NewVCenterSimulator() + if err != nil { + t.Fatalf("unexpected error: '%s'", err) + } + defer sim.Close() + + _, datastore := sim.ChooseSimulatorPreCreatedDatastore() + + config := &CreateConfig{ + Name: "mock-vm-with-policy", + Host: "DC0_H0", + Datastore: datastore.Name, + NICs: []NIC{ + { + Network: "VM Network", + NetworkCard: "vmxnet3", + }, + }, + StorageConfig: StorageConfig{ + DiskControllerType: []string{"pvscsi"}, + Storage: []Disk{ + { + DiskSize: 10240, + DiskThinProvisioned: true, + ControllerIndex: 0, + StoragePolicyID: "aaaabbbb-cccc-dddd-eeee-ffffffffffff", + }, + { + DiskSize: 20480, + ControllerIndex: 0, + // No policy on this disk — VmProfile should still be set + // from the first disk's policy. + }, + }, + }, + } + + vm, err := sim.driver.CreateVM(config) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if vm == nil { + t.Fatal("expected a VM, got nil") + } +} + func TestVirtualMachineDriver_CloneWithPrimaryDiskResize(t *testing.T) { sim, err := NewVCenterSimulator() if err != nil { diff --git a/builder/vsphere/iso/step_create.go b/builder/vsphere/iso/step_create.go index 99bf434f..98fd2105 100644 --- a/builder/vsphere/iso/step_create.go +++ b/builder/vsphere/iso/step_create.go @@ -204,12 +204,21 @@ func (s *StepCreateVM) Run(_ context.Context, state multistep.StateBag) multiste var disks []driver.Disk for _, disk := range s.Config.StorageConfig.Storage { - disks = append(disks, driver.Disk{ + dd := driver.Disk{ DiskSize: disk.DiskSize, DiskEagerlyScrub: disk.DiskEagerlyScrub, DiskThinProvisioned: disk.DiskThinProvisioned, ControllerIndex: disk.DiskControllerIndex, - }) + } + if disk.StoragePolicyName != "" { + id, err := d.FindStoragePolicyID(disk.StoragePolicyName) + if err != nil { + state.Put("error", fmt.Errorf("error resolving storage policy %q: %v", disk.StoragePolicyName, err)) + return multistep.ActionHalt + } + dd.StoragePolicyID = id + } + disks = append(disks, dd) } datastoreName := s.Location.Datastore diff --git a/builder/vsphere/iso/step_create_test.go b/builder/vsphere/iso/step_create_test.go index 1b26d2c8..a030256d 100644 --- a/builder/vsphere/iso/step_create_test.go +++ b/builder/vsphere/iso/step_create_test.go @@ -7,6 +7,7 @@ package iso import ( "context" "errors" + "fmt" "io" "path" "strings" @@ -229,7 +230,7 @@ func TestStepCreateVM_Run(t *testing.T) { t.Fatalf("unexpected result: expected '%s' to be called", "CreateVM") } if diff := cmp.Diff(driverMock.CreateConfig, driverCreateConfig(step.Config, step.Location)); diff != "" { - t.Fatalf("unexpected result: %s", diff) + t.Fatalf("unexpected CreateConfig: %s", diff) } vm, ok := state.GetOk("vm") if !ok { @@ -390,7 +391,8 @@ func createConfig() *CreateConfig { // driverCreateConfig converts CreateConfig and LocationConfig into driver.CreateConfig for virtual machine creation. // It maps network interfaces, disks, and other configuration details to the required driver.CreateConfig structure. -func driverCreateConfig(config *CreateConfig, location *common.LocationConfig) *driver.CreateConfig { +// resolvedPolicyIDs maps disk index to pre-resolved storage policy UUID (empty string = no policy). +func driverCreateConfig(config *CreateConfig, location *common.LocationConfig, resolvedPolicyIDs ...string) *driver.CreateConfig { var networkCards []driver.NIC for _, nic := range config.NICs { networkCards = append(networkCards, driver.NIC{ @@ -402,13 +404,17 @@ func driverCreateConfig(config *CreateConfig, location *common.LocationConfig) * } var disks []driver.Disk - for _, disk := range config.StorageConfig.Storage { - disks = append(disks, driver.Disk{ + for i, disk := range config.StorageConfig.Storage { + d := driver.Disk{ DiskSize: disk.DiskSize, DiskEagerlyScrub: disk.DiskEagerlyScrub, DiskThinProvisioned: disk.DiskThinProvisioned, ControllerIndex: disk.DiskControllerIndex, - }) + } + if i < len(resolvedPolicyIDs) { + d.StoragePolicyID = resolvedPolicyIDs[i] + } + disks = append(disks, d) } return &driver.CreateConfig{ @@ -429,3 +435,55 @@ func driverCreateConfig(config *CreateConfig, location *common.LocationConfig) * Version: config.Version, } } + +// TestStepCreateVM_Run_WithStoragePolicy verifies that a storage policy name is +// resolved to a UUID and forwarded to CreateVM as StoragePolicyID on the disk. +func TestStepCreateVM_Run_WithStoragePolicy(t *testing.T) { + const policyName = "gold-policy" + const policyUUID = "aaaabbbb-cccc-dddd-eeee-ffffffffffff" + + state := basicStateBag() + driverMock := driver.NewDriverMock() + driverMock.FindStoragePolicyIDResult = policyUUID + state.Put("driver", driverMock) + + config := createConfig() + config.StorageConfig.Storage[0].StoragePolicyName = policyName + + step := &StepCreateVM{Config: config, Location: basicLocationConfig()} + + if action := step.Run(context.TODO(), state); action == multistep.ActionHalt { + t.Fatalf("unexpected halt: %v", state.Get("error")) + } + + if !driverMock.FindStoragePolicyIDCalled { + t.Fatal("expected FindStoragePolicyID to be called") + } + if driverMock.FindStoragePolicyIDName != policyName { + t.Fatalf("expected policy name %q, got %q", policyName, driverMock.FindStoragePolicyIDName) + } + if diff := cmp.Diff(driverMock.CreateConfig, driverCreateConfig(config, basicLocationConfig(), policyUUID)); diff != "" { + t.Fatalf("unexpected CreateConfig: %s", diff) + } +} + +// TestStepCreateVM_Run_StoragePolicyNotFound verifies that a missing storage +// policy causes the step to halt with a descriptive error. +func TestStepCreateVM_Run_StoragePolicyNotFound(t *testing.T) { + state := basicStateBag() + driverMock := driver.NewDriverMock() + driverMock.FindStoragePolicyIDErr = fmt.Errorf("no pbm profile found with name: %q", "nonexistent") + state.Put("driver", driverMock) + + config := createConfig() + config.StorageConfig.Storage[0].StoragePolicyName = "nonexistent" + + step := &StepCreateVM{Config: config, Location: basicLocationConfig()} + + if action := step.Run(context.TODO(), state); action != multistep.ActionHalt { + t.Fatalf("expected ActionHalt, got %v", action) + } + if state.Get("error") == nil { + t.Fatal("expected an error in state, got nil") + } +} diff --git a/docs-partials/builder/vsphere/common/DiskConfig-not-required.mdx b/docs-partials/builder/vsphere/common/DiskConfig-not-required.mdx index d369f8fa..a4b77bed 100644 --- a/docs-partials/builder/vsphere/common/DiskConfig-not-required.mdx +++ b/docs-partials/builder/vsphere/common/DiskConfig-not-required.mdx @@ -9,4 +9,8 @@ - `disk_controller_index` (int) - The assigned disk controller for the disk. Defaults to the first controller, `(0)`. +- `storage_policy` (string) - The name of the storage policy to apply to the disk. The storage policy + must already exist on the vCenter Server. If not specified, the default + storage policy of the target datastore is used. +