Merge branch 'master' into f-aws-route53zone-tags

* master:
  provider/aws: Fix dependency violation when deleting Internet Gateways
  command/remote-config: failing tests
  update CHANGELOG
  command/remote-config: do a pull with `terraform remote config`
  command/remote-{pull,push}: colorize and show success output
  command/remote-config: lowercase the type so that Atlas works, for example
  command/remote-config: show flag parse errors
  command/remote-config: remove weird error case that shows no error message
  command: when setting up state, only write back if local is newer

CHANGELOG.md

@@ -40,6 +40,8 @@ IMPROVEMENTS:
       like refresh.
   * core: Autoload `terraform.tfvars.json` as well as `terraform.tfvars` [GH-1030]
   * core: `.tf` files that start with a period are now ignored. [GH-1227]
+  * command/remote-config: After enabling remote state, a `pull` is
+      automatically done initially.
   * providers/google: Add `size` option to disk blocks for instances. [GH-1284]
 
 BUG FIXES:

builtin/providers/aws/resource_aws_internet_gateway.go

@@ -199,39 +199,14 @@ func resourceAwsInternetGatewayDetach(d *schema.ResourceData, meta interface{})
 		d.Id(),
 		vpcID.(string))
 
-	wait := true
-	err := ec2conn.DetachInternetGateway(&ec2.DetachInternetGatewayRequest{
-		InternetGatewayID: aws.String(d.Id()),
-		VPCID:             aws.String(vpcID.(string)),
-	})
-	if err != nil {
-		ec2err, ok := err.(aws.APIError)
-		if ok {
-			if ec2err.Code == "InvalidInternetGatewayID.NotFound" {
-				err = nil
-				wait = false
-			} else if ec2err.Code == "Gateway.NotAttached" {
-				err = nil
-				wait = false
-			}
-		}
-
-		if err != nil {
-			return err
-		}
-	}
-
-	if !wait {
-		return nil
-	}
-
 	// Wait for it to be fully detached before continuing
 	log.Printf("[DEBUG] Waiting for internet gateway (%s) to detach", d.Id())
 	stateConf := &resource.StateChangeConf{
-		Pending: []string{"attached", "detaching", "available"},
+		Pending: []string{"detaching"},
 		Target:  "detached",
-		Refresh: IGAttachStateRefreshFunc(ec2conn, d.Id(), "detached"),
-		Timeout: 1 * time.Minute,
+		Refresh: detachIGStateRefreshFunc(ec2conn, d.Id(), vpcID.(string)),
+		Timeout: 2 * time.Minute,
+		Delay:   10 * time.Second,
 	}
 	if _, err := stateConf.WaitForState(); err != nil {
 		return fmt.Errorf(
@@ -242,6 +217,32 @@ func resourceAwsInternetGatewayDetach(d *schema.ResourceData, meta interface{})
 	return nil
 }
 
+// InstanceStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch
+// an EC2 instance.
+func detachIGStateRefreshFunc(conn *ec2.EC2, instanceID, vpcID string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		err := conn.DetachInternetGateway(&ec2.DetachInternetGatewayRequest{
+			InternetGatewayID: aws.String(instanceID),
+			VPCID:             aws.String(vpcID),
+		})
+		if err != nil {
+			ec2err, ok := err.(aws.APIError)
+			if ok {
+				if ec2err.Code == "InvalidInternetGatewayID.NotFound" {
+					return nil, "Not Found", err
+				} else if ec2err.Code == "Gateway.NotAttached" {
+					return "detached", "detached", nil
+				} else if ec2err.Code == "DependencyViolation" {
+					return nil, "detaching", nil
+				}
+			}
+		}
+		// DetachInternetGateway only returns an error, so if it's nil, assume we're
+		// detached
+		return "detached", "detached", nil
+	}
+}
+
 // IGStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch
 // an internet gateway.
 func IGStateRefreshFunc(ec2conn *ec2.EC2, id string) resource.StateRefreshFunc {
@@ -300,10 +301,6 @@ func IGAttachStateRefreshFunc(ec2conn *ec2.EC2, id string, expected string) reso
 
 		ig := &resp.InternetGateways[0]
 
-		if time.Now().Sub(start) > 10*time.Second {
-			return ig, expected, nil
-		}
-
 		if len(ig.Attachments) == 0 {
 			// No attachments, we're detached
 			return ig, "detached", nil

command/remote_config.go

@@ -41,14 +41,12 @@ func (c *RemoteConfigCommand) Run(args []string) int {
 	cmdFlags.Var((*FlagKV)(&config), "backend-config", "config")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
+		c.Ui.Error(fmt.Sprintf("\nError parsing CLI flags: %s", err))
 		return 1
 	}
 
-	// Show help if given no inputs
-	if !c.conf.disableRemote && c.remoteConf.Type == "atlas" && len(config) == 0 {
-		cmdFlags.Usage()
-		return 1
-	}
+	// Lowercase the type
+	c.remoteConf.Type = strings.ToLower(c.remoteConf.Type)
 
 	// Set the local state path
 	c.statePath = c.conf.statePath
@@ -88,29 +86,63 @@ func (c *RemoteConfigCommand) Run(args []string) int {
 		return c.disableRemoteState()
 	}
 
-	// Ensure there is no conflict
+	// Ensure there is no conflict, and then do the correct operation
+	var result int
 	haveCache := !remoteState.Empty()
 	haveLocal := !localState.Empty()
 	switch {
 	case haveCache && haveLocal:
 		c.Ui.Error(fmt.Sprintf("Remote state is enabled, but non-managed state file '%s' is also present!",
 			c.conf.statePath))
-		return 1
+		result = 1
 
 	case !haveCache && !haveLocal:
 		// If we don't have either state file, initialize a blank state file
-		return c.initBlankState()
+		result = c.initBlankState()
 
 	case haveCache && !haveLocal:
 		// Update the remote state target potentially
-		return c.updateRemoteConfig()
+		result = c.updateRemoteConfig()
 
 	case !haveCache && haveLocal:
 		// Enable remote state management
-		return c.enableRemoteState()
+		result = c.enableRemoteState()
 	}
 
-	panic("unhandled case")
+	// If there was an error, return right away
+	if result != 0 {
+		return result
+	}
+
+	// If we're not pulling, then do nothing
+	if !c.conf.pullOnDisable {
+		return result
+	}
+
+	// Otherwise, refresh the state
+	stateResult, err := c.StateRaw(c.StateOpts())
+	if err != nil {
+		c.Ui.Error(fmt.Sprintf(
+			"Error while performing the initial pull. The error message is shown\n"+
+				"below. Note that remote state was properly configured, so you don't\n"+
+				"need to reconfigure. You can now use `push` and `pull` directly.\n"+
+				"\n%s", err))
+		return 1
+	}
+
+	state := stateResult.State
+	if err := state.RefreshState(); err != nil {
+		c.Ui.Error(fmt.Sprintf(
+			"Error while performing the initial pull. The error message is shown\n"+
+				"below. Note that remote state was properly configured, so you don't\n"+
+				"need to reconfigure. You can now use `push` and `pull` directly.\n"+
+				"\n%s", err))
+		return 1
+	}
+
+	c.Ui.Output(c.Colorize().Color(fmt.Sprintf(
+		"[reset][bold][green]Remote state configured and pulled.")))
+	return 0
 }
 
 // disableRemoteState is used to disable remote state management,
@@ -328,9 +360,10 @@ Options:
   -disable               Disables remote state management and migrates the state
                          to the -state path.
 
-  -pull=true             Controls if the remote state is pulled before disabling.
-                         This defaults to true to ensure the latest state is cached
-                         before disabling.
+  -pull=true             If disabling, this controls if the remote state is
+                         pulled before disabling. If enabling, this controls
+                         if the remote state is pulled after enabling. This
+                         defaults to true.
 
   -state=path            Path to read state. Defaults to "terraform.tfstate"
                          unless remote state is enabled.

command/remote_config_test.go

@@ -245,6 +245,7 @@ func TestRemoteConfig_initBlank(t *testing.T) {
 		"-backend=http",
 		"-backend-config", "address=http://example.com",
 		"-backend-config", "access_token=test",
+		"-pull=false",
 	}
 	if code := c.Run(args); code != 0 {
 		t.Fatalf("bad: \n%s", ui.ErrorWriter.String())
@@ -321,6 +322,7 @@ func TestRemoteConfig_updateRemote(t *testing.T) {
 		"-backend=http",
 		"-backend-config", "address=http://example.com",
 		"-backend-config", "access_token=test",
+		"-pull=false",
 	}
 	if code := c.Run(args); code != 0 {
 		t.Fatalf("bad: \n%s", ui.ErrorWriter.String())
@@ -376,6 +378,7 @@ func TestRemoteConfig_enableRemote(t *testing.T) {
 		"-backend=http",
 		"-backend-config", "address=http://example.com",
 		"-backend-config", "access_token=test",
+		"-pull=false",
 	}
 	if code := c.Run(args); code != 0 {
 		t.Fatalf("bad: \n%s", ui.ErrorWriter.String())

command/remote_pull.go

@@ -61,7 +61,8 @@ func (c *RemotePullCommand) Run(args []string) int {
 		c.Ui.Error(fmt.Sprintf("%s", change))
 		return 1
 	} else {
-		c.Ui.Output(fmt.Sprintf("%s", change))
+		c.Ui.Output(c.Colorize().Color(fmt.Sprintf(
+			"[reset][bold][green]%s", change)))
 	}
 
 	return 0

command/remote_pull_test.go

@@ -80,15 +80,6 @@ func testRemoteState(t *testing.T, s *terraform.State, c int) (*terraform.Remote
 	var b64md5 string
 	buf := bytes.NewBuffer(nil)
 
-	if s != nil {
-		enc := json.NewEncoder(buf)
-		if err := enc.Encode(s); err != nil {
-			t.Fatalf("err: %v", err)
-		}
-		md5 := md5.Sum(buf.Bytes())
-		b64md5 = base64.StdEncoding.EncodeToString(md5[:16])
-	}
-
 	cb := func(resp http.ResponseWriter, req *http.Request) {
 		if req.Method == "PUT" {
 			resp.WriteHeader(c)
@@ -98,13 +89,28 @@ func testRemoteState(t *testing.T, s *terraform.State, c int) (*terraform.Remote
 			resp.WriteHeader(404)
 			return
 		}
+
 		resp.Header().Set("Content-MD5", b64md5)
 		resp.Write(buf.Bytes())
 	}
+
 	srv := httptest.NewServer(http.HandlerFunc(cb))
 	remote := &terraform.RemoteState{
 		Type:   "http",
 		Config: map[string]string{"address": srv.URL},
 	}
+
+	if s != nil {
+		// Set the remote data
+		s.Remote = remote
+
+		enc := json.NewEncoder(buf)
+		if err := enc.Encode(s); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		md5 := md5.Sum(buf.Bytes())
+		b64md5 = base64.StdEncoding.EncodeToString(md5[:16])
+	}
+
 	return remote, srv
 }

command/remote_push.go

@@ -68,6 +68,8 @@ func (c *RemotePushCommand) Run(args []string) int {
 		return 1
 	}
 
+	c.Ui.Output(c.Colorize().Color(
+		"[reset][bold][green]State successfully pushed!"))
 	return 0
 }
 

command/state.go

@@ -231,10 +231,20 @@ func remoteState(
 				"Error reloading remote state: {{err}}", err)
 		}
 		switch cache.RefreshResult() {
+		// All the results below can be safely ignored since it means the
+		// pull was successful in some way. Noop = nothing happened.
+		// Init = both are empty. UpdateLocal = local state was older and
+		// updated.
+		//
+		// We don't have to do anything, the pull was successful.
 		case state.CacheRefreshNoop:
 		case state.CacheRefreshInit:
-		case state.CacheRefreshLocalNewer:
 		case state.CacheRefreshUpdateLocal:
+
+		// Our local state has a higher serial number than remote, so we
+		// want to explicitly sync the remote side with our local so that
+		// the remote gets the latest serial number.
+		case state.CacheRefreshLocalNewer:
 			// Write our local state out to the durable storage to start.
 			if err := cache.WriteState(local); err != nil {
 				return nil, errwrap.Wrapf(

terraform/state.go

@@ -344,6 +344,10 @@ func (r *RemoteState) Equals(other *RemoteState) bool {
 	return true
 }
 
+func (r *RemoteState) GoString() string {
+	return fmt.Sprintf("*%#v", *r)
+}
+
 // ModuleState is used to track all the state relevant to a single
 // module. Previous to Terraform 0.3, all state belonged to the "root"
 // module.

website/source/docs/commands/remote-config.html.markdown

@@ -73,8 +73,9 @@ The command-line flags are all optional. The list of available flags are:
 * `-path=path` - Path of the remote state in Consul. Required for the
   Consul backend.
 
-* `-pull=true` - Controls if the remote state is pulled before disabling.
-  This defaults to true to ensure the latest state is cached before disabling.
+* `-pull=true` - Controls if the remote state is pulled before disabling
+  or after enabling. This defaults to true to ensure the latest state
+  is available under both conditions.
 
 * `-state=path` - Path to read state. Defaults to "terraform.tfstate"
   unless remote state is enabled.