command/state: update and fix the state rm command

command/state_list.go

@@ -40,12 +40,11 @@ func (c *StateListCommand) Run(args []string) int {
 	env := c.Workspace()
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
+		c.Ui.Error(fmt.Sprintf(errStateLoadingState, err))
 		return 1
 	}
-
 	if err := stateMgr.RefreshState(); err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
+		c.Ui.Error(fmt.Sprintf("Failed to refresh state: %s", err))
 		return 1
 	}
 
@@ -65,7 +64,7 @@ func (c *StateListCommand) Run(args []string) int {
 	for _, result := range results {
 		if is, ok := result.Value.(*states.ResourceInstance); ok {
 			if *lookupId == "" || *lookupId == states.LegacyInstanceObjectID(is.Current) {
-				c.Ui.Output(result.Address)
+				c.Ui.Output(result.Address.String())
 			}
 		}
 	}

command/state_meta.go

@@ -1,15 +1,14 @@
 package command
 
 import (
-	"errors"
 	"fmt"
 	"time"
 
+	"github.com/hashicorp/terraform/addrs"
 	backendlocal "github.com/hashicorp/terraform/backend/local"
 	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/states"
 	"github.com/hashicorp/terraform/states/statemgr"
-	"github.com/hashicorp/terraform/terraform"
 )
 
 // StateMeta is the meta struct that should be embedded in state subcommands.
@@ -79,22 +78,40 @@ func (c *StateMeta) State() (state.State, error) {
 	return realState, nil
 }
 
-// filterInstance filters a single instance out of filter results.
+func (c *StateMeta) filter(state *states.State, args []string) ([]*states.FilterResult, error) {
-func (c *StateMeta) filterInstance(rs []*states.FilterResult) (*states.FilterResult, error) {
+	var results []*states.FilterResult
-	var result *states.FilterResult
+
-	for _, r := range rs {
+	filter := &states.Filter{State: state}
-		if _, ok := r.Value.(*terraform.InstanceState); !ok {
+	for _, arg := range args {
-			continue
+		filtered, err := filter.Filter(arg)
+		if err != nil {
+			return nil, err
 		}
 
-		if result != nil {
+	filtered:
-			return nil, errors.New(errStateMultiple)
+		for _, result := range filtered {
+			switch result.Address.(type) {
+			case addrs.ModuleInstance:
+				for _, result := range filtered {
+					if _, ok := result.Address.(addrs.ModuleInstance); ok {
+						results = append(results, result)
+					}
+				}
+				break filtered
+			case addrs.AbsResource:
+				for _, result := range filtered {
+					if _, ok := result.Address.(addrs.AbsResource); ok {
+						results = append(results, result)
+					}
+				}
+				break filtered
+			case addrs.AbsResourceInstance:
+				results = append(results, result)
+			}
+		}
 	}
 
-		result = r
+	return results, nil
-	}
-
-	return result, nil
 }
 
 const errStateMultiple = `Multiple instances found for the given pattern!

command/state_pull.go

@@ -39,11 +39,11 @@ func (c *StatePullCommand) Run(args []string) int {
 	env := c.Workspace()
 	stateMgr, err := b.StateMgr(env)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
+		c.Ui.Error(fmt.Sprintf(errStateLoadingState, err))
 		return 1
 	}
 	if err := stateMgr.RefreshState(); err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
+		c.Ui.Error(fmt.Sprintf("Failed to refresh state: %s", err))
 		return 1
 	}
 
@@ -60,7 +60,7 @@ func (c *StatePullCommand) Run(args []string) int {
 	var buf bytes.Buffer
 	err = statefile.Write(stateFile, &buf)
 	if err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
+		c.Ui.Error(fmt.Sprintf("Failed to write state: %s", err))
 		return 1
 	}
 

command/state_push.go

@@ -78,7 +78,7 @@ func (c *StatePushCommand) Run(args []string) int {
 		return 1
 	}
 	if err := stateMgr.RefreshState(); err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load destination state: %s", err))
+		c.Ui.Error(fmt.Sprintf("Failed to refresh destination state: %s", err))
 		return 1
 	}
 	dstState := stateMgr.State()
@@ -103,7 +103,7 @@ func (c *StatePushCommand) Run(args []string) int {
 		return 1
 	}
 	if err := stateMgr.PersistState(); err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to write state: %s", err))
+		c.Ui.Error(fmt.Sprintf("Failed to persist state: %s", err))
 		return 1
 	}
 

command/state_rm.go

@@ -1,14 +1,14 @@
 package command
 
 import (
-	"bytes"
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/mitchellh/cli"
 
 	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/tfdiags"
+	"github.com/hashicorp/terraform/states"
 )
 
 // StateRmCommand is a Command implementation that shows a single resource.
@@ -23,29 +23,27 @@ func (c *StateRmCommand) Run(args []string) int {
 	}
 
 	cmdFlags := c.Meta.flagSet("state show")
-	var dryRun bool
-	cmdFlags.BoolVar(&dryRun, "dry-run", false, "dry run")
 	cmdFlags.StringVar(&c.backupPath, "backup", "-", "backup")
 	cmdFlags.StringVar(&c.statePath, "state", "", "path")
+	dryRun := cmdFlags.Bool("dry-run", false, "dry run")
 	if err := cmdFlags.Parse(args); err != nil {
 		return cli.RunResultHelp
 	}
 	args = cmdFlags.Args()
 
-	var diags tfdiags.Diagnostics
-
 	if len(args) < 1 {
 		c.Ui.Error("At least one resource address is required.")
 		return 1
 	}
 
+	// Get the state
 	stateMgr, err := c.State()
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateLoadingState, err))
 		return 1
 	}
 	if err := stateMgr.RefreshState(); err != nil {
-		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
+		c.Ui.Error(fmt.Sprintf("Failed to refresh state: %s", err))
 		return 1
 	}
 
@@ -55,86 +53,95 @@ func (c *StateRmCommand) Run(args []string) int {
 		return 1
 	}
 
-	toRemove := make([]addrs.AbsResourceInstance, len(args))
+	results, err := c.filter(state, args)
-	for i, rawAddr := range args {
+	if err != nil {
-		addr, moreDiags := addrs.ParseAbsResourceInstanceStr(rawAddr)
+		c.Ui.Error(fmt.Sprintf(errStateFilter, err))
-		diags = diags.Append(moreDiags)
+		return cli.RunResultHelp
-		toRemove[i] = addr
-	}
-	if diags.HasErrors() {
-		c.showDiagnostics(diags)
-		return 1
 	}
 
-	// We will first check that all of the instances are present, so we can
+	// If we have no results, just exit early, we're not going to do anything.
-	// either remove all of them successfully or make no change at all.
+	// While what happens below is fairly fast, this is an important early
-	// (If we're in dry run mode, this is also where we print out what
+	// exit since the prune below might modify the state more and we don't
-	// we would've done.)
+	// want to modify the state if we don't have to.
-	var currentCount, deposedCount int
+	if len(results) == 0 {
-	var dryRunBuf bytes.Buffer
+		if *dryRun {
-	for _, addr := range toRemove {
+			c.Ui.Output("Would have removed nothing.")
-		is := state.ResourceInstance(addr)
+		} else {
-		if is == nil {
+			c.Ui.Output("No matching resources found.")
-			diags = diags.Append(tfdiags.Sourceless(
-				tfdiags.Error,
-				"No such resource instance in state",
-				fmt.Sprintf("There is no resource instance in the current state with the address %s.", addr),
-			))
-			continue
 		}
-		if is.Current != nil {
+		return 0
-			currentCount++
-		}
-		deposedCount += len(is.Deposed)
-		if dryRun {
-			if is.Current != nil {
-				fmt.Fprintf(&dryRunBuf, "Would remove %s\n", addr)
-			}
-			for k := range is.Deposed {
-				fmt.Fprintf(&dryRunBuf, "Would remove %s deposed object %s\n", addr, k)
-			}
-		}
-	}
-	if diags.HasErrors() {
-		c.showDiagnostics(diags)
-		return 1
 	}
 
-	if dryRun {
+	prefix := "Remove resource "
-		c.Ui.Output(fmt.Sprintf("%s\nWould've removed %d current and %d deposed objects, without -dry-run.", dryRunBuf.String(), currentCount, deposedCount))
+	if *dryRun {
+		prefix = "Would remove resource "
+	}
+
+	var isCount int
+	ss := state.SyncWrapper()
+	for _, result := range results {
+		switch addr := result.Address.(type) {
+		case addrs.ModuleInstance:
+			var output []string
+			for _, rs := range result.Value.(*states.Module).Resources {
+				for k := range rs.Instances {
+					isCount++
+					output = append(output, prefix+rs.Addr.Absolute(addr).Instance(k).String())
+				}
+			}
+			if len(output) > 0 {
+				c.Ui.Output(strings.Join(sort.StringSlice(output), "\n"))
+			}
+			if !*dryRun {
+				ss.RemoveModule(addr)
+			}
+
+		case addrs.AbsResource:
+			var output []string
+			for k := range result.Value.(*states.Resource).Instances {
+				isCount++
+				output = append(output, prefix+addr.Instance(k).String())
+			}
+			if len(output) > 0 {
+				c.Ui.Output(strings.Join(sort.StringSlice(output), "\n"))
+			}
+			if !*dryRun {
+				ss.RemoveResource(addr)
+			}
+
+		case addrs.AbsResourceInstance:
+			isCount++
+			c.Ui.Output(prefix + addr.String())
+			if !*dryRun {
+				ss.ForgetResourceInstanceAll(addr)
+			}
+		}
+	}
+
+	if *dryRun {
+		if isCount == 0 {
+			c.Ui.Output("Would have removed nothing.")
+		}
 		return 0 // This is as far as we go in dry-run mode
 	}
 
-	// Now we will actually remove them. Due to our validation above, we should
+	// Prune the state before writing and persisting it.
-	// succeed in removing every one.
+	state.PruneResourceHusks()
-	// We'll use the "SyncState" wrapper to do this not because we're doing
-	// any concurrent work here (we aren't) but because it guarantees to clean
-	// up any leftover empty module we might leave behind.
-	ss := state.SyncWrapper()
-	for _, addr := range toRemove {
-		ss.ForgetResourceInstanceAll(addr)
-	}
-
-	switch {
-	case currentCount == 0:
-		c.Ui.Output(fmt.Sprintf("Removed %d deposed objects.", deposedCount))
-	case deposedCount == 0:
-		c.Ui.Output(fmt.Sprintf("Removed %d objects.", currentCount))
-	default:
-		c.Ui.Output(fmt.Sprintf("Removed %d current and %d deposed objects.", currentCount, deposedCount))
-	}
 
 	if err := stateMgr.WriteState(state); err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateRmPersist, err))
 		return 1
 	}
-
 	if err := stateMgr.PersistState(); err != nil {
 		c.Ui.Error(fmt.Sprintf(errStateRmPersist, err))
 		return 1
 	}
 
-	c.Ui.Output("Updated state written successfully.")
+	if isCount == 0 {
+		c.Ui.Output("No matching resources found.")
+	} else {
+		c.Ui.Output(fmt.Sprintf("Successfully removed %d resource(s).", isCount))
+	}
 	return 0
 }
 

command/state_rm_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/helper/copy"
 	"github.com/hashicorp/terraform/states"
-	"github.com/hashicorp/terraform/terraform"
 )
 
 func TestStateRm(t *testing.T) {
@@ -170,21 +169,17 @@ func TestStateRmNonExist(t *testing.T) {
 		"-state", statePath,
 		"test_instance.baz", // doesn't exist in the state constructed above
 	}
-	if code := c.Run(args); code != 1 {
+	if code := c.Run(args); code != 0 {
-		t.Errorf("wrong exit status %d; want %d", code, 1)
+		t.Fatalf("expected exit status %d, got: %d", 0, code)
 	}
 
-	if msg := ui.ErrorWriter.String(); !strings.Contains(msg, "No such resource instance in state") {
+	if msg := ui.OutputWriter.String(); !strings.Contains(msg, "No matching resources found") {
-		t.Errorf("not the error we were looking for:\n%s", msg)
+		t.Fatalf("unexpected output:\n%s", msg)
 	}
 
 }
 
 func TestStateRm_backupExplicit(t *testing.T) {
-	td := tempDir(t)
-	defer os.RemoveAll(td)
-	backupPath := filepath.Join(td, "backup")
-
 	state := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(
 			addrs.Resource{
@@ -212,6 +207,7 @@ func TestStateRm_backupExplicit(t *testing.T) {
 		)
 	})
 	statePath := testStateFile(t, state)
+	backupPath := statePath + ".mybackup"
 
 	p := testProvider()
 	ui := new(cli.MockUi)
@@ -280,11 +276,11 @@ func TestStateRm_needsInit(t *testing.T) {
 
 	args := []string{"foo"}
 	if code := c.Run(args); code == 0 {
-		t.Fatal("expected error\noutput:", ui.OutputWriter)
+		t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
 	}
 
 	if !strings.Contains(ui.ErrorWriter.String(), "Initialization") {
-		t.Fatal("expected initialization error, got:\n", ui.ErrorWriter)
+		t.Fatalf("expected initialization error, got:\n%s", ui.ErrorWriter.String())
 	}
 }
 
@@ -294,49 +290,45 @@ func TestStateRm_backendState(t *testing.T) {
 	defer os.RemoveAll(td)
 	defer testChdir(t, td)()
 
-	state := &terraform.State{
+	state := states.BuildState(func(s *states.SyncState) {
-		Modules: []*terraform.ModuleState{
+		s.SetResourceInstanceCurrent(
-			&terraform.ModuleState{
+			addrs.Resource{
-				Path: []string{"root"},
+				Mode: addrs.ManagedResourceMode,
-				Resources: map[string]*terraform.ResourceState{
-					"test_instance.foo": &terraform.ResourceState{
 				Type: "test_instance",
-						Primary: &terraform.InstanceState{
+				Name: "foo",
-							ID: "bar",
+			}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
-							Attributes: map[string]string{
+			&states.ResourceInstanceObjectSrc{
-								"foo": "value",
+				AttrsJSON: []byte(`{"id":"bar","foo":"value","bar":"value"}`),
-								"bar": "value",
+				Status:    states.ObjectReady,
 			},
-						},
+			addrs.ProviderConfig{Type: "test"}.Absolute(addrs.RootModuleInstance),
-					},
+		)
-
+		s.SetResourceInstanceCurrent(
-					"test_instance.bar": &terraform.ResourceState{
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
 				Type: "test_instance",
-						Primary: &terraform.InstanceState{
+				Name: "bar",
-							ID: "foo",
+			}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
-							Attributes: map[string]string{
+			&states.ResourceInstanceObjectSrc{
-								"foo": "value",
+				AttrsJSON: []byte(`{"id":"foo","foo":"value","bar":"value"}`),
-								"bar": "value",
+				Status:    states.ObjectReady,
 			},
-						},
+			addrs.ProviderConfig{Type: "test"}.Absolute(addrs.RootModuleInstance),
-					},
+		)
-				},
+	})
-			},
-		},
-	}
 
-	// the local backend state file is "foo"
 	statePath := "local-state.tfstate"
 	backupPath := "local-state.backup"
 
 	f, err := os.Create(statePath)
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("failed to create state file %s: %s", statePath, err)
 	}
 	defer f.Close()
 
-	if err := terraform.WriteState(state, f); err != nil {
+	err = writeStateForTesting(state, f)
-		t.Fatal(err)
+	if err != nil {
+		t.Fatalf("failed to write state to file %s: %s", statePath, err)
 	}
 
 	p := testProvider()

states/state_filter.go

@@ -91,7 +91,7 @@ func (f *Filter) filterSingle(addr addrs.Targetable) []*FilterResult {
 			if (addr == nil && !m.Addr.IsRoot()) ||
 				(!filter.IsRoot() && (filter.Equal(m.Addr) || filter.IsAncestor(m.Addr))) {
 				results = append(results, &FilterResult{
-					Address: m.Addr.String(),
+					Address: m.Addr,
 					Value:   m,
 				})
 			}
@@ -104,7 +104,7 @@ func (f *Filter) filterSingle(addr addrs.Targetable) []*FilterResult {
 		for _, rs := range m.Resources {
 			if f.relevant(addr, rs.Addr.Absolute(m.Addr), addrs.NoKey) {
 				results = append(results, &FilterResult{
-					Address: rs.Addr.Absolute(m.Addr).String(),
+					Address: rs.Addr.Absolute(m.Addr),
 					Value:   rs,
 				})
 			}
@@ -112,7 +112,7 @@ func (f *Filter) filterSingle(addr addrs.Targetable) []*FilterResult {
 			for key, is := range rs.Instances {
 				if f.relevant(addr, rs.Addr.Absolute(m.Addr), key) {
 					results = append(results, &FilterResult{
-						Address: rs.Addr.Absolute(m.Addr).Instance(key).String(),
+						Address: rs.Addr.Absolute(m.Addr).Instance(key),
 						Value:   is,
 					})
 				}
@@ -144,7 +144,7 @@ func (f *Filter) relevant(filter addrs.Targetable, rs addrs.AbsResource, key add
 // match multiple things within a state (curently modules and resources).
 type FilterResult struct {
 	// Address is the address that can be used to reference this exact result.
-	Address string
+	Address addrs.Targetable
 
 	// Value is the actual value. This must be type switched on. It can be
 	// any either a `Module` or `ResourceInstance`.
@@ -179,8 +179,8 @@ func (s FilterResultSlice) Less(i, j int) bool {
 	a, b := s[i], s[j]
 
 	// If the addresses are different it is just lexographic sorting
-	if a.Address != b.Address {
+	if a.Address.String() != b.Address.String() {
-		return a.Address < b.Address
+		return a.Address.String() < b.Address.String()
 	}
 
 	// Addresses are the same, which means it matters on the type

states/sync.go

@@ -4,9 +4,8 @@ import (
 	"log"
 	"sync"
 
-	"github.com/zclconf/go-cty/cty"
-
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/zclconf/go-cty/cty"
 )
 
 // SyncState is a wrapper around State that provides concurrency-safe access to
@@ -49,6 +48,17 @@ func (s *SyncState) Module(addr addrs.ModuleInstance) *Module {
 	return ret
 }
 
+// RemoveModule removes the entire state for the given module, taking with
+// it any resources associated with the module. This should generally be
+// called only for modules whose resources have all been destroyed, but
+// that is not enforced by this method.
+func (s *SyncState) RemoveModule(addr addrs.ModuleInstance) {
+	s.lock.Lock()
+	defer s.lock.Unlock()
+
+	s.state.RemoveModule(addr)
+}
+
 // OutputValue returns a snapshot of the state of the output value with the
 // given address, or nil if no such output value is tracked.
 //