Merge with latest Hashicorp master

CHANGELOG.md

@@ -2,16 +2,58 @@
 
 FEATURES:
 
+ * **New Data Source:** `aws_elastic_beanstalk_solution_stack` [GH-14944]
+ * **New Data Source:** `aws_elasticache_cluster` [GH-14895]
+ * **New Resource:** `aws_ssm_patch_baseline` [GH-14954]
+ * **New Resource:** `aws_ssm_patch_group` [GH-14954]
  * **New Resource:** `librato_metric` [GH-14562]
  * **New Resource:** `digitalocean_certificate` [GH-14578]
+ * **New Resource:** `vcd_edgegateway_vpn` [GH-13123]
+ * **New Resource:** `vault_mount` [GH-14456]
+ * **New Interpolation Function:** `bcrypt` [GH-14725]
 
 IMPROVEMENTS:
 
 * provider/aws: Expose RDS instance and cluster resource id [GH-14882]
+* provider/aws: Export internal tunnel addresses + document [GH-14835]
+* provider/aws: Fix misleading error in aws_route validation [GH-14972]
+* provider/aws: Support import of aws_lambda_event_source_mapping [GH-14898]
+* Updating the Azure SDK to v10.0.2-beta [GH-14004]
+* provider/azurerm: Ignore case sensivity in Azurerm resource enums [GH-14861]
+* provider/digitalocean: Add support for changing TTL on DigitalOcean domain records. [GH-14805]
+* provider/google: Add ability to import Google Compute persistent disks [GH-14573]
+* provider/google: `google_container_cluster.master_auth` should be optional [GH-14630]
+* provider/google: Add CORS support for `google_storage_bucket` [GH-14695]
+* provider/kubernetes: Upgrade K8S from 1.5.3 to 1.6.1 [GH-14923]
+* provider/kubernetes: Provide more details about why PVC failed to bind [GH-15019]
+* provider/openstack: Add support provider networks [GH-10265]
+* provider/openstack: Allow numerical protocols in security group rules [GH-14917]
+* provider/openstack: Sort request/response headers in debug output [GH-14956]
+* provider/openstack: Add support for FWaaS routerinsertion extension [GH-12589]
+* provider/openstack: Add Terraform version to UserAgent string [GH-14955]
+* provisioner/chef: Use `helpers.shema.Provisoner` in Chef provisioner V2 [GH-14681]
 
 BUG FIXES:
 
+* provider/alicloud: set `alicloud_nat_gateway` zone to be Computed to avoid perpetual diffs [GH-15050]
+* provider/alicloud: set provider to read env vars for access key and secrey key if empty strings [GH-15050]
+* provider/aws: ForceNew aws_launch_config on ebs_block_device change [GH-14899]
+* provider/aws: Avoid crash when EgressOnly IGW disappears [GH-14929]
+* provider/aws: Allow IPv6/IPv4 addresses to coexist [GH-13702]
+* provider/aws: Expect exception on deletion of APIG Usage Plan Key [GH-14958]
+* provider/aws: Fix panic on nil dead_letter_config [GH-14964]
+* provider/aws: Work around IAM eventual consistency in CW Log Subs [GH-14959]
+* provider/aws: Fix ModifyInstanceAttribute on new instances [GH-14992]
+* provider/aws: Fix issue with removing tags in aws_cloudwatch_log_group [GH-14886]
+* provider/azurerm: Preserve the Subnet properties on Update [GH-13877]
+* provider/digitalocean: Refresh DO loadbalancer from state if 404 [GH-14897]
 * provider/github: Do not set incorrect values in github_team data source [GH-14859]
+* provider/google: use a mutex to prevent concurrent sql instance operations [GH-14424]
+* provider/google: Set instances to computed in compute_instance_group [GH-15025]
+* provider/kubernetes: Ignore internal k8s labels in `kubernetes_persistent_volume` [GH-13716]
+* provider/postgresql: Fix for leaking credentials in the provider [GH-14817]
+* provider/postgresql: Drop the optional WITH token from CREATE ROLE. [GH-14864]
+* provider/rancher: refresh rancher_host from state on nil or removed host [GH-15015]
 
 ## 0.9.6 (May 25, 2017)
 

backend/remote-state/consul/backend_state.go

@@ -91,9 +91,10 @@ func (b *Backend) State(name string) (state.State, error) {
 	// Build the state client
 	var stateMgr state.State = &remote.State{
 		Client: &RemoteClient{
-			Client: client,
-			Path:   path,
-			GZip:   gzip,
+			Client:    client,
+			Path:      path,
+			GZip:      gzip,
+			lockState: b.lock,
 		},
 	}
 

backend/remote-state/consul/client.go

@@ -7,6 +7,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log"
+	"sync"
 	"time"
 
 	consulapi "github.com/hashicorp/consul/api"
@@ -26,11 +28,31 @@ type RemoteClient struct {
 	Path   string
 	GZip   bool
 
+	mu sync.Mutex
+	// lockState is true if we're using locks
+	lockState bool
+
+	// The index of the last state we wrote.
+	// If this is > 0, Put will perform a CAS to ensure that the state wasn't
+	// changed during the operation. This is important even with locks, because
+	// if the client loses the lock for some reason, then reacquires it, we
+	// need to make sure that the state was not modified.
+	modifyIndex uint64
+
 	consulLock *consulapi.Lock
 	lockCh     <-chan struct{}
+
+	info *state.LockInfo
+
+	// cancel the goroutine which is monitoring the lock.
+	monitorCancel chan struct{}
+	monitorDone   chan struct{}
 }
 
 func (c *RemoteClient) Get() (*remote.Payload, error) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	pair, _, err := c.Client.KV().Get(c.Path, nil)
 	if err != nil {
 		return nil, err
@@ -39,6 +61,8 @@ func (c *RemoteClient) Get() (*remote.Payload, error) {
 		return nil, nil
 	}
 
+	c.modifyIndex = pair.ModifyIndex
+
 	payload := pair.Value
 	// If the payload starts with 0x1f, it's gzip, not json
 	if len(pair.Value) >= 1 && pair.Value[0] == '\x1f' {
@@ -57,6 +81,9 @@ func (c *RemoteClient) Get() (*remote.Payload, error) {
 }
 
 func (c *RemoteClient) Put(data []byte) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	payload := data
 	if c.GZip {
 		if compressedState, err := compressState(data); err == nil {
@@ -67,14 +94,50 @@ func (c *RemoteClient) Put(data []byte) error {
 	}
 
 	kv := c.Client.KV()
-	_, err := kv.Put(&consulapi.KVPair{
-		Key:   c.Path,
-		Value: payload,
-	}, nil)
-	return err
+
+	// default to doing a CAS
+	verb := consulapi.KVCAS
+
+	// Assume a 0 index doesn't need a CAS for now, since we are either
+	// creating a new state or purposely overwriting one.
+	if c.modifyIndex == 0 {
+		verb = consulapi.KVSet
+	}
+
+	// KV.Put doesn't return the new index, so we use a single operation
+	// transaction to get the new index with a single request.
+	txOps := consulapi.KVTxnOps{
+		&consulapi.KVTxnOp{
+			Verb:  verb,
+			Key:   c.Path,
+			Value: payload,
+			Index: c.modifyIndex,
+		},
+	}
+
+	ok, resp, _, err := kv.Txn(txOps, nil)
+	if err != nil {
+		return err
+	}
+
+	// transaction was rolled back
+	if !ok {
+		return fmt.Errorf("consul CAS failed with transaction errors: %v", resp.Errors)
+	}
+
+	if len(resp.Results) != 1 {
+		// this probably shouldn't happen
+		return fmt.Errorf("expected on 1 response value, got: %d", len(resp.Results))
+	}
+
+	c.modifyIndex = resp.Results[0].ModifyIndex
+	return nil
 }
 
 func (c *RemoteClient) Delete() error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	kv := c.Client.KV()
 	_, err := kv.Delete(c.Path, nil)
 	return err
@@ -113,18 +176,36 @@ func (c *RemoteClient) getLockInfo() (*state.LockInfo, error) {
 }
 
 func (c *RemoteClient) Lock(info *state.LockInfo) (string, error) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if !c.lockState {
+		return "", nil
+	}
+
+	c.info = info
+
+	// These checks only are to ensure we strictly follow the specification.
+	// Terraform shouldn't ever re-lock, so provide errors for the 2 possible
+	// states if this is called.
 	select {
 	case <-c.lockCh:
 		// We had a lock, but lost it.
-		// Since we typically only call lock once, we shouldn't ever see this.
-		return "", errors.New("lost consul lock")
+		return "", errors.New("lost consul lock, cannot re-lock")
 	default:
 		if c.lockCh != nil {
 			// we have an active lock already
 			return "", fmt.Errorf("state %q already locked", c.Path)
 		}
 	}
 
+	return c.lock()
+}
+
+// called after a lock is acquired
+var testLockHook func()
+
+func (c *RemoteClient) lock() (string, error) {
 	if c.consulLock == nil {
 		opts := &consulapi.LockOptions{
 			Key: c.Path + lockSuffix,
@@ -163,19 +244,83 @@ func (c *RemoteClient) Lock(info *state.LockInfo) (string, error) {
 
 	c.lockCh = lockCh
 
-	err = c.putLockInfo(info)
+	err = c.putLockInfo(c.info)
 	if err != nil {
-		if unlockErr := c.Unlock(info.ID); unlockErr != nil {
+		if unlockErr := c.unlock(c.info.ID); unlockErr != nil {
 			err = multierror.Append(err, unlockErr)
 		}
 
 		return "", err
 	}
 
-	return info.ID, nil
+	// Start a goroutine to monitor the lock state.
+	// If we lose the lock to due communication issues with the consul agent,
+	// attempt to immediately reacquire the lock. Put will verify the integrity
+	// of the state by using a CAS operation.
+	c.monitorCancel = make(chan struct{})
+	c.monitorDone = make(chan struct{})
+	go func(cancel, done chan struct{}) {
+		defer func() {
+			close(done)
+		}()
+		select {
+		case <-c.lockCh:
+			for {
+				c.mu.Lock()
+				c.consulLock = nil
+				_, err := c.lock()
+				c.mu.Unlock()
+
+				if err != nil {
+					// We failed to get the lock, keep trying as long as
+					// terraform is running. There may be changes in progress,
+					// so there's no use in aborting. Either we eventually
+					// reacquire the lock, or a Put will fail on a CAS.
+					log.Printf("[ERROR] attempting to reacquire lock: %s", err)
+					time.Sleep(time.Second)
+
+					select {
+					case <-cancel:
+						return
+					default:
+					}
+					continue
+				}
+
+				// if the error was nil, the new lock started a new copy of
+				// this goroutine.
+				return
+			}
+
+		case <-cancel:
+			return
+		}
+	}(c.monitorCancel, c.monitorDone)
+
+	if testLockHook != nil {
+		testLockHook()
+	}
+
+	return c.info.ID, nil
 }
 
 func (c *RemoteClient) Unlock(id string) error {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if !c.lockState {
+		return nil
+	}
+
+	return c.unlock(id)
+}
+
+func (c *RemoteClient) unlock(id string) error {
+	// cancel our monitoring goroutine
+	if c.monitorCancel != nil {
+		close(c.monitorCancel)
+	}
+
 	// this doesn't use the lock id, because the lock is tied to the consul client.
 	if c.consulLock == nil || c.lockCh == nil {
 		return nil
@@ -187,20 +332,28 @@ func (c *RemoteClient) Unlock(id string) error {
 	default:
 	}
 
-	err := c.consulLock.Unlock()
+	kv := c.Client.KV()
+
+	var errs error
+
+	if _, err := kv.Delete(c.Path+lockInfoSuffix, nil); err != nil {
+		errs = multierror.Append(errs, err)
+	}
+
+	if err := c.consulLock.Unlock(); err != nil {
+		errs = multierror.Append(errs, err)
+	}
+
+	// the monitoring goroutine may be in a select on this chan, so we need to
+	// wait for it to return before changing the value.
+	<-c.monitorDone
 	c.lockCh = nil
 
 	// This is only cleanup, and will fail if the lock was immediately taken by
 	// another client, so we don't report an error to the user here.
 	c.consulLock.Destroy()
 
-	kv := c.Client.KV()
-	_, delErr := kv.Delete(c.Path+lockInfoSuffix, nil)
-	if delErr != nil {
-		err = multierror.Append(err, delErr)
-	}
-
-	return err
+	return errs
 }
 
 func compressState(data []byte) ([]byte, error) {