From 43c7bd648c4536ef2b33b4da49cffae8e0aacc34 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 23 Feb 2017 18:27:16 -0500 Subject: [PATCH] fix sorting of module resources during state mv Module resource were being sorted lexically by name by the state filter. If there are 10 or more resources, the order won't match the index order, and resources will have different indexes in their new location. Sort the FilterResults by index numerically when the names match. Clean up the module String output for visual inspection by sorting Resource name parts numerically when they are an integer value. --- command/state_mv_test.go | 283 ++++++++++++++++++++++++++++++++++++++ terraform/state.go | 47 ++++++- terraform/state_filter.go | 9 +- 3 files changed, 337 insertions(+), 2 deletions(-) diff --git a/command/state_mv_test.go b/command/state_mv_test.go index 9033db4e0..d479b4ccb 100644 --- a/command/state_mv_test.go +++ b/command/state_mv_test.go @@ -370,6 +370,184 @@ func TestStateMv_stateOutNew_count(t *testing.T) { testStateOutput(t, backups[0], testStateMvCount_stateOutOriginal) } +// Modules with more than 10 resources were sorted lexically, causing the +// indexes in the new location to change. +func TestStateMv_stateOutNew_largeCount(t *testing.T) { + state := &terraform.State{ + Modules: []*terraform.ModuleState{ + &terraform.ModuleState{ + Path: []string{"root"}, + Resources: map[string]*terraform.ResourceState{ + "test_instance.foo.0": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo0", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.1": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo1", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.2": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo2", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.3": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo3", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.4": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo4", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.5": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo5", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.6": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo6", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.7": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo7", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.8": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo8", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.9": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo9", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.foo.10": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo10", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + + "test_instance.bar": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + }, + }, + }, + } + + statePath := testStateFile(t, state) + stateOutPath := statePath + ".out" + + p := testProvider() + ui := new(cli.MockUi) + c := &StateMvCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + args := []string{ + "-state", statePath, + "-state-out", stateOutPath, + "test_instance.foo", + "test_instance.bar", + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + // Test it is correct + testStateOutput(t, stateOutPath, testStateMvLargeCount_stateOut) + testStateOutput(t, statePath, testStateMvLargeCount_stateOutSrc) + + // Test we have backups + backups := testStateBackups(t, filepath.Dir(statePath)) + if len(backups) != 1 { + t.Fatalf("bad: %#v", backups) + } + testStateOutput(t, backups[0], testStateMvLargeCount_stateOutOriginal) +} + func TestStateMv_stateOutNew_nestedModule(t *testing.T) { state := &terraform.State{ Modules: []*terraform.ModuleState{ @@ -506,6 +684,111 @@ test_instance.foo.1: foo = value ` +const testStateMvLargeCount_stateOut = ` +test_instance.bar.0: + ID = foo0 + bar = value + foo = value +test_instance.bar.1: + ID = foo1 + bar = value + foo = value +test_instance.bar.2: + ID = foo2 + bar = value + foo = value +test_instance.bar.3: + ID = foo3 + bar = value + foo = value +test_instance.bar.4: + ID = foo4 + bar = value + foo = value +test_instance.bar.5: + ID = foo5 + bar = value + foo = value +test_instance.bar.6: + ID = foo6 + bar = value + foo = value +test_instance.bar.7: + ID = foo7 + bar = value + foo = value +test_instance.bar.8: + ID = foo8 + bar = value + foo = value +test_instance.bar.9: + ID = foo9 + bar = value + foo = value +test_instance.bar.10: + ID = foo10 + bar = value + foo = value +` + +const testStateMvLargeCount_stateOutSrc = ` +test_instance.bar: + ID = bar + bar = value + foo = value +` + +const testStateMvLargeCount_stateOutOriginal = ` +test_instance.bar: + ID = bar + bar = value + foo = value +test_instance.foo.0: + ID = foo0 + bar = value + foo = value +test_instance.foo.1: + ID = foo1 + bar = value + foo = value +test_instance.foo.2: + ID = foo2 + bar = value + foo = value +test_instance.foo.3: + ID = foo3 + bar = value + foo = value +test_instance.foo.4: + ID = foo4 + bar = value + foo = value +test_instance.foo.5: + ID = foo5 + bar = value + foo = value +test_instance.foo.6: + ID = foo6 + bar = value + foo = value +test_instance.foo.7: + ID = foo7 + bar = value + foo = value +test_instance.foo.8: + ID = foo8 + bar = value + foo = value +test_instance.foo.9: + ID = foo9 + bar = value + foo = value +test_instance.foo.10: + ID = foo10 + bar = value + foo = value +` + const testStateMvNestedModule_stateOut = ` module.bar: diff --git a/terraform/state.go b/terraform/state.go index f9cc5602c..2ea827c25 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -1164,7 +1164,8 @@ func (m *ModuleState) String() string { for name, _ := range m.Resources { names = append(names, name) } - sort.Strings(names) + + sort.Sort(resourceNameSort(names)) for _, k := range names { rs := m.Resources[k] @@ -1204,6 +1205,7 @@ func (m *ModuleState) String() string { attrKeys = append(attrKeys, ak) } + sort.Strings(attrKeys) for _, ak := range attrKeys { @@ -1234,6 +1236,7 @@ func (m *ModuleState) String() string { for k, _ := range m.Outputs { ks = append(ks, k) } + sort.Strings(ks) for _, k := range ks { @@ -2034,6 +2037,48 @@ func WriteState(d *State, dst io.Writer) error { return nil } +// resourceNameSort implements the sort.Interface to sort name parts lexically for +// strings and numerically for integer indexes. +type resourceNameSort []string + +func (r resourceNameSort) Len() int { return len(r) } +func (r resourceNameSort) Swap(i, j int) { r[i], r[j] = r[j], r[i] } + +func (r resourceNameSort) Less(i, j int) bool { + iParts := strings.Split(r[i], ".") + jParts := strings.Split(r[j], ".") + + end := len(iParts) + if len(jParts) < end { + end = len(jParts) + } + + for idx := 0; idx < end; idx++ { + if iParts[idx] == jParts[idx] { + continue + } + + // sort on the first non-matching part + iInt, iIntErr := strconv.Atoi(iParts[idx]) + jInt, jIntErr := strconv.Atoi(jParts[idx]) + + switch { + case iIntErr == nil && jIntErr == nil: + // sort numerically if both parts are integers + return iInt < jInt + case iIntErr == nil: + // numbers sort before strings + return true + case jIntErr == nil: + return false + default: + return iParts[idx] < jParts[idx] + } + } + + return false +} + // moduleStateSort implements sort.Interface to sort module states type moduleStateSort []*ModuleState diff --git a/terraform/state_filter.go b/terraform/state_filter.go index 9d3a5fb32..2dcb11b76 100644 --- a/terraform/state_filter.go +++ b/terraform/state_filter.go @@ -34,7 +34,7 @@ func (f *StateFilter) Filter(fs ...string) ([]*StateFilterResult, error) { as[i] = a } - // If we werent given any filters, then we list all + // If we weren't given any filters, then we list all if len(fs) == 0 { as = append(as, &ResourceAddress{Index: -1}) } @@ -250,6 +250,13 @@ func (s StateFilterResultSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] } func (s StateFilterResultSlice) Less(i, j int) bool { a, b := s[i], s[j] + // if these address contain an index, we want to sort by index rather than name + addrA, errA := ParseResourceAddress(a.Address) + addrB, errB := ParseResourceAddress(b.Address) + if errA == nil && errB == nil && addrA.Name == addrB.Name && addrA.Index != addrB.Index { + return addrA.Index < addrB.Index + } + // If the addresses are different it is just lexographic sorting if a.Address != b.Address { return a.Address < b.Address