Fix instance/template metadata support

Update our instance template to include metadata_startup_script, to
match our instance resource. Also, we've resolved the diff errors around
metadata.startup-script, and people want to use that to create startup
scripts that don't force a restart when they're changed, so let's stop
disallowing it.

Also, we had a bunch of calls to `schema.ResourceData.Set` that ignored
the errors, so I added error handling for those calls. It's mostly
bundled with this code because I couldn't be sure whether it was the
root of bugs or not, so I took care of it while addressing the startup
script issue.

builtin/providers/google/resource_compute_instance.go

@@ -111,10 +111,9 @@ func resourceComputeInstance() *schema.Resource {
 			},
 
 			"metadata": &schema.Schema{
-				Type:         schema.TypeMap,
+				Type:     schema.TypeMap,
-				Optional:     true,
+				Optional: true,
-				Elem:         schema.TypeString,
+				Elem:     schema.TypeString,
-				ValidateFunc: validateInstanceMetadata,
 			},
 
 			"metadata_startup_script": &schema.Schema{
@@ -634,10 +633,10 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 	md := instance.Metadata
 
 	_md := MetadataFormatSchema(d.Get("metadata").(map[string]interface{}), md)
-	delete(_md, "startup-script")
 
 	if script, scriptExists := d.GetOk("metadata_startup_script"); scriptExists {
 		d.Set("metadata_startup_script", script)
+		delete(_md, "startup-script")
 	}
 
 	if err = d.Set("metadata", _md); err != nil {
@@ -1010,12 +1009,3 @@ func resourceInstanceTags(d *schema.ResourceData) *compute.Tags {
 
 	return tags
 }
-
-func validateInstanceMetadata(v interface{}, k string) (ws []string, es []error) {
-	mdMap := v.(map[string]interface{})
-	if _, ok := mdMap["startup-script"]; ok {
-		es = append(es, fmt.Errorf(
-			"Use metadata_startup_script instead of a startup-script key in %q.", k))
-	}
-	return
-}

builtin/providers/google/resource_compute_instance_template.go

@@ -173,6 +173,12 @@ func resourceComputeInstanceTemplate() *schema.Resource {
 				ForceNew: true,
 			},
 
+			"metadata_startup_script": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			},
+
 			"metadata_fingerprint": &schema.Schema{
 				Type:     schema.TypeString,
 				Computed: true,
@@ -477,6 +483,7 @@ func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interfac
 		return err
 	}
 	instanceProperties.Disks = disks
+
 	metadata, err := resourceInstanceMetadata(d)
 	if err != nil {
 		return err
@@ -693,7 +700,6 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
 			log.Printf("[WARN] Removing Instance Template %q because it's gone", d.Get("name").(string))
 			// The resource doesn't exist anymore
 			d.SetId("")
-
 			return nil
 		}
 
@@ -702,44 +708,89 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
 
 	// Set the metadata fingerprint if there is one.
 	if instanceTemplate.Properties.Metadata != nil {
-		d.Set("metadata_fingerprint", instanceTemplate.Properties.Metadata.Fingerprint)
+		if err = d.Set("metadata_fingerprint", instanceTemplate.Properties.Metadata.Fingerprint); err != nil {
+			return fmt.Errorf("Error setting metadata_fingerprint: %s", err)
+		}
+
+		md := instanceTemplate.Properties.Metadata
+
+		_md := flattenMetadata(md)
+
+		if script, scriptExists := d.GetOk("metadata_startup_script"); scriptExists {
+			if err = d.Set("metadata_startup_script", script); err != nil {
+				return fmt.Errorf("Error setting metadata_startup_script: %s", err)
+			}
+			delete(_md, "startup-script")
+		}
+		if err = d.Set("metadata", _md); err != nil {
+			return fmt.Errorf("Error setting metadata: %s", err)
+		}
 	}
 
 	// Set the tags fingerprint if there is one.
 	if instanceTemplate.Properties.Tags != nil {
-		d.Set("tags_fingerprint", instanceTemplate.Properties.Tags.Fingerprint)
+		if err = d.Set("tags_fingerprint", instanceTemplate.Properties.Tags.Fingerprint); err != nil {
+			return fmt.Errorf("Error setting tags_fingerprint: %s", err)
+		}
+	}
+	if err = d.Set("self_link", instanceTemplate.SelfLink); err != nil {
+		return fmt.Errorf("Error setting self_link: %s", err)
+	}
+	if err = d.Set("name", instanceTemplate.Name); err != nil {
+		return fmt.Errorf("Error setting name: %s", err)
 	}
-	d.Set("self_link", instanceTemplate.SelfLink)
-	d.Set("name", instanceTemplate.Name)
 	if instanceTemplate.Properties.Disks != nil {
-		d.Set("disk", flattenDisks(instanceTemplate.Properties.Disks, d))
+		if err = d.Set("disk", flattenDisks(instanceTemplate.Properties.Disks, d)); err != nil {
+			return fmt.Errorf("Error setting disk: %s", err)
+		}
 	}
-	d.Set("description", instanceTemplate.Description)
+	if err = d.Set("description", instanceTemplate.Description); err != nil {
-	d.Set("machine_type", instanceTemplate.Properties.MachineType)
+		return fmt.Errorf("Error setting description: %s", err)
-	d.Set("can_ip_forward", instanceTemplate.Properties.CanIpForward)
+	}
-	if instanceTemplate.Properties.Metadata != nil {
+	if err = d.Set("machine_type", instanceTemplate.Properties.MachineType); err != nil {
-		d.Set("metadata", flattenMetadata(instanceTemplate.Properties.Metadata))
+		return fmt.Errorf("Error setting machine_type: %s", err)
+	}
+
+	if err = d.Set("can_ip_forward", instanceTemplate.Properties.CanIpForward); err != nil {
+		return fmt.Errorf("Error setting can_ip_forward: %s", err)
+	}
+
+	if err = d.Set("instance_description", instanceTemplate.Properties.Description); err != nil {
+		return fmt.Errorf("Error setting instance_description: %s", err)
+	}
+	if err = d.Set("project", project); err != nil {
+		return fmt.Errorf("Error setting project: %s", err)
 	}
-	d.Set("instance_description", instanceTemplate.Properties.Description)
-	d.Set("project", project)
 	if instanceTemplate.Properties.NetworkInterfaces != nil {
 		networkInterfaces, region := flattenNetworkInterfaces(instanceTemplate.Properties.NetworkInterfaces)
-		d.Set("network_interface", networkInterfaces)
+		if err = d.Set("network_interface", networkInterfaces); err != nil {
+			return fmt.Errorf("Error setting network_interface: %s", err)
+		}
 		// region is where to look up the subnetwork if there is one attached to the instance template
 		if region != "" {
-			d.Set("region", region)
+			if err = d.Set("region", region); err != nil {
+				return fmt.Errorf("Error setting region: %s", err)
+			}
 		}
 	}
 	if instanceTemplate.Properties.Scheduling != nil {
 		scheduling, autoRestart := flattenScheduling(instanceTemplate.Properties.Scheduling)
-		d.Set("scheduling", scheduling)
+		if err = d.Set("scheduling", scheduling); err != nil {
-		d.Set("automatic_restart", autoRestart)
+			return fmt.Errorf("Error setting scheduling: %s", err)
+		}
+		if err = d.Set("automatic_restart", autoRestart); err != nil {
+			return fmt.Errorf("Error setting automatic_restart: %s", err)
+		}
 	}
 	if instanceTemplate.Properties.Tags != nil {
-		d.Set("tags", instanceTemplate.Properties.Tags.Items)
+		if err = d.Set("tags", instanceTemplate.Properties.Tags.Items); err != nil {
+			return fmt.Errorf("Error setting tags: %s", err)
+		}
 	}
 	if instanceTemplate.Properties.ServiceAccounts != nil {
-		d.Set("service_account", flattenServiceAccounts(instanceTemplate.Properties.ServiceAccounts))
+		if err = d.Set("service_account", flattenServiceAccounts(instanceTemplate.Properties.ServiceAccounts)); err != nil {
+			return fmt.Errorf("Error setting service_account: %s", err)
+		}
 	}
 	return nil
 }

builtin/providers/google/resource_compute_instance_template_test.go

@@ -115,6 +115,26 @@ func TestAccComputeInstanceTemplate_subnet_custom(t *testing.T) {
 	})
 }
 
+func TestAccComputeInstanceTemplate_metadata_startup_script(t *testing.T) {
+	var instanceTemplate compute.InstanceTemplate
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceTemplateDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeInstanceTemplate_startup_script,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeInstanceTemplateExists(
+						"google_compute_instance_template.foobar", &instanceTemplate),
+					testAccCheckComputeInstanceTemplateStartupScript(&instanceTemplate, "echo 'Hello'"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckComputeInstanceTemplateDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 
@@ -268,6 +288,31 @@ func testAccCheckComputeInstanceTemplateTag(instanceTemplate *compute.InstanceTe
 	}
 }
 
+func testAccCheckComputeInstanceTemplateStartupScript(instanceTemplate *compute.InstanceTemplate, n string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if instanceTemplate.Properties.Metadata == nil && n == "" {
+			return nil
+		} else if instanceTemplate.Properties.Metadata == nil && n != "" {
+			return fmt.Errorf("Expected metadata.startup-script to be '%s', metadata wasn't set at all", n)
+		}
+		for _, item := range instanceTemplate.Properties.Metadata.Items {
+			if item.Key != "startup-script" {
+				continue
+			}
+			if item.Value != nil && *item.Value == n {
+				return nil
+			} else if item.Value == nil && n == "" {
+				return nil
+			} else if item.Value == nil && n != "" {
+				return fmt.Errorf("Expected metadata.startup-script to be '%s', wasn't set", n)
+			} else if *item.Value != n {
+				return fmt.Errorf("Expected metadata.startup-script to be '%s', got '%s'", n, *item.Value)
+			}
+		}
+		return fmt.Errorf("This should never be reached.")
+	}
+}
+
 var testAccComputeInstanceTemplate_basic = fmt.Sprintf(`
 resource "google_compute_instance_template" "foobar" {
 	name = "instancet-test-%s"
@@ -421,3 +466,26 @@ resource "google_compute_instance_template" "foobar" {
 		foo = "bar"
 	}
 }`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10))
+
+var testAccComputeInstanceTemplate_startup_script = fmt.Sprintf(`
+resource "google_compute_instance_template" "foobar" {
+	name = "instance-test-%s"
+	machine_type = "n1-standard-1"
+
+	disk {
+		source_image = "debian-8-jessie-v20160803"
+		auto_delete = true
+		disk_size_gb = 10
+		boot = true
+	}
+
+	metadata {
+		foo = "bar"
+	}
+
+	network_interface{
+		network = "default"
+	}
+	
+	metadata_startup_script = "echo 'Hello'"
+}`, acctest.RandString(10))

website/source/docs/providers/google/r/compute_instance_template.html.markdown

@@ -136,6 +136,11 @@ The following arguments are supported:
 * `metadata` - (Optional) Metadata key/value pairs to make available from
     within instances created from this template.
 
+* `metadata_startup_script` - (Optional) An alternative to using the
+    startup-script metadata key, mostly to match the compute_instance resource.
+    This replaces the startup-script metadata key on the created instance and 
+    thus the two mechanisms are not allowed to be used simultaneously.
+
 * `network_interface` - (Required) Networks to attach to instances created from
     this template. This can be specified multiple times for multiple networks.
     Structure is documented below.