Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add instant clone #1516

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions vsphere/internal/helper/storagepod/storage_pod_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/vmware/govmomi"
"github.com/vmware/govmomi/find"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/property"
"github.com/vmware/govmomi/vim25/mo"
"github.com/vmware/govmomi/vim25/types"
)
Expand Down Expand Up @@ -521,3 +522,56 @@ func IsMember(pod *object.StoragePod, ds *object.Datastore) (bool, error) {
}
return true, nil
}

// get_recommended_datastore from storagePod
func GetRecommendDatastore(client *govmomi.Client,
fo *object.Folder,
name string,
timeout int,
pod *object.StoragePod,
) (*object.Datastore, error) {
sdrsEnabled, err := StorageDRSEnabled(pod)
if err != nil {
return nil, err
}
if !sdrsEnabled {
return nil, fmt.Errorf("storage DRS is not enabled on datastore cluster %q", pod.Name())
}
log.Printf(
"[DEBUG] Instant Cloning virtual machine to %q on datastore cluster %q",
fmt.Sprintf("%s/%s", fo.InventoryPath, name),
pod.Name(),
)
sps := types.StoragePlacementSpec{
Type: string(types.StoragePlacementSpecPlacementTypeCreate),
PodSelectionSpec: types.StorageDrsPodSelectionSpec{
StoragePod: types.NewReference(pod.Reference()),
},
}
log.Printf("[DEBUG] Acquiring Storage DRS recommendations (type: %q)", sps.Type)
srm := object.NewStorageResourceManager(client.Client)
ctx, cancel := context.WithTimeout(context.Background(), 100)
defer cancel()

placement, err := srm.RecommendDatastores(ctx, sps)
if err != nil {
return nil, err
}
recs := placement.Recommendations
if len(recs) < 1 {
return nil, fmt.Errorf("no storage DRS recommendations were found for the requested action (type: %q)", sps.Type)
}
// result to pin disks to recommended datastores
ds := recs[0].Action[0].(*types.StoragePlacementAction).Destination
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest checking len(recs[0].Action) before accessing.
If it is always populated by design you can add a comment, otherwise it looks like a panic-prone line


var mds mo.Datastore
err = property.DefaultCollector(client.Client).RetrieveOne(ctx, ds, []string{"name"}, &mds)
if err != nil {
return nil, err
}

datastore := object.NewDatastore(client.Client, ds)
datastore.InventoryPath = mds.Name
return datastore, nil

}
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,30 @@ func Clone(c *govmomi.Client, src *object.VirtualMachine, f *object.Folder, name
return FromMOID(c, result.Result.(types.ManagedObjectReference).Value)
}

// InstantClone wraps the creation of a virtual machine instantly and the subsequent waiting of
// the task. A higher-level virtual machine object is returned.
func InstantClone(c *govmomi.Client, src *object.VirtualMachine, f *object.Folder, name string, spec types.VirtualMachineInstantCloneSpec, timeout int) (*object.VirtualMachine, error) {
log.Printf("[DEBUG] Instant Cloning virtual machine %q", fmt.Sprintf("%s/%s", f.InventoryPath, name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the Folder pointer and the source VM name are used only for logging and not for the logic inside the function the logging should be done outside the function call / the logging should be changed not to use these variables

ctx, cancel := context.WithTimeout(context.Background(), time.Minute*time.Duration(timeout))
defer cancel()
task, err := src.InstantClone(ctx, spec)
if err != nil {
if ctx.Err() == context.DeadlineExceeded {
err = errors.New("timeout waiting for instant clone to complete")
}
return nil, err
}
result, err := task.WaitForResult(ctx, nil)
if err != nil {
if ctx.Err() == context.DeadlineExceeded {
err = errors.New("timeout waiting for instant clone to complete")
}
return nil, err
}
log.Printf("[DEBUG] virtual machine %q: instant clone complete (MOID: %q)", fmt.Sprintf("%s/%s", f.InventoryPath, name), result.Result.(types.ManagedObjectReference).Value)
return FromMOID(c, result.Result.(types.ManagedObjectReference).Value)
}

// Deploy clones a virtual machine from a content library item.
func Deploy(deployData *VCenterDeploy) (*types.ManagedObjectReference, error) {
log.Printf("[DEBUG] virtualmachine.Deploy: Deploying VM from Content Library item.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/datastore"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/folder"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/hostsystem"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/resourcepool"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/storagepod"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/virtualmachine"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/virtualdevice"
"github.com/vmware/govmomi"
Expand Down Expand Up @@ -41,6 +43,11 @@ func VirtualMachineCloneSchema() map[string]*schema.Schema {
Required: true,
Description: "The UUID of the source virtual machine or template.",
},
"instant_clone": {
Type: schema.TypeBool,
Optional: true,
Description: "Whether or not to create a instant clone when cloning. When this option is used, the source VM must be in a running state.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "..an instant.."

},
"linked_clone": {
Type: schema.TypeBool,
Optional: true,
Expand Down Expand Up @@ -309,3 +316,70 @@ func ExpandVirtualMachineCloneSpec(d *schema.ResourceData, c *govmomi.Client) (t
log.Printf("[DEBUG] ExpandVirtualMachineCloneSpec: Clone spec prep complete")
return spec, vm, nil
}

// ExpandVirtualMachineInstantCloneSpec creates an Instant clone spec for an existing virtual machine.
//
// The clone spec built by this function for the clone contains the target
// datastore, the source snapshot in the event of linked clones, and a relocate
// spec that contains the new locations and configuration details of the new
// virtual disks.
func ExpandVirtualMachineInstantCloneSpec(d *schema.ResourceData, client *govmomi.Client) (types.VirtualMachineInstantCloneSpec, *object.VirtualMachine, error) {

var spec types.VirtualMachineInstantCloneSpec
log.Printf("[DEBUG] ExpandVirtualMachineInstantCloneSpec: Preparing InstantClone spec for VM")

//find parent vm
tUUID := d.Get("clone.0.template_uuid").(string) // uuid moid or parent VM name
log.Printf("[DEBUG] ExpandVirtualMachineInstantCloneSpec: Instant Cloning from UUID: %s", tUUID)
vm, err := virtualmachine.FromUUID(client, tUUID)
if err != nil {
return spec, nil, fmt.Errorf("cannot locate virtual machine with UUID %q: %s", tUUID, err)
}
// Populate the datastore only if we have a datastore ID. The ID may not be
// specified in the event a datastore cluster is specified instead.
if dsID, ok := d.GetOk("datastore_id"); ok {
ds, err := datastore.FromID(client, dsID.(string))
if err != nil {
return spec, nil, fmt.Errorf("error locating datastore for VM: %s", err)
}
spec.Location.Datastore = types.NewReference(ds.Reference())
}
// Set the target resource pool.
poolID := d.Get("resource_pool_id").(string)
pool, err := resourcepool.FromID(client, poolID)
if err != nil {
return spec, nil, fmt.Errorf("could not find resource pool ID %q: %s", poolID, err)
}
poolRef := pool.Reference()
spec.Location.Pool = &poolRef

// set the folder // when folder specified
fo, err := folder.VirtualMachineFolderFromObject(client, pool, d.Get("folder").(string))
if err != nil {
return spec, nil, err
}
folderRef := fo.Reference()
spec.Location.Folder = &folderRef

// else if
// datastore cluster
var ds *object.Datastore
if _, ok := d.GetOk("datastore_cluster_id"); ok {
pod, err := storagepod.FromID(client, d.Get("datastore_cluster_id").(string))
if err != nil {
return spec, nil, fmt.Errorf("error getting datastore cluster: %s", err)
}
if pod != nil {
ds, err = storagepod.GetRecommendDatastore(client, fo, d.Get("datastore_cluster_id").(string), 100, pod)
if err != nil {
return spec, nil, err
}
spec.Location.Datastore = types.NewReference(ds.Reference())
}
}
// set the name
spec.Name = d.Get("name").(string)

log.Printf("[DEBUG] ExpandVirtualMachineInstantCloneSpec: Instant Clone spec prep complete")
return spec, vm, nil
}
26 changes: 23 additions & 3 deletions vsphere/resource_vsphere_virtual_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import (
"context"
"errors"
"fmt"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/guestoscustomizations"
"log"
"net"
"os"
"path/filepath"
"strings"
"time"

"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/guestoscustomizations"

"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/contentlibrary"
"github.com/hashicorp/terraform-provider-vsphere/vsphere/internal/helper/ovfdeploy"

Expand Down Expand Up @@ -1494,6 +1495,22 @@ func resourceVSphereVirtualMachineCreateClone(d *schema.ResourceData, meta inter
name := d.Get("name").(string)
timeout := d.Get("clone.0.timeout").(int)
var vm *object.VirtualMachine

// instant Clone
if d.Get("clone.0.instant_clone").(bool) {
log.Printf("[DEBUG] %s: Instant Clone being created from VM", resourceVSphereVirtualMachineIDString(d))
// Expand the clone spec. We get the source VM here too.
cloneSpec, srcVM, err := vmworkflow.ExpandVirtualMachineInstantCloneSpec(d, client)
if err != nil {
return nil, err
}
vm, err = virtualmachine.InstantClone(client, srcVM, fo, name, cloneSpec, timeout)
if err != nil {
return nil, fmt.Errorf("error Instant cloning virtual machine: %s", err)
}
return vm, resourceVSphereVirtualMachinePostDeployChanges(d, meta, vm, false)
}

switch contentlibrary.IsContentLibraryItem(meta.(*Client).restClient, d.Get("clone.0.template_uuid").(string)) {
case true:
deploySpec, err := createVCenterDeploy(d, meta)
Expand Down Expand Up @@ -1760,9 +1777,12 @@ func resourceVSphereVirtualMachinePostDeployChanges(d *schema.ResourceData, meta
}
// Finally time to power on the virtual machine!
pTimeout := time.Duration(d.Get("poweron_timeout").(int)) * time.Second
if err := virtualmachine.PowerOn(vm, pTimeout); err != nil {
return fmt.Errorf("error powering on virtual machine: %s", err)
if vprops.Runtime.PowerState == types.VirtualMachinePowerStatePoweredOff {
if err := virtualmachine.PowerOn(vm, pTimeout); err != nil {
return fmt.Errorf("error powering on virtual machine: %s", err)
}
}

// If we customized, wait on customization.
if cw != nil {
log.Printf("[DEBUG] %s: Waiting for VM customization to complete", resourceVSphereVirtualMachineIDString(d))
Expand Down
113 changes: 113 additions & 0 deletions vsphere/resource_vsphere_virtual_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,24 @@ func TestAccResourceVSphereVirtualMachine_cloneWithBadSizeWithLinkedClone(t *tes
})
}

func TestAccResourceVSphereVirtualMachine_InstantClone(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() {
RunSweepers()
testAccPreCheck(t)
testAccResourceVSphereVirtualMachinePreInstantCloneCheck(t)
},
Providers: testAccProviders,
CheckDestroy: testAccResourceVSphereVirtualMachineCheckExists(false),
Steps: []resource.TestStep{
{
Config: testAccResourceVSphereVirtualMachineConfigInstantClone(),
Check: testAccResourceVSphereVirtualMachineCheckExists(true),
},
},
})
}

func TestAccResourceVSphereVirtualMachine_cloneWithBadSizeWithoutLinkedClone(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() {
Expand Down Expand Up @@ -2688,6 +2706,39 @@ func testAccSriovPreCheck(t *testing.T) {
}
}

func testAccResourceVSphereVirtualMachinePreInstantCloneCheck(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was not necessary the entire testAccResourceVSphereVirtualMachinePreCheck function to be copied, a single function containing just the check
if os.Getenv("TF_VAR_VSPHERE_USE_INSTANT_CLONE") == "" { t.Skip("set TF_VAR_VSPHERE_USE_INSTANT_CLONE to run vsphere_virtual_machine acceptance tests") } could be implemeted and added in the preCheck property like for example it is done in TestAccResourceVSphereVirtualMachine_basic

// Note that TF_VAR_VSPHERE_USE_INSTANT_CLONE is also a variable and its presence
// speeds up tests greatly, but it's not a necessary variable, so we don't
// enforce it here.
if os.Getenv("TF_VAR_VSPHERE_DATACENTER") == "" {
t.Skip("set TF_VAR_VSPHERE_DATACENTER to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_CLUSTER") == "" {
t.Skip("set TF_VAR_VSPHERE_CLUSTER to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_PG_NAME") == "" {
t.Skip("set TF_VAR_VSPHERE_NETWORK_LABEL to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_NFS_DS_NAME") == "" {
t.Skip("set TF_VAR_VSPHERE_NFS_DS_NAME to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_RESOURCE_POOL") == "" {
t.Skip("set TF_VAR_VSPHERE_RESOURCE_POOL to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_TEMPLATE") == "" {
t.Skip("set TF_VAR_VSPHERE_TEMPLATE to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_ESXI1") == "" {
t.Skip("set TF_VAR_VSPHERE_ESXI_HOST to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_ESXI2") == "" {
t.Skip("set TF_VAR_VSPHERE_ESXI_HOST2 to run vsphere_virtual_machine acceptance tests")
}
if os.Getenv("TF_VAR_VSPHERE_USE_INSTANT_CLONE") == "" {
t.Skip("set TF_VAR_VSPHERE_USE_INSTANT_CLONE to run vsphere_virtual_machine acceptance tests")
}
}

func testAccResourceVSphereVirtualMachineCheckExists(expected bool) resource.TestCheckFunc {
return func(s *terraform.State) error {
_, err := testGetVirtualMachine(s, "vm")
Expand Down Expand Up @@ -3596,6 +3647,16 @@ func testAccResourceVSphereVirtualMachineConfigBase() string {
testhelper.ConfigDataRootPortGroup1())
}

func testAccResourceVSphereVirtualMachineInstantCloneConfigBase() string {
return testhelper.CombineConfigs(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minimal set of required resources should be specified here

testhelper.ConfigDataRootDC1(),
testhelper.ConfigDataRootHost1(),
testhelper.ConfigDataRootHost2(),
testhelper.ConfigDataRootDS1(),
testhelper.ConfigDataRootComputeCluster1(),
testhelper.ConfigDataRootVMNet())
}

func testAccResourceVSphereVirtualMachineConfigComputedValue() string {
return fmt.Sprintf(`

Expand Down Expand Up @@ -7218,6 +7279,58 @@ resource "vsphere_virtual_machine" "vm" {
)
}

func testAccResourceVSphereVirtualMachineConfigInstantClone() string {
return fmt.Sprintf(`
%s // Mix and match config

data "vsphere_virtual_machine" "template" {
name = "%s"
datacenter_id = data.vsphere_datacenter.rootdc1.id
}

variable "instant_clone" {
default = "%s"
}

resource "vsphere_virtual_machine" "vm" {
name = "testacc-test"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use minumum required set of properties for the test, for example the hardware like CPU, Memory, Disk are not necessary since a vm is just being copied and those cant be changed + what will happen if the values in the HCL are different then the values from the template VM ?

resource_pool_id = "${vsphere_resource_pool.pool1.id}"
datastore_id = vsphere_nas_datastore.ds1.id

num_cpus = 2
memory = 2048
guest_id = "${data.vsphere_virtual_machine.template.guest_id}"

wait_for_guest_net_timeout = -1

network_interface {
network_id = "${data.vsphere_network.network1.id}"
adapter_type = "${data.vsphere_virtual_machine.template.network_interface_types[0]}"
}

disk {
label = "disk0"
size = "${data.vsphere_virtual_machine.template.disks.0.size}"
eagerly_scrub = "${data.vsphere_virtual_machine.template.disks.0.eagerly_scrub}"
thin_provisioned = "${data.vsphere_virtual_machine.template.disks.0.thin_provisioned}"
}

clone {
template_uuid = "${data.vsphere_virtual_machine.template.id}"
instant_clone = "${var.instant_clone != "" ? "true" : "false" }"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instant_clone should be explicitly set to true since the test will be skipped in every case if the TF_VAR_VSPHERE_USE_INSTANT_CLONE is set to something falsy

}

cdrom {
client_device = true
}
}
`,
testAccResourceVSphereVirtualMachineConfigBase(),
os.Getenv("TF_VAR_VSPHERE_TEMPLATE"),
os.Getenv("TF_VAR_VSPHERE_USE_INSTANT_CLONE"),
)
}

func testAccResourceVSphereVirtualMachineConfigHighSensitivity() string {
return fmt.Sprintf(`

Expand Down
Loading