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

Enhancement: Added resource for snapshot and revert snapshot #107

Merged
merged 13 commits into from
Sep 8, 2017
2 changes: 2 additions & 0 deletions vsphere/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ func Provider() terraform.ResourceProvider {
"vsphere_folder": resourceVSphereFolder(),
"vsphere_virtual_disk": resourceVSphereVirtualDisk(),
"vsphere_virtual_machine": resourceVSphereVirtualMachine(),
"vsphere_snapshot": resourceVSphereSnapshot(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the name of these resources? I think it would be helpful to people who are unfamiliar with how snapshots work to understand that a snapshot is not a snapshot of a disk but a VM. Hence, how about vsphere_virtual_machine_snapshot and vsphere_virtual_machine_snapshot_revert?

"vsphere_snapshot_revert": resourceVSphereRevertSnapshot(),
},

ConfigureFunc: providerConfigure,
Expand Down
77 changes: 77 additions & 0 deletions vsphere/resource_vsphere_revert_snapshot.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package vsphere

import (
"fmt"
"github.com/hashicorp/terraform/helper/schema"
"golang.org/x/net/context"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the context package used here to the standard library context package (versus golang.org/x/net/context)?

"log"
)

func resourceVSphereRevertSnapshot() *schema.Resource {
return &schema.Resource{
Create: resourceVSphereSnapshotRevert,
Delete: resourceVSphereSnapshotDummyDelete,
Read: resourceVSphereSnapshotDummyRead,

Schema: map[string]*schema.Schema{
"datacenter": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"vm_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DefaultFunc: schema.EnvDefaultFunc("VSPHERE_VM_NAME", nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think a DefaultFunc is necessary here - if anything it might lead to confusion or data loss if it's defined by mistake and not documented - can you remove it?

If this was being used for testing, it is probably better to generate your configuration on the fly with a configuration generation function in the _test.go file.

},
"folder": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DefaultFunc: schema.EnvDefaultFunc("VSPHERE_VM_FOLDER", nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with this DefaultFunc - if you can remove it and re-factor the tests it would be great!

},
"snapshot_name": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be changed to something a bit more unique? It seems like FindSnapshot takes a ManagedObjectReference.Value that might be more suitable for sourcing as a snapshot ID. This will allow for the removal of the VM and folder name too and hence should make snapshot lookup easier and less fragile.

Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"suppress_power_on": {
Type: schema.TypeBool,
Optional: true,
ForceNew: true,
},
},
}
}

func resourceVSphereSnapshotRevert(d *schema.ResourceData, meta interface{}) error {
vm, err := findVM(d, meta)
if err != nil {
return fmt.Errorf("Error while getting the VirtualMachine :%s", err)
}
task, err := vm.RevertToSnapshot(context.TODO(), d.Get("snapshot_name").(string), d.Get("suppress_power_on").(bool))
if err != nil {
log.Printf("[ERROR] Error While Creating the Task for Revert Snapshot: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

A note on logs: Terraform currently does nothing with INFO and ERROR messages that is anything more meaningful than DEBUG... can all of these messages be changed to DEBUG?

return fmt.Errorf("Error While Creating the Task for Revert Snapshot: %s", err)
}
log.Printf("[INFO] Task created for Revert Snapshot: %v", task)

err = task.Wait(context.TODO())
if err != nil {
log.Printf("[ERROR] Error While waiting for the Task of Revert Snapshot: %v", err)
return fmt.Errorf("Error While waiting for the Task of Revert Snapshot: %s", err)
}
log.Printf("[INFO] Revert Snapshot completed %v", d.Get("snapshot_name").(string))
d.SetId(d.Get("snapshot_name").(string))
return nil

}

func resourceVSphereSnapshotDummyRead(d *schema.ResourceData, meta interface{}) error {
return nil
}

func resourceVSphereSnapshotDummyDelete(d *schema.ResourceData, meta interface{}) error {
return nil
}
101 changes: 101 additions & 0 deletions vsphere/resource_vsphere_revert_snapshot_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package vsphere

import (
"fmt"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
"github.com/vmware/govmomi"
"github.com/vmware/govmomi/find"
"github.com/vmware/govmomi/vim25/mo"
"golang.org/x/net/context"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the context package used here to the standard library context package (versus golang.org/x/net/context)?

"os"
"testing"
)

func testBasicPreCheckSnapshotRevert(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.

Can this leading space be removed?

testAccPreCheck(t)

if v := os.Getenv("VSPHERE_VM_NAME"); v == "" {
t.Fatal("env variable VSPHERE_VM_NAME must be set for acceptance tests")
}

if v := os.Getenv("VSPHERE_VM_FOLDER"); v == "" {
t.Fatal("env variable VSPHERE_VM_FOLDER must be set for acceptance tests")
}
}

func TestAccVmSnanpshotRevert_Basic(t *testing.T) {
snapshot_name := "SnapshotForTestingTerraform"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckVmSnapshotRevertDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccCheckVmSnapshotRevertConfig_basic,
Check: resource.ComposeTestCheckFunc(
testAccCheckVmCurrentSnapshot("vsphere_snapshot_revert.Test_terraform_cases", snapshot_name),
),
},
},
})
}

func testAccCheckVmSnapshotRevertDestroy(s *terraform.State) error {

return nil
}

func testAccCheckVmCurrentSnapshot(n, snapshot_name string) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]

if !ok {
return fmt.Errorf("Not found: %s", n)
}

if rs.Primary.ID == "" {
return fmt.Errorf("No Vm Snapshot ID is set")
}
client := testAccProvider.Meta().(*govmomi.Client)

dc, err := getDatacenter(client, "")
if err != nil {
return fmt.Errorf("error %s", err)
}
finder := find.NewFinder(client.Client, true)
finder = finder.SetDatacenter(dc)
vm, err := finder.VirtualMachine(context.TODO(), vmPath(os.Getenv("VSPHERE_VM_FOLDER"), os.Getenv("VSPHERE_VM_NAME")))
if err != nil {
return fmt.Errorf("error %s", err)
}

var vm_object mo.VirtualMachine

err = vm.Properties(context.TODO(), vm.Reference(), []string{"snapshot"}, &vm_object)

if err != nil {
return nil
}
current_snap := vm_object.Snapshot.CurrentSnapshot
snapshot, err := vm.FindSnapshot(context.TODO(), snapshot_name)

if err != nil {
return fmt.Errorf("Error while getting the snapshot %v", snapshot)
}
if fmt.Sprintf("<%s>", snapshot) == fmt.Sprintf("<%s>", current_snap) {
return nil
}

return fmt.Errorf("Test Case failed for revert snapshot. Current snapshot does not match to reverted snapshot")
}
}

const testAccCheckVmSnapshotRevertConfig_basic = `
resource "vsphere_snapshot_revert" "Test_terraform_cases"{

snapshot_name = "SnapshotForTestingTerraform"
suppress_power_on = "true"
}`
205 changes: 205 additions & 0 deletions vsphere/resource_vsphere_snapshot.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
package vsphere
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the file name here to resource_vsphere_virtual_machine_snapshot.go?


import (
"fmt"
"log"
"strings"

"github.com/hashicorp/terraform/helper/schema"
"github.com/vmware/govmomi"
"github.com/vmware/govmomi/find"
"github.com/vmware/govmomi/object"
"golang.org/x/net/context"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change the context package used here to the standard library context package (versus golang.org/x/net/context)?

)

func resourceVSphereSnapshot() *schema.Resource {
return &schema.Resource{
Create: resourceVSphereSnapshotCreate,
Read: resourceVSphereSnapshotRead,
Delete: resourceVSphereSnapshotDelete,

Schema: map[string]*schema.Schema{
"datacenter": {
Copy link
Contributor

Choose a reason for hiding this comment

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

After the VM reference has been changed to UUID, you can get rid of this attribute.

Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"vm_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DefaultFunc: schema.EnvDefaultFunc("VSPHERE_VM_NAME", nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think a DefaultFunc is necessary here - if anything it might lead to confusion or data loss if it's defined by mistake and not documented - can you remove it?

If this was being used for testing, it is probably better to generate your configuration on the fly with a configuration generation function in the _test.go file.

},
"folder": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
DefaultFunc: schema.EnvDefaultFunc("VSPHERE_VM_FOLDER", nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with this DefaultFunc - if you can remove it and re-factor the tests it would be great!

},
"snapshot_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"description": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"memory": {
Type: schema.TypeBool,
Required: true,
ForceNew: true,
},
"quiesce": {
Type: schema.TypeBool,
Required: true,
ForceNew: true,
},
"remove_children": {
Type: schema.TypeBool,
Optional: true,
ForceNew: true,
},
"consolidate": {
Type: schema.TypeBool,
Optional: true,
ForceNew: true,
},
},
}
}

func resourceVSphereSnapshotCreate(d *schema.ResourceData, meta interface{}) error {
vm, err := findVM(d, meta)
if err != nil {
return fmt.Errorf("Error while getting the VirtualMachine :%s", err)
}
task, err := vm.CreateSnapshot(context.TODO(), d.Get("snapshot_name").(string), d.Get("description").(string), d.Get("memory").(bool), d.Get("quiesce").(bool))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to get out of the pattern of the vSphere provider using general blocking contexts for these operations - this could potentially mean the user may have to wait 20 minutes for a failed API call to return.

Can you make sure to replace all calls to context.TODO() with a pattern similar to:

ctx, cancel := context.WithTimeout(context.Background(), defaultAPITimeout) // This is 5 mins
defer cancel()
// your function that needs a context here

This will ensure that we put a timeout on function calls that are more reasonable for the operations that they are trying to perform.

Use a separate context for each call. If you find the code getting a little messy, break the code up into different functions. I'd recommend no more than 2 contexts per function with the latter being for waiting on tasks - like what is below:

ctx, cancel := context.WithTimeout(context.Background(), defaultAPITimeout) // This is 5 mins
defer cancel()
task, err := object.FunctionThatNeedsContext(ctx)
if err != nil {
  return err
}
tctx, tcancel := context.WithTimeout(context.Background(), defaultAPITimeout)
defer tcancel()
info, err := task.WaitForResult(tctx, nil)
// Process info.Result here

if err != nil {
log.Printf("[ERROR] Error While Creating the Task for Create Snapshot: %v", err)
return fmt.Errorf(" Error While Creating the Task for Create Snapshot: %s", err)
}
log.Printf("[INFO] Task created for Create Snapshot: %v", task)
err = task.Wait(context.TODO())
if err != nil {
log.Printf("[ERROR] Error While waiting for the Task for Create Snapshot: %v", err)
return fmt.Errorf(" Error While waiting for the Task for Create Snapshot: %s", err)
}
log.Printf("[INFO] Create Snapshot completed %v", d.Get("snapshot_name").(string))
d.SetId(d.Get("snapshot_name").(string))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be changed to something a bit more unique? If it's suitable, I would prefer this to be the value of the MO reference that comes back.

More info: https://pubs.vmware.com/vsphere-6-5/index.jsp#com.vmware.wssdk.apiref.doc/vim.VirtualMachine.html#createSnapshot

This could then be relayed into the vsphere_snapshot_revert resource, making it much less fragile.

d.Set("snapshot_name", d.Get("snapshot_name").(string))
d.Set("vm_name", d.Get("vm_name").(string))
d.Set("folder", d.Get("folder").(string))
d.Set("description", d.Get("description").(string))
d.Set("memory", d.Get("memory").(bool))
d.Set("quiesce", d.Get("quiesce").(bool))

if v, ok := d.GetOk("consolidate"); ok {
d.Set("consolidate", v.(bool))

}
if v, ok := d.GetOk("remove_children"); ok {
d.Set("remove_children", v.(bool))
}

return nil
}

func resourceVSphereSnapshotDelete(d *schema.ResourceData, meta interface{}) error {
vm, err := findVM(d, meta)
if err != nil {
return fmt.Errorf("Error while getting the VirtualMachine :%s", err)
}
resourceVSphereSnapshotRead(d, meta)
if d.Id() == "" {
log.Printf("[ERROR] Error While finding the Snapshot: %v", err)
return nil
}
log.Printf("[INFO] Deleting snapshot with name: %v", d.Get("snapshot_name").(string))
var consolidate_ptr *bool
var remove_children bool

if v, ok := d.GetOk("consolidate"); ok {
consolidate := v.(bool)
consolidate_ptr = &consolidate
} else {

consolidate := true
consolidate_ptr = &consolidate
}
if v, ok := d.GetOk("remove_children"); ok {
remove_children = v.(bool)
} else {

remove_children = false
}

task, err := vm.RemoveSnapshot(context.TODO(), d.Get("snapshot_name").(string), remove_children, consolidate_ptr)

if err != nil {
log.Printf("[ERROR] Error While Creating the Task for Delete Snapshot: %v", err)
return fmt.Errorf("Error While Creating the Task for Delete Snapshot: %s", err)
}
log.Printf("[INFO] Task created for Delete Snapshot: %v", task)

err = task.Wait(context.TODO())
if err != nil {
log.Printf("[ERROR] Error While waiting for the Task of Delete Snapshot: %v", err)
return fmt.Errorf("Error While waiting for the Task of Delete Snapshot: %s", err)
}
log.Printf("[INFO] Delete Snapshot completed %v", d.Get("snapshot_name").(string))

return nil
}

func resourceVSphereSnapshotRead(d *schema.ResourceData, meta interface{}) error {
vm, err := findVM(d, meta)
if err != nil {
return fmt.Errorf("Error while getting the VirtualMachine :%s", err)
}
snapshotName := d.Get("snapshot_name").(string)
snapshot, err := vm.FindSnapshot(context.TODO(), snapshotName)

if err != nil {
if strings.Contains(err.Error(), "No snapshots for this VM") || strings.Contains(err.Error(), "snapshot \""+snapshotName+"\" not found") {
log.Printf("[ERROR] Error While finding the Snapshot: %v", err)
d.SetId("")
return nil
}
log.Printf("[ERROR] Error While finding the Snapshot: %v", err)
return fmt.Errorf("Error while finding the Snapshot :%s", err)
}
log.Printf("[INFO] Snapshot found: %v", snapshot)
return nil
}

func findVM(d *schema.ResourceData, meta interface{}) (*object.VirtualMachine, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Once this is changed to find by UUID, can you move this function to a new file called virtual_machine_helper.go? I want to try and start ensuring that all helpers are organized by their inparticular MO.

Also, make sure the function name is self-documenting - in this case, it would be virtualMachineFromUUID.

client := meta.(*govmomi.Client)
var dc *object.Datacenter
var err error
if v, ok := d.GetOk("datacenter"); ok {
dc, err = getDatacenter(client, v.(string))
} else {
dc, err = getDatacenter(client, "")
}
if err != nil {
log.Printf("[ERROR] Error While getting the DC: %v", err)
return nil, fmt.Errorf("Error While getting the DC: %s", err)
}
log.Printf("[INFO] DataCenter is: %v", dc)
log.Println("[INFO] Getting Finder:")
finder := find.NewFinder(client.Client, true)
log.Printf("[INFO] Finder is: %v", finder)
log.Println("[INFO] Setting DataCenter:")
finder = finder.SetDatacenter(dc)
log.Printf("[INFO] DataCenter is Set: %v", finder)
log.Println("[INFO] Getting VM Object: ")
vm, err := finder.VirtualMachine(context.TODO(), vmPath(d.Get("folder").(string), d.Get("vm_name").(string)))
if err != nil {
log.Printf("[ERROR] Error While getting the Virtual machine object: %v", err)
return nil, err
}
log.Printf("[INFO] Virtual Machine FOUND: %v", vm)
return vm, nil
}
Loading