Refactored project/instance metadata to use same code whenever possible

Also added optimistic locking to instance metadata

builtin/providers/google/metadata.go

@@ -0,0 +1,71 @@
+package google
+
+import (
+	"fmt"
+
+	"google.golang.org/api/compute/v1"
+)
+
+const FINGERPRINT_RETRIES = 10
+const FINGERPRINT_FAIL = "Invalid fingerprint."
+
+// Since the google compute API uses optimistic locking, there is a chance
+// we need to resubmit our updated metadata. To do this, you need to provide
+// an update function that attempts to submit your metadata
+func MetadataRetryWrapper(update func() error) error {
+	attempt := 0
+	for attempt < FINGERPRINT_RETRIES {
+		err := update()
+		if err != nil && err.Error() == FINGERPRINT_FAIL {
+			attempt++
+		} else {
+			return err
+		}
+	}
+
+	return fmt.Errorf("Failed to update metadata after %d retries", attempt);
+}
+
+// Update the metadata (serverMD) according to the provided diff (oldMDMap v
+// newMDMap).
+func MetadataUpdate(oldMDMap map[string]interface{}, newMDMap map[string]interface{}, serverMD *compute.Metadata) {
+	curMDMap := make(map[string]string)
+	// Load metadata on server into map
+	for _, kv := range serverMD.Items {
+		// If the server state has a key that we had in our old
+		// state, but not in our new state, we should delete it
+		_, okOld := oldMDMap[kv.Key]
+		_, okNew := newMDMap[kv.Key]
+		if okOld && !okNew {
+			continue
+		} else {
+			curMDMap[kv.Key] = *kv.Value
+		}
+	}
+
+	// Insert new metadata into existing metadata (overwriting when needed)
+	for key, val := range newMDMap {
+		curMDMap[key] = val.(string)
+	}
+
+	// Reformat old metadata into a list
+	serverMD.Items = nil
+	for key, val := range curMDMap {
+		v := val;
+		serverMD.Items = append(serverMD.Items, &compute.MetadataItems{
+			Key:   key,
+			Value: &v,
+		})
+	}
+}
+
+// Format metadata from the server data format -> schema data format
+func MetadataFormatSchema(md *compute.Metadata) (map[string]interface{}) {
+	newMD := make(map[string]interface{})
+
+	for _, kv := range md.Items {
+		newMD[kv.Key] = *kv.Value
+	}
+
+	return newMD
+}

builtin/providers/google/resource_compute_instance.go

@@ -256,6 +256,23 @@ func resourceComputeInstance() *schema.Resource {
 	}
 }
 
+func getInstance(config *Config, d *schema.ResourceData) (*compute.Instance, error) {
+	instance, err := config.clientCompute.Instances.Get(
+		config.Project, d.Get("zone").(string), d.Id()).Do()
+	if err != nil {
+		if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
+			// The resource doesn't exist anymore
+			d.SetId("")
+
+			return nil, fmt.Errorf("Resource %s no longer exists", config.Project)
+		}
+
+		return nil, fmt.Errorf("Error reading instance: %s", err)
+	}
+
+	return instance, nil
+}
+
 func resourceOperationWaitZone(
 	config *Config, op *compute.Operation, zone string, activity string) error {
 
@@ -517,17 +534,16 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
 func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
-	instance, err := config.clientCompute.Instances.Get(
+	instance, err := getInstance(config, d);
-		config.Project, d.Get("zone").(string), d.Id()).Do()
 	if err != nil {
-		if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
+		return err
-			// The resource doesn't exist anymore
+	}
-			d.SetId("")
 
-			return nil
+	// Synch metadata 
-		}
+	md := instance.Metadata
 
-		return fmt.Errorf("Error reading instance: %s", err)
+	if err = d.Set("metadata", MetadataFormatSchema(md)); err != nil {
+		return fmt.Errorf("Error setting metadata: %s", err)
 	}
 
 	d.Set("can_ip_forward", instance.CanIpForward)
@@ -655,17 +671,9 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
 
 	zone := d.Get("zone").(string)
 
-	instance, err := config.clientCompute.Instances.Get(
+	instance, err := getInstance(config, d);
-		config.Project, zone, d.Id()).Do()
 	if err != nil {
-		if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
+		return err
-			// The resource doesn't exist anymore
-			d.SetId("")
-
-			return nil
-		}
-
-		return fmt.Errorf("Error reading instance: %s", err)
 	}
 
 	// Enable partial mode for the resource since it is possible
@@ -673,23 +681,38 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
 
 	// If the Metadata has changed, then update that.
 	if d.HasChange("metadata") {
-		metadata, err := resourceInstanceMetadata(d)
+		o, n := d.GetChange("metadata")
-		if err != nil {
+
-			return fmt.Errorf("Error updating metadata: %s", err)
+		updateMD := func() error {
-		}
+			// Reload the instance in the case of a fingerprint mismatch
-		op, err := config.clientCompute.Instances.SetMetadata(
+			instance, err = getInstance(config, d);
-			config.Project, zone, d.Id(), metadata).Do()
+			if err != nil {
-		if err != nil {
+				return err
-			return fmt.Errorf("Error updating metadata: %s", err)
+			}
+
+			md := instance.Metadata
+
+			MetadataUpdate(o.(map[string]interface{}), n.(map[string]interface{}), md)
+
+			if err != nil {
+				return fmt.Errorf("Error updating metadata: %s", err)
+			}
+			op, err := config.clientCompute.Instances.SetMetadata(
+				config.Project, zone, d.Id(), md).Do()
+			if err != nil {
+				return fmt.Errorf("Error updating metadata: %s", err)
+			}
+
+			opErr := resourceOperationWaitZone(config, op, zone, "metadata to update")
+			if opErr != nil {
+				return opErr
+			}
+
+			d.SetPartial("metadata")
+			return nil
 		}
 
-		// 1 5 2
+		MetadataRetryWrapper(updateMD)
-		opErr := resourceOperationWaitZone(config, op, zone, "metadata to update")
-		if opErr != nil {
-			return opErr
-		}
-
-		d.SetPartial("metadata")
 	}
 
 	if d.HasChange("tags") {

builtin/providers/google/resource_compute_project_metadata.go

@@ -30,9 +30,6 @@ func resourceComputeProjectMetadata() *schema.Resource {
 	}
 }
 
-const FINGERPRINT_RETRIES = 10
-const FINGERPRINT_FAIL = "Invalid fingerprint."
-
 func resourceOperationWaitGlobal(config *Config, op *compute.Operation, activity string) error {
 	w := &OperationWaiter{
 		Service: config.clientCompute,
@@ -58,11 +55,9 @@ func resourceOperationWaitGlobal(config *Config, op *compute.Operation, activity
 }
 
 func resourceComputeProjectMetadataCreate(d *schema.ResourceData, meta interface{}) error {
-	attempt := 0
-
 	config := meta.(*Config)
 
-	for attempt < FINGERPRINT_RETRIES {
+	createMD := func() error {
 		// Load project service
 		log.Printf("[DEBUG] Loading project service: %s", config.Project)
 		project, err := config.clientCompute.Projects.Get(config.Project).Do()
@@ -97,20 +92,15 @@ func resourceComputeProjectMetadataCreate(d *schema.ResourceData, meta interface
 
 		log.Printf("[DEBUG] SetCommonMetadata: %d (%s)", op.Id, op.SelfLink)
 
-		// Optimistic locking requires the fingerprint received to match
+		return resourceOperationWaitGlobal(config, op, "SetCommonMetadata")
-		// the fingerprint we send the server, if there is a mismatch then we
-		// are working on old data, and must retry
-		err = resourceOperationWaitGlobal(config, op, "SetCommonMetadata")
-		if err == nil {
-			return resourceComputeProjectMetadataRead(d, meta)
-		} else if err.Error() == FINGERPRINT_FAIL {
-			attempt++
-		} else {
-			return err
-		}
 	}
 
-	return fmt.Errorf("Error, unable to set metadata resource after %d attempts", attempt)
+	err := MetadataRetryWrapper(createMD)
+	if err != nil {
+		return err;
+	}
+
+	return resourceComputeProjectMetadataRead(d, meta);
 }
 
 func resourceComputeProjectMetadataRead(d *schema.ResourceData, meta interface{}) error {
@@ -125,13 +115,7 @@ func resourceComputeProjectMetadataRead(d *schema.ResourceData, meta interface{}
 
 	md := project.CommonInstanceMetadata
 
-	newMD := make(map[string]interface{})
+	if err = d.Set("metadata", MetadataFormatSchema(md)); err != nil {
-
-	for _, kv := range md.Items {
-		newMD[kv.Key] = *kv.Value
-	}
-
-	if err = d.Set("metadata", newMD); err != nil {
 		return fmt.Errorf("Error setting metadata: %s", err)
 	}
 
@@ -141,15 +125,12 @@ func resourceComputeProjectMetadataRead(d *schema.ResourceData, meta interface{}
 }
 
 func resourceComputeProjectMetadataUpdate(d *schema.ResourceData, meta interface{}) error {
-	attempt := 0
-
 	config := meta.(*Config)
 
 	if d.HasChange("metadata") {
 		o, n := d.GetChange("metadata")
-		oMDMap, nMDMap := o.(map[string]interface{}), n.(map[string]interface{})
 
-		for attempt < FINGERPRINT_RETRIES {
+		updateMD := func() error {
 			// Load project service
 			log.Printf("[DEBUG] Loading project service: %s", config.Project)
 			project, err := config.clientCompute.Projects.Get(config.Project).Do()
@@ -159,35 +140,7 @@ func resourceComputeProjectMetadataUpdate(d *schema.ResourceData, meta interface
 
 			md := project.CommonInstanceMetadata
 
-			curMDMap := make(map[string]string)
+		    MetadataUpdate(o.(map[string]interface{}), n.(map[string]interface{}), md)
-			// Load metadata on server into map
-			for _, kv := range md.Items {
-				// If the server state has a key that we had in our old
-				// state, but not in our new state, we should delete it
-				_, okOld := oMDMap[kv.Key]
-				_, okNew := nMDMap[kv.Key]
-				if okOld && !okNew {
-					continue
-				} else {
-					if kv.Value != nil {
-						curMDMap[kv.Key] = *kv.Value
-					}
-				}
-			}
-
-			// Insert new metadata into existing metadata (overwriting when needed)
-			for key, val := range nMDMap {
-				curMDMap[key] = val.(string)
-			}
-
-			// Reformat old metadata into a list
-			md.Items = nil
-			for key, val := range curMDMap {
-				md.Items = append(md.Items, &compute.MetadataItems{
-					Key:   key,
-					Value: &val,
-				})
-			}
 
 			op, err := config.clientCompute.Projects.SetCommonInstanceMetadata(config.Project, md).Do()
 
@@ -200,17 +153,15 @@ func resourceComputeProjectMetadataUpdate(d *schema.ResourceData, meta interface
 			// Optimistic locking requires the fingerprint received to match
 			// the fingerprint we send the server, if there is a mismatch then we
 			// are working on old data, and must retry
-			err = resourceOperationWaitGlobal(config, op, "SetCommonMetadata")
+			return resourceOperationWaitGlobal(config, op, "SetCommonMetadata")
-			if err == nil {
-				return resourceComputeProjectMetadataRead(d, meta)
-			} else if err.Error() == FINGERPRINT_FAIL {
-				attempt++
-			} else {
-				return err
-			}
 		}
 
-		return fmt.Errorf("Error, unable to set metadata resource after %d attempts", attempt)
+		err := MetadataRetryWrapper(updateMD)
+		if err != nil {
+			return err;
+		}
+
+		return resourceComputeProjectMetadataRead(d, meta);
 	}
 
 	return nil