From e510289d2b574db519dd89f2a9bcb355beb29488 Mon Sep 17 00:00:00 2001 From: Thomas Poindessous Date: Mon, 27 Feb 2017 12:45:36 +0100 Subject: [PATCH 1/6] Enable use of URI for snapshot name --- .../providers/google/resource_compute_disk.go | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/builtin/providers/google/resource_compute_disk.go b/builtin/providers/google/resource_compute_disk.go index 2b4148baa..198e8cdd3 100644 --- a/builtin/providers/google/resource_compute_disk.go +++ b/builtin/providers/google/resource_compute_disk.go @@ -3,6 +3,7 @@ package google import ( "fmt" "log" + "regexp" "github.com/hashicorp/terraform/helper/schema" "google.golang.org/api/compute/v1" @@ -129,17 +130,21 @@ func resourceComputeDiskCreate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("snapshot"); ok { snapshotName := v.(string) - log.Printf("[DEBUG] Loading snapshot: %s", snapshotName) - snapshotData, err := config.clientCompute.Snapshots.Get( - project, snapshotName).Do() + match, _ := regexp.MatchString("^http", snapshotName) + if match { + disk.SourceSnapshot = snapshotName + } else { + log.Printf("[DEBUG] Loading snapshot: %s", snapshotName) + snapshotData, err := config.clientCompute.Snapshots.Get( + project, snapshotName).Do() - if err != nil { - return fmt.Errorf( - "Error loading snapshot '%s': %s", - snapshotName, err) + if err != nil { + return fmt.Errorf( + "Error loading snapshot '%s': %s", + snapshotName, err) + } + disk.SourceSnapshot = snapshotData.SelfLink } - - disk.SourceSnapshot = snapshotData.SelfLink } if v, ok := d.GetOk("disk_encryption_key_raw"); ok { From deb9dae35cc5679d32b6973d1123af36f20e0560 Mon Sep 17 00:00:00 2001 From: Thomas Poindessous Date: Mon, 27 Feb 2017 14:26:00 +0100 Subject: [PATCH 2/6] Be more specific on the regexp used to detect URI --- builtin/providers/google/resource_compute_disk.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/providers/google/resource_compute_disk.go b/builtin/providers/google/resource_compute_disk.go index 198e8cdd3..18fc9d04f 100644 --- a/builtin/providers/google/resource_compute_disk.go +++ b/builtin/providers/google/resource_compute_disk.go @@ -130,7 +130,7 @@ func resourceComputeDiskCreate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("snapshot"); ok { snapshotName := v.(string) - match, _ := regexp.MatchString("^http", snapshotName) + match, _ := regexp.MatchString("^https://www.googleapis.com/compute", snapshotName) if match { disk.SourceSnapshot = snapshotName } else { From 674d635926d157ec5de38a625cb03481095b41be Mon Sep 17 00:00:00 2001 From: Thomas Poindessous Date: Mon, 6 Mar 2017 22:59:40 +0100 Subject: [PATCH 3/6] Golint from Atom --- .../providers/google/resource_compute_disk.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/builtin/providers/google/resource_compute_disk.go b/builtin/providers/google/resource_compute_disk.go index 18fc9d04f..04cbca51c 100644 --- a/builtin/providers/google/resource_compute_disk.go +++ b/builtin/providers/google/resource_compute_disk.go @@ -130,20 +130,20 @@ func resourceComputeDiskCreate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("snapshot"); ok { snapshotName := v.(string) - match, _ := regexp.MatchString("^https://www.googleapis.com/compute", snapshotName) + match, _ := regexp.MatchString("^https://www.googleapis.com/compute", snapshotName) if match { - disk.SourceSnapshot = snapshotName + disk.SourceSnapshot = snapshotName } else { - log.Printf("[DEBUG] Loading snapshot: %s", snapshotName) - snapshotData, err := config.clientCompute.Snapshots.Get( - project, snapshotName).Do() + log.Printf("[DEBUG] Loading snapshot: %s", snapshotName) + snapshotData, err := config.clientCompute.Snapshots.Get( + project, snapshotName).Do() - if err != nil { - return fmt.Errorf( - "Error loading snapshot '%s': %s", - snapshotName, err) - } - disk.SourceSnapshot = snapshotData.SelfLink + if err != nil { + return fmt.Errorf( + "Error loading snapshot '%s': %s", + snapshotName, err) + } + disk.SourceSnapshot = snapshotData.SelfLink } } From 4df07f2ed9b83bb59f1c8c68d1ea7932a3d418c6 Mon Sep 17 00:00:00 2001 From: Thomas Poindessous Date: Mon, 6 Mar 2017 23:00:09 +0100 Subject: [PATCH 4/6] Added a test acceptance for the new functionality. GOOGLE_COMPUTE_DISK_SNAPSHOT_URI must be set to a valid snapshot's uri like one of the output of gcloud compute snapshots list --uri GOOGLE_COMPUTE_DISK_SNAPSHOT_URI should be replaced by a proper snapshot made by TF (#11690) --- builtin/providers/google/provider_test.go | 4 +++ .../google/resource_compute_disk_test.go | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/builtin/providers/google/provider_test.go b/builtin/providers/google/provider_test.go index b6f6859e4..a93038fe7 100644 --- a/builtin/providers/google/provider_test.go +++ b/builtin/providers/google/provider_test.go @@ -78,6 +78,10 @@ func testAccPreCheck(t *testing.T) { if v := os.Getenv("GOOGLE_XPN_HOST_PROJECT"); v == "" { t.Fatal("GOOGLE_XPN_HOST_PROJECT must be set for acceptance tests") } + + if v := os.Getenv("GOOGLE_COMPUTE_DISK_SNAPSHOT_URI"); v == "" { + t.Fatal("GOOGLE_COMPUTE_DISK_SNAPSHOT_URI must be set for acceptance tests") + } } func TestProvider_getRegionFromZone(t *testing.T) { diff --git a/builtin/providers/google/resource_compute_disk_test.go b/builtin/providers/google/resource_compute_disk_test.go index 478144e7e..7b1f4042e 100644 --- a/builtin/providers/google/resource_compute_disk_test.go +++ b/builtin/providers/google/resource_compute_disk_test.go @@ -2,6 +2,7 @@ package google import ( "fmt" + "os" "testing" "github.com/hashicorp/terraform/helper/acctest" @@ -30,6 +31,26 @@ func TestAccComputeDisk_basic(t *testing.T) { }) } +func TestAccComputeDisk_from_snapshot_uri(t *testing.T) { + diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + var disk compute.Disk + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeDiskDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeDisk_from_snapshot_uri(diskName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeDiskExists( + "google_compute_disk.foobar", &disk), + ), + }, + }, + }) +} + func TestAccComputeDisk_encryption(t *testing.T) { diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) var disk compute.Disk @@ -130,6 +151,17 @@ resource "google_compute_disk" "foobar" { }`, diskName) } +func testAccComputeDisk_from_snapshot_uri(diskName string) string { + uri := os.Getenv("GOOGLE_COMPUTE_DISK_SNAPSHOT_URI") + return fmt.Sprintf(` +resource "google_compute_disk" "foobar" { + name = "%s" + snapshot = "%s" + type = "pd-ssd" + zone = "us-central1-a" +}`, diskName, uri) +} + func testAccComputeDisk_encryption(diskName string) string { return fmt.Sprintf(` resource "google_compute_disk" "foobar" { From 96a67766bf94fdba15f607a9f620cc7df3675a9b Mon Sep 17 00:00:00 2001 From: Thomas Poindessous Date: Mon, 22 May 2017 23:36:53 +0200 Subject: [PATCH 5/6] Corrected test for generating disk from a snapshot URI from another project --- .../google/resource_compute_disk_test.go | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/builtin/providers/google/resource_compute_disk_test.go b/builtin/providers/google/resource_compute_disk_test.go index 7b1f4042e..c86d42203 100644 --- a/builtin/providers/google/resource_compute_disk_test.go +++ b/builtin/providers/google/resource_compute_disk_test.go @@ -33,6 +33,10 @@ func TestAccComputeDisk_basic(t *testing.T) { func TestAccComputeDisk_from_snapshot_uri(t *testing.T) { diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + firstDiskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + snapshotName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + var xpn_host = os.Getenv("GOOGLE_XPN_HOST_PROJECT") + var disk compute.Disk resource.Test(t, resource.TestCase{ @@ -41,10 +45,10 @@ func TestAccComputeDisk_from_snapshot_uri(t *testing.T) { CheckDestroy: testAccCheckComputeDiskDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccComputeDisk_from_snapshot_uri(diskName), + Config: testAccComputeDisk_from_snapshot_uri(firstDiskName, snapshotName, diskName, xpn_host), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( - "google_compute_disk.foobar", &disk), + "google_compute_disk.seconddisk", &disk), ), }, }, @@ -151,15 +155,29 @@ resource "google_compute_disk" "foobar" { }`, diskName) } -func testAccComputeDisk_from_snapshot_uri(diskName string) string { - uri := os.Getenv("GOOGLE_COMPUTE_DISK_SNAPSHOT_URI") +func testAccComputeDisk_from_snapshot_uri(firstDiskName string, snapshotName string, diskName string, xpn_host string) string { return fmt.Sprintf(` -resource "google_compute_disk" "foobar" { + resource "google_compute_disk" "foobar" { + name = "%s" + image = "debian-8-jessie-v20160803" + size = 50 + type = "pd-ssd" + zone = "us-central1-a" + project = "%s" + } + +resource "google_compute_snapshot" "snapdisk" { + name = "%s" + source_disk = "${google_compute_disk.foobar.name}" + zone = "us-central1-a" + project = "%s" +} +resource "google_compute_disk" "seconddisk" { name = "%s" - snapshot = "%s" + snapshot = "${google_compute_snapshot.snapdisk.self_link}" type = "pd-ssd" zone = "us-central1-a" -}`, diskName, uri) +}`, firstDiskName, xpn_host, snapshotName, xpn_host, diskName) } func testAccComputeDisk_encryption(diskName string) string { From be17d7ac122c29c5c0dce2b82312493b812b03ec Mon Sep 17 00:00:00 2001 From: Paddy Date: Tue, 23 May 2017 14:28:06 -0700 Subject: [PATCH 6/6] Remove required env var, fix test names. We no longer need to set an env var (yaaay!) and our test names use camelCase not snake_case, though that confusion is understandable. --- builtin/providers/google/provider_test.go | 4 ---- builtin/providers/google/resource_compute_disk_test.go | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/builtin/providers/google/provider_test.go b/builtin/providers/google/provider_test.go index a93038fe7..b6f6859e4 100644 --- a/builtin/providers/google/provider_test.go +++ b/builtin/providers/google/provider_test.go @@ -78,10 +78,6 @@ func testAccPreCheck(t *testing.T) { if v := os.Getenv("GOOGLE_XPN_HOST_PROJECT"); v == "" { t.Fatal("GOOGLE_XPN_HOST_PROJECT must be set for acceptance tests") } - - if v := os.Getenv("GOOGLE_COMPUTE_DISK_SNAPSHOT_URI"); v == "" { - t.Fatal("GOOGLE_COMPUTE_DISK_SNAPSHOT_URI must be set for acceptance tests") - } } func TestProvider_getRegionFromZone(t *testing.T) { diff --git a/builtin/providers/google/resource_compute_disk_test.go b/builtin/providers/google/resource_compute_disk_test.go index c86d42203..91f29c985 100644 --- a/builtin/providers/google/resource_compute_disk_test.go +++ b/builtin/providers/google/resource_compute_disk_test.go @@ -31,7 +31,7 @@ func TestAccComputeDisk_basic(t *testing.T) { }) } -func TestAccComputeDisk_from_snapshot_uri(t *testing.T) { +func TestAccComputeDisk_fromSnapshotURI(t *testing.T) { diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) firstDiskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) snapshotName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) @@ -45,7 +45,7 @@ func TestAccComputeDisk_from_snapshot_uri(t *testing.T) { CheckDestroy: testAccCheckComputeDiskDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccComputeDisk_from_snapshot_uri(firstDiskName, snapshotName, diskName, xpn_host), + Config: testAccComputeDisk_fromSnapshotURI(firstDiskName, snapshotName, diskName, xpn_host), Check: resource.ComposeTestCheckFunc( testAccCheckComputeDiskExists( "google_compute_disk.seconddisk", &disk), @@ -155,7 +155,7 @@ resource "google_compute_disk" "foobar" { }`, diskName) } -func testAccComputeDisk_from_snapshot_uri(firstDiskName string, snapshotName string, diskName string, xpn_host string) string { +func testAccComputeDisk_fromSnapshotURI(firstDiskName, snapshotName, diskName, xpn_host string) string { return fmt.Sprintf(` resource "google_compute_disk" "foobar" { name = "%s"