fix disk behaivor in compute_instance_from_template (#2695)

2024-07-03 08:42:39 +00:00 · 2018-12-18 10:22:13 -08:00 · 2018-12-18 10:22:13 -08:00 · ccd73f3bb6
commit ccd73f3bb6
parent cfbe351e4a
3 changed files with 368 additions and 7 deletions
--- a/google/provider.go
+++ b/google/provider.go
@ -72,8 +72,8 @@ func Provider() terraform.ResourceProvider {
 			"google_compute_forwarding_rule":                  dataSourceGoogleComputeForwardingRule(),
 			"google_compute_image":                            dataSourceGoogleComputeImage(),
 			"google_compute_instance":                         dataSourceGoogleComputeInstance(),
-			"google_compute_instance_group":                   dataSourceGoogleComputeInstanceGroup(),
 			"google_compute_global_address":                   dataSourceGoogleComputeGlobalAddress(),
+			"google_compute_instance_group":                   dataSourceGoogleComputeInstanceGroup(),
 			"google_compute_lb_ip_ranges":                     dataSourceGoogleComputeLbIpRanges(),
 			"google_compute_network":                          dataSourceGoogleComputeNetwork(),
 			"google_compute_regions":                          dataSourceGoogleComputeRegions(),
--- a/google/resource_compute_instance_from_template.go
+++ b/google/resource_compute_instance_from_template.go
@ -6,6 +6,8 @@ import (

 	"github.com/hashicorp/terraform/helper/schema"
 	strcase "github.com/stoewer/go-strcase"
+	computeBeta "google.golang.org/api/compute/v0.beta"
+	compute "google.golang.org/api/compute/v1"
 )

 func resourceComputeInstanceFromTemplate() *schema.Resource {
@ -98,8 +100,26 @@ func resourceComputeInstanceFromTemplateCreate(d *schema.ResourceData, meta inte
 		return err
 	}

-	// Force send all top-level fields in case they're overridden to zero values.
+	tpl, err := ParseInstanceTemplateFieldValue(d.Get("source_instance_template").(string), d, config)
+	if err != nil {
+		return err
+	}
+
+	it, err := config.clientComputeBeta.InstanceTemplates.Get(project, tpl.Name).Do()
+	if err != nil {
+		return err
+	}
+
+	instance.Disks, err = adjustInstanceFromTemplateDisks(d, config, it, zone, project)
+	if err != nil {
+		return err
+	}
+
+	// Force send all top-level fields that have been set in case they're overridden to zero values.
 	// TODO: consider doing so for nested fields as well.
+	// Initialize ForceSendFields to empty so we don't get things that the instance resource
+	// always force-sends.
+	instance.ForceSendFields = []string{}
 	for f, s := range computeInstanceFromTemplateSchema() {
 		// It seems that GetOkExists always returns true for sets.
 		// TODO: confirm this and file issue against Terraform core.
@ -116,11 +136,6 @@ func resourceComputeInstanceFromTemplateCreate(d *schema.ResourceData, meta inte
 		}
 	}

-	tpl, err := ParseInstanceTemplateFieldValue(d.Get("source_instance_template").(string), d, config)
-	if err != nil {
-		return err
-	}
-
 	log.Printf("[INFO] Requesting instance creation")
 	op, err := config.clientComputeBeta.Instances.Insert(project, zone.Name, instance).SourceInstanceTemplate(tpl.RelativeLink()).Do()
 	if err != nil {
@ -140,3 +155,71 @@ func resourceComputeInstanceFromTemplateCreate(d *schema.ResourceData, meta inte

 	return resourceComputeInstanceRead(d, meta)
 }
+
+// Instances have disks spread across multiple schema properties. This function
+// ensures that overriding one of these properties does not override the others.
+func adjustInstanceFromTemplateDisks(d *schema.ResourceData, config *Config, it *computeBeta.InstanceTemplate, zone *compute.Zone, project string) ([]*computeBeta.AttachedDisk, error) {
+	disks := []*computeBeta.AttachedDisk{}
+	if _, hasBootDisk := d.GetOk("boot_disk"); hasBootDisk {
+		bootDisk, err := expandBootDisk(d, config, zone, project)
+		if err != nil {
+			return nil, err
+		}
+		disks = append(disks, bootDisk)
+	} else {
+		// boot disk was not overridden, so use the one from the instance template
+		for _, disk := range it.Properties.Disks {
+			if disk.Boot {
+				if dt := disk.InitializeParams.DiskType; dt != "" {
+					// Instances need a URL for the disk type, but instance templates
+					// only have the name (since they're global).
+					disk.InitializeParams.DiskType = fmt.Sprintf("zones/%s/diskTypes/%s", zone.Name, dt)
+				}
+				disks = append(disks, disk)
+				break
+			}
+		}
+	}
+
+	if _, hasScratchDisk := d.GetOk("scratch_disk"); hasScratchDisk {
+		scratchDisks, err := expandScratchDisks(d, config, zone, project)
+		if err != nil {
+			return nil, err
+		}
+		disks = append(disks, scratchDisks...)
+	} else {
+		// scratch disks were not overridden, so use the ones from the instance template
+		for _, disk := range it.Properties.Disks {
+			if disk.Type == "SCRATCH" {
+				disks = append(disks, disk)
+			}
+		}
+	}
+
+	attachedDisksCount := d.Get("attached_disk.#").(int)
+	if attachedDisksCount > 0 {
+		for i := 0; i < attachedDisksCount; i++ {
+			diskConfig := d.Get(fmt.Sprintf("attached_disk.%d", i)).(map[string]interface{})
+			disk, err := expandAttachedDisk(diskConfig, d, config)
+			if err != nil {
+				return nil, err
+			}
+
+			disks = append(disks, disk)
+		}
+	} else {
+		// attached disks were not overridden, so use the ones from the instance template
+		for _, disk := range it.Properties.Disks {
+			if !disk.Boot && disk.Type != "SCRATCH" {
+				if s := disk.Source; s != "" {
+					// Instances need a URL for the disk source, but instance templates
+					// only have the name (since they're global).
+					disk.Source = fmt.Sprintf("zones/%s/disks/%s", zone.Name, s)
+				}
+				disks = append(disks, disk)
+			}
+		}
+	}
+
+	return disks, nil
+}
--- a/google/resource_compute_instance_from_template_test.go
+++ b/google/resource_compute_instance_from_template_test.go
@ -2,6 +2,7 @@ package google

 import (
 	"fmt"
+	"regexp"
 	"testing"

 	"github.com/hashicorp/terraform/helper/acctest"
@ -37,6 +38,93 @@ func TestAccComputeInstanceFromTemplate_basic(t *testing.T) {
 	})
 }

+func TestAccComputeInstanceFromTemplate_overrideBootDisk(t *testing.T) {
+	t.Parallel()
+
+	var instance compute.Instance
+	instanceName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	templateName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	templateDisk := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	overrideDisk := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	resourceName := "google_compute_instance_from_template.inst"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceFromTemplateDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeInstanceFromTemplate_overrideBootDisk(templateDisk, overrideDisk, templateName, instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeInstanceExists(resourceName, &instance),
+
+					// Check that fields were set based on the template
+					resource.TestCheckResourceAttr(resourceName, "boot_disk.#", "1"),
+					resource.TestMatchResourceAttr(resourceName, "boot_disk.0.source", regexp.MustCompile(overrideDisk)),
+				),
+			},
+		},
+	})
+}
+
+func TestAccComputeInstanceFromTemplate_overrideAttachedDisk(t *testing.T) {
+	t.Parallel()
+
+	var instance compute.Instance
+	instanceName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	templateName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	templateDisk := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	overrideDisk := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	resourceName := "google_compute_instance_from_template.inst"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceFromTemplateDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeInstanceFromTemplate_overrideAttachedDisk(templateDisk, overrideDisk, templateName, instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeInstanceExists(resourceName, &instance),
+
+					// Check that fields were set based on the template
+					resource.TestCheckResourceAttr(resourceName, "attached_disk.#", "1"),
+					resource.TestMatchResourceAttr(resourceName, "attached_disk.0.source", regexp.MustCompile(overrideDisk)),
+				),
+			},
+		},
+	})
+}
+
+func TestAccComputeInstanceFromTemplate_overrideScratchDisk(t *testing.T) {
+	t.Parallel()
+
+	var instance compute.Instance
+	instanceName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	templateName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	templateDisk := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	overrideDisk := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+	resourceName := "google_compute_instance_from_template.inst"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceFromTemplateDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeInstanceFromTemplate_overrideScratchDisk(templateDisk, overrideDisk, templateName, instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeInstanceExists(resourceName, &instance),
+
+					// Check that fields were set based on the template
+					resource.TestCheckResourceAttr(resourceName, "scratch_disk.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "scratch_disk.0.interface", "NVME"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckComputeInstanceFromTemplateDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)

@ -112,3 +200,193 @@ resource "google_compute_instance_from_template" "foobar" {
 }
 `, template, template, instance)
 }
+
+func testAccComputeInstanceFromTemplate_overrideBootDisk(templateDisk, overrideDisk, template, instance string) string {
+	return fmt.Sprintf(`
+data "google_compute_image" "my_image" {
+	family  = "debian-9"
+	project = "debian-cloud"
+}
+
+resource "google_compute_disk" "template_disk" {
+	name = "%s"
+	image = "${data.google_compute_image.my_image.self_link}"
+	size = 10
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_disk" "override_disk" {
+	name = "%s"
+	image = "${data.google_compute_image.my_image.self_link}"
+	size = 20
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_instance_template" "template" {
+	name = "%s"
+	machine_type = "n1-standard-1"
+
+	disk {
+		source_image = "${data.google_compute_image.my_image.self_link}"
+		auto_delete = true
+		disk_size_gb = 100
+		boot = true
+	}
+
+	disk {
+		source = "${google_compute_disk.template_disk.name}"
+		auto_delete = false
+		boot = false
+	}
+
+	network_interface {
+		network = "default"
+	}
+
+	metadata {
+		foo = "bar"
+	}
+
+	can_ip_forward = true
+}
+
+resource "google_compute_instance_from_template" "inst" {
+	name           = "%s"
+	zone           = "us-central1-a"
+
+	source_instance_template = "${google_compute_instance_template.template.self_link}"
+
+	// Overrides
+	boot_disk {
+		source = "${google_compute_disk.override_disk.self_link}"
+	}
+}
+`, templateDisk, overrideDisk, template, instance)
+}
+
+func testAccComputeInstanceFromTemplate_overrideAttachedDisk(templateDisk, overrideDisk, template, instance string) string {
+	return fmt.Sprintf(`
+data "google_compute_image" "my_image" {
+	family  = "debian-9"
+	project = "debian-cloud"
+}
+
+resource "google_compute_disk" "template_disk" {
+	name = "%s"
+	image = "${data.google_compute_image.my_image.self_link}"
+	size = 10
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_disk" "override_disk" {
+	name = "%s"
+	image = "${data.google_compute_image.my_image.self_link}"
+	size = 20
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_instance_template" "template" {
+	name = "%s"
+	machine_type = "n1-standard-1"
+
+	disk {
+		source_image = "${data.google_compute_image.my_image.self_link}"
+		auto_delete = true
+		disk_size_gb = 100
+		boot = true
+	}
+
+	disk {
+		source = "${google_compute_disk.template_disk.name}"
+		auto_delete = false
+		boot = false
+	}
+
+	disk {
+		source_image = "debian-cloud/debian-9"
+		auto_delete = true
+		boot = false
+	}
+
+	network_interface {
+		network = "default"
+	}
+}
+
+resource "google_compute_instance_from_template" "inst" {
+	name           = "%s"
+	zone           = "us-central1-a"
+
+	source_instance_template = "${google_compute_instance_template.template.self_link}"
+
+	// Overrides
+	attached_disk {
+		source = "${google_compute_disk.override_disk.name}"
+	}
+}
+`, templateDisk, overrideDisk, template, instance)
+}
+
+func testAccComputeInstanceFromTemplate_overrideScratchDisk(templateDisk, overrideDisk, template, instance string) string {
+	return fmt.Sprintf(`
+data "google_compute_image" "my_image" {
+	family  = "debian-9"
+	project = "debian-cloud"
+}
+
+resource "google_compute_disk" "template_disk" {
+	name = "%s"
+	image = "${data.google_compute_image.my_image.self_link}"
+	size = 10
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_disk" "override_disk" {
+	name = "%s"
+	image = "${data.google_compute_image.my_image.self_link}"
+	size = 20
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_instance_template" "template" {
+	name = "%s"
+	machine_type = "n1-standard-1"
+
+	disk {
+		source_image = "${data.google_compute_image.my_image.self_link}"
+		auto_delete = true
+		disk_size_gb = 100
+		boot = true
+	}
+
+	disk {
+		type = "SCRATCH"
+		interface = "SCSI"
+		auto_delete = true
+		boot = false
+	}
+
+	network_interface {
+		network = "default"
+	}
+}
+
+resource "google_compute_instance_from_template" "inst" {
+	name           = "%s"
+	zone           = "us-central1-a"
+
+	source_instance_template = "${google_compute_instance_template.template.self_link}"
+
+	// Overrides
+	scratch_disk {
+		interface = "NVME"
+	}
+}
+`, templateDisk, overrideDisk, template, instance)
+}