Merge pull request #2 from hashicorp/master

Bringing back from Original Fork

.github/CONTRIBUTING.md

@@ -37,6 +37,7 @@ The current list of HashiCorp Providers is as follows:
  * `aws`
  * `azurerm`
  * `google`
+ * `opc`
 
 Our testing standards are the same for both HashiCorp and Community providers,
 and HashiCorp runs full acceptance test suites for every provider nightly to
@@ -201,6 +202,9 @@ Implementing a new resource is a good way to learn more about how Terraform
 interacts with upstream APIs. There are plenty of examples to draw from in the
 existing resources, but you still get to implement something completely new.
 
+ - [ ] __Minimal LOC__: It can be inefficient for both the reviewer
+   and author to go through long feedback cycles on a big PR with many
+   resources. We therefore encourage you to only submit **1 resource at a time**.
  - [ ] __Acceptance tests__: New resources should include acceptance tests
    covering their behavior. See [Writing Acceptance
    Tests](#writing-acceptance-tests) below for a detailed guide on how to
@@ -223,6 +227,11 @@ Implementing a new provider gives Terraform the ability to manage resources in
 a whole new API. It's a larger undertaking, but brings major new functionality
 into Terraform.
 
+ - [ ] __Minimal initial LOC__: Some providers may be big and it can be
+   inefficient for both reviewer & author to go through long feedback cycles
+   on a big PR with many resources. We encourage you to only submit
+   the necessary minimum in a single PR, ideally **just the first resource**
+   of the provider.
  - [ ] __Acceptance tests__: Each provider should include an acceptance test
    suite with tests for each resource should include acceptance tests covering
    its behavior. See [Writing Acceptance Tests](#writing-acceptance-tests) below

.travis.yml

@@ -2,7 +2,7 @@ dist: trusty
 sudo: false
 language: go
 go:
-- 1.8
+- 1.8.1
 
 # add TF_CONSUL_TEST=1 to run consul tests
 # they were causing timouts in travis
@@ -39,4 +39,4 @@ notifications:
 matrix:
   fast_finish: true
   allow_failures:
-  - go: tip
+  - go: tip

Makefile

@@ -75,8 +75,8 @@ cover:
 # vet runs the Go source code static analysis tool `vet` to find
 # any common errors.
 vet:
-	@echo "go vet ."
-	@go vet $$(go list ./... | grep -v vendor/) ; if [ $$? -eq 1 ]; then \
+	@echo 'go vet $$(go list ./... | grep -v /terraform/vendor/)'
+	@go vet $$(go list ./... | grep -v /terraform/vendor/) ; if [ $$? -eq 1 ]; then \
 		echo ""; \
 		echo "Vet found suspicious constructs. Please check the reported constructs"; \
 		echo "and fix them if necessary before submitting the code for review."; \

README.md

@@ -5,7 +5,7 @@ Terraform
 - [![Gitter chat](https://badges.gitter.im/hashicorp-terraform/Lobby.png)](https://gitter.im/hashicorp-terraform/Lobby)
 - Mailing list: [Google Groups](http://groups.google.com/group/terraform-tool)
 
-![Terraform](https://raw.githubusercontent.com/hashicorp/terraform/master/website/source/assets/images/readme.png)
+![Terraform](https://rawgithub.com/hashicorp/terraform/master/website/source/assets/images/logo-hashicorp.svg)
 
 Terraform is a tool for building, changing, and versioning infrastructure safely and efficiently. Terraform can manage existing and popular service providers as well as custom in-house solutions.
 

Vagrantfile

@@ -5,7 +5,7 @@
 VAGRANTFILE_API_VERSION = "2"
 
 # Software version variables
-GOVERSION = "1.8"
+GOVERSION = "1.8.1"
 UBUNTUVERSION = "16.04"
 
 # CPU and RAM can be adjusted depending on your system

backend/backend.go

@@ -7,6 +7,7 @@ package backend
 import (
 	"context"
 	"errors"
+	"time"
 
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/state"
@@ -132,6 +133,9 @@ type Operation struct {
 	// state.Lockers for its duration, and Unlock when complete.
 	LockState bool
 
+	// The duration to retry obtaining a State lock.
+	StateLockTimeout time.Duration
+
 	// Environment is the named state that should be loaded from the Backend.
 	Environment string
 }

backend/local/backend.go

@@ -170,9 +170,30 @@ func (b *Local) DeleteState(name string) error {
 }
 
 func (b *Local) State(name string) (state.State, error) {
+	statePath, stateOutPath, backupPath := b.StatePaths(name)
+
 	// If we have a backend handling state, defer to that.
 	if b.Backend != nil {
-		return b.Backend.State(name)
+		s, err := b.Backend.State(name)
+		if err != nil {
+			return nil, err
+		}
+
+		// make sure we always have a backup state, unless it disabled
+		if backupPath == "" {
+			return s, nil
+		}
+
+		// see if the delegated backend returned a BackupState of its own
+		if s, ok := s.(*state.BackupState); ok {
+			return s, nil
+		}
+
+		s = &state.BackupState{
+			Real: s,
+			Path: backupPath,
+		}
+		return s, nil
 	}
 
 	if s, ok := b.states[name]; ok {
@@ -183,8 +204,6 @@ func (b *Local) State(name string) (state.State, error) {
 		return nil, err
 	}
 
-	statePath, stateOutPath, backupPath := b.StatePaths(name)
-
 	// Otherwise, we need to load the state.
 	var s state.State = &state.LocalState{
 		Path:    statePath,

backend/local/backend_apply.go

@@ -9,7 +9,7 @@ import (
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/backend"
-	clistate "github.com/hashicorp/terraform/command/state"
+	"github.com/hashicorp/terraform/command/clistate"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
@@ -52,9 +52,12 @@ func (b *Local) opApply(
 	}
 
 	if op.LockState {
+		lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout)
+		defer cancel()
+
 		lockInfo := state.NewLockInfo()
 		lockInfo.Operation = op.Type.String()
-		lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize())
+		lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize())
 		if err != nil {
 			runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err)
 			return
@@ -99,7 +102,9 @@ func (b *Local) opApply(
 	doneCh := make(chan struct{})
 	go func() {
 		defer close(doneCh)
-		applyState, applyErr = tfCtx.Apply()
+		_, applyErr = tfCtx.Apply()
+		// we always want the state, even if apply failed
+		applyState = tfCtx.State()
 
 		/*
 			// Record any shadow errors for later
@@ -116,7 +121,7 @@ func (b *Local) opApply(
 	select {
 	case <-ctx.Done():
 		if b.CLI != nil {
-			b.CLI.Output("Interrupt received. Gracefully shutting down...")
+			b.CLI.Output("stopping apply operation...")
 		}
 
 		// Stop execution

backend/local/backend_plan.go

@@ -10,8 +10,8 @@ import (
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/backend"
+	"github.com/hashicorp/terraform/command/clistate"
 	"github.com/hashicorp/terraform/command/format"
-	clistate "github.com/hashicorp/terraform/command/state"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
@@ -61,9 +61,12 @@ func (b *Local) opPlan(
 	}
 
 	if op.LockState {
+		lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout)
+		defer cancel()
+
 		lockInfo := state.NewLockInfo()
 		lockInfo.Operation = op.Type.String()
-		lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize())
+		lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize())
 		if err != nil {
 			runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err)
 			return

backend/local/backend_refresh.go

@@ -9,7 +9,7 @@ import (
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/backend"
-	clistate "github.com/hashicorp/terraform/command/state"
+	"github.com/hashicorp/terraform/command/clistate"
 	"github.com/hashicorp/terraform/config/module"
 	"github.com/hashicorp/terraform/state"
 )
@@ -51,9 +51,12 @@ func (b *Local) opRefresh(
 	}
 
 	if op.LockState {
+		lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout)
+		defer cancel()
+
 		lockInfo := state.NewLockInfo()
 		lockInfo.Operation = op.Type.String()
-		lockID, err := clistate.Lock(opState, lockInfo, b.CLI, b.Colorize())
+		lockID, err := clistate.Lock(lockCtx, opState, lockInfo, b.CLI, b.Colorize())
 		if err != nil {
 			runningOp.Err = errwrap.Wrapf("Error locking state: {{err}}", err)
 			return

backend/local/backend_test.go

@@ -169,29 +169,51 @@ func TestLocal_addAndRemoveStates(t *testing.T) {
 // verify it's being called.
 type testDelegateBackend struct {
 	*Local
+
+	// return a sentinel error on these calls
+	stateErr  bool
+	statesErr bool
+	deleteErr bool
 }
 
 var errTestDelegateState = errors.New("State called")
 var errTestDelegateStates = errors.New("States called")
 var errTestDelegateDeleteState = errors.New("Delete called")
 
 func (b *testDelegateBackend) State(name string) (state.State, error) {
-	return nil, errTestDelegateState
+	if b.stateErr {
+		return nil, errTestDelegateState
+	}
+	s := &state.LocalState{
+		Path:    "terraform.tfstate",
+		PathOut: "terraform.tfstate",
+	}
+	return s, nil
 }
 
 func (b *testDelegateBackend) States() ([]string, error) {
-	return nil, errTestDelegateStates
+	if b.statesErr {
+		return nil, errTestDelegateStates
+	}
+	return []string{"default"}, nil
 }
 
 func (b *testDelegateBackend) DeleteState(name string) error {
-	return errTestDelegateDeleteState
+	if b.deleteErr {
+		return errTestDelegateDeleteState
+	}
+	return nil
 }
 
 // verify that the MultiState methods are dispatched to the correct Backend.
 func TestLocal_multiStateBackend(t *testing.T) {
 	// assign a separate backend where we can read the state
 	b := &Local{
-		Backend: &testDelegateBackend{},
+		Backend: &testDelegateBackend{
+			stateErr:  true,
+			statesErr: true,
+			deleteErr: true,
+		},
 	}
 
 	if _, err := b.State("test"); err != errTestDelegateState {
@@ -205,7 +227,43 @@ func TestLocal_multiStateBackend(t *testing.T) {
 	if err := b.DeleteState("test"); err != errTestDelegateDeleteState {
 		t.Fatal("expected errTestDelegateDeleteState, got:", err)
 	}
+}
+
+// verify that a remote state backend is always wrapped in a BackupState
+func TestLocal_remoteStateBackup(t *testing.T) {
+	// assign a separate backend to mock a remote state backend
+	b := &Local{
+		Backend: &testDelegateBackend{},
+	}
+
+	s, err := b.State("default")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	bs, ok := s.(*state.BackupState)
+	if !ok {
+		t.Fatal("remote state is not backed up")
+	}
 
+	if bs.Path != DefaultStateFilename+DefaultBackupExtension {
+		t.Fatal("bad backup location:", bs.Path)
+	}
+
+	// do the same with a named state, which should use the local env directories
+	s, err = b.State("test")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	bs, ok = s.(*state.BackupState)
+	if !ok {
+		t.Fatal("remote state is not backed up")
+	}
+
+	if bs.Path != filepath.Join(DefaultEnvDir, "test", DefaultStateFilename+DefaultBackupExtension) {
+		t.Fatal("bad backup location:", bs.Path)
+	}
 }
 
 // change into a tmp dir and return a deferable func to change back and cleanup

backend/remote-state/consul/backend_state.go

@@ -102,22 +102,19 @@ func (b *Backend) State(name string) (state.State, error) {
 		stateMgr = &state.LockDisabled{Inner: stateMgr}
 	}
 
-	// Get the locker, which we know always exists
-	stateMgrLocker := stateMgr.(state.Locker)
-
 	// Grab a lock, we use this to write an empty state if one doesn't
 	// exist already. We have to write an empty state as a sentinel value
 	// so States() knows it exists.
 	lockInfo := state.NewLockInfo()
 	lockInfo.Operation = "init"
-	lockId, err := stateMgrLocker.Lock(lockInfo)
+	lockId, err := stateMgr.Lock(lockInfo)
 	if err != nil {
 		return nil, fmt.Errorf("failed to lock state in Consul: %s", err)
 	}
 
 	// Local helper function so we can call it multiple places
 	lockUnlock := func(parent error) error {
-		if err := stateMgrLocker.Unlock(lockId); err != nil {
+		if err := stateMgr.Unlock(lockId); err != nil {
 			return fmt.Errorf(strings.TrimSpace(errStateUnlock), lockId, err)
 		}
 

backend/remote-state/consul/client.go

@@ -121,16 +121,15 @@ func (c *RemoteClient) Lock(info *state.LockInfo) (string, error) {
 	default:
 		if c.lockCh != nil {
 			// we have an active lock already
-			return "", nil
+			return "", fmt.Errorf("state %q already locked", c.Path)
 		}
 	}
 
 	if c.consulLock == nil {
 		opts := &consulapi.LockOptions{
 			Key: c.Path + lockSuffix,
-			// We currently don't procide any options to block terraform and
-			// retry lock acquisition, but we can wait briefly in case the
-			// lock is about to be freed.
+			// only wait briefly, so terraform has the choice to fail fast or
+			// retry as needed.
 			LockWaitTime: time.Second,
 			LockTryOnce:  true,
 		}
@@ -191,6 +190,10 @@ func (c *RemoteClient) Unlock(id string) error {
 	err := c.consulLock.Unlock()
 	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 {