From 19c1241a500fa3b02d7f7af8b70fb99f48d58b3e Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Mon, 22 Oct 2018 15:52:53 +0200 Subject: [PATCH] command/state: update and fix the state rm command --- command/state_list.go | 7 +- command/state_meta.go | 43 ++++++++---- command/state_pull.go | 6 +- command/state_push.go | 4 +- command/state_rm.go | 139 ++++++++++++++++++++------------------- command/state_rm_test.go | 80 ++++++++++------------ states/state_filter.go | 12 ++-- states/sync.go | 14 +++- 8 files changed, 165 insertions(+), 140 deletions(-) diff --git a/command/state_list.go b/command/state_list.go index 83cb0018e..8925ff5a0 100644 --- a/command/state_list.go +++ b/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()) } } } diff --git a/command/state_meta.go b/command/state_meta.go index 424f8cc05..4cf4034c0 100644 --- a/command/state_meta.go +++ b/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) filterInstance(rs []*states.FilterResult) (*states.FilterResult, error) { - var result *states.FilterResult - for _, r := range rs { - if _, ok := r.Value.(*terraform.InstanceState); !ok { - continue +func (c *StateMeta) filter(state *states.State, args []string) ([]*states.FilterResult, error) { + var results []*states.FilterResult + + filter := &states.Filter{State: state} + for _, arg := range args { + filtered, err := filter.Filter(arg) + if err != nil { + return nil, err } - if result != nil { - return nil, errors.New(errStateMultiple) + filtered: + 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 result, nil + return results, nil } const errStateMultiple = `Multiple instances found for the given pattern! diff --git a/command/state_pull.go b/command/state_pull.go index 4a5f9be2c..6724f6628 100644 --- a/command/state_pull.go +++ b/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 } diff --git a/command/state_push.go b/command/state_push.go index 74928a95e..ff7904914 100644 --- a/command/state_push.go +++ b/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 } diff --git a/command/state_rm.go b/command/state_rm.go index 94a6da6a8..e3b2afd62 100644 --- a/command/state_rm.go +++ b/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)) - for i, rawAddr := range args { - addr, moreDiags := addrs.ParseAbsResourceInstanceStr(rawAddr) - diags = diags.Append(moreDiags) - toRemove[i] = addr - } - if diags.HasErrors() { - c.showDiagnostics(diags) - return 1 + results, err := c.filter(state, args) + if err != nil { + c.Ui.Error(fmt.Sprintf(errStateFilter, err)) + return cli.RunResultHelp } - // We will first check that all of the instances are present, so we can - // either remove all of them successfully or make no change at all. - // (If we're in dry run mode, this is also where we print out what - // we would've done.) - var currentCount, deposedCount int - var dryRunBuf bytes.Buffer - for _, addr := range toRemove { - is := state.ResourceInstance(addr) - if is == nil { - 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 we have no results, just exit early, we're not going to do anything. + // While what happens below is fairly fast, this is an important early + // exit since the prune below might modify the state more and we don't + // want to modify the state if we don't have to. + if len(results) == 0 { + if *dryRun { + c.Ui.Output("Would have removed nothing.") + } else { + c.Ui.Output("No matching resources found.") } - if is.Current != nil { - currentCount++ - } - deposedCount += len(is.Deposed) - if dryRun { - if is.Current != nil { - fmt.Fprintf(&dryRunBuf, "Would remove %s\n", addr) + return 0 + } + + prefix := "Remove resource " + 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()) + } } - for k := range is.Deposed { - fmt.Fprintf(&dryRunBuf, "Would remove %s deposed object %s\n", addr, k) + 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 diags.HasErrors() { - c.showDiagnostics(diags) - return 1 - } - if dryRun { - c.Ui.Output(fmt.Sprintf("%s\nWould've removed %d current and %d deposed objects, without -dry-run.", dryRunBuf.String(), currentCount, deposedCount)) + 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 - // succeed in removing every one. - // 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)) - } + // Prune the state before writing and persisting it. + state.PruneResourceHusks() 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 } diff --git a/command/state_rm_test.go b/command/state_rm_test.go index 40f846b07..7274c8f53 100644 --- a/command/state_rm_test.go +++ b/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 { - t.Errorf("wrong exit status %d; want %d", code, 1) + if code := c.Run(args); code != 0 { + t.Fatalf("expected exit status %d, got: %d", 0, code) } - if msg := ui.ErrorWriter.String(); !strings.Contains(msg, "No such resource instance in state") { - t.Errorf("not the error we were looking for:\n%s", msg) + if msg := ui.OutputWriter.String(); !strings.Contains(msg, "No matching resources found") { + 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{ - Modules: []*terraform.ModuleState{ - &terraform.ModuleState{ - Path: []string{"root"}, - Resources: map[string]*terraform.ResourceState{ - "test_instance.foo": &terraform.ResourceState{ - Type: "test_instance", - Primary: &terraform.InstanceState{ - ID: "bar", - Attributes: map[string]string{ - "foo": "value", - "bar": "value", - }, - }, - }, - - "test_instance.bar": &terraform.ResourceState{ - Type: "test_instance", - Primary: &terraform.InstanceState{ - ID: "foo", - Attributes: map[string]string{ - "foo": "value", - "bar": "value", - }, - }, - }, - }, + state := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"id":"bar","foo":"value","bar":"value"}`), + Status: states.ObjectReady, }, - }, - } + addrs.ProviderConfig{Type: "test"}.Absolute(addrs.RootModuleInstance), + ) + s.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "bar", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{"id":"foo","foo":"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 { - t.Fatal(err) + err = writeStateForTesting(state, f) + if err != nil { + t.Fatalf("failed to write state to file %s: %s", statePath, err) } p := testProvider() diff --git a/states/state_filter.go b/states/state_filter.go index 31104878e..bec5f57f2 100644 --- a/states/state_filter.go +++ b/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 { - return a.Address < b.Address + if a.Address.String() != b.Address.String() { + return a.Address.String() < b.Address.String() } // Addresses are the same, which means it matters on the type diff --git a/states/sync.go b/states/sync.go index abadd0c52..780f83bb8 100644 --- a/states/sync.go +++ b/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. //