From f915c5d9570b746919eda6104b34515e090abd59 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 24 Apr 2020 10:33:55 -0400 Subject: [PATCH] remove EachMode from resource state Due to the fact that resources can transition between each modes, trying to track the mode for a resource as a whole in state doesn't work, because there may be instances with a mode different from the resource as a whole. This is difficult for core to track, as this metadata being changed as a side effect from multiple places often causes core to see the incorrect mode when evaluating instances. Since core can always determine the correct mode to evaluate from the configuration, we don't need to interrogate the state to know the mode. Once core no longer needs to reference EachMode from states, the resource state can simply be a container for instances, and doesn't need to try and track the "current" mode. --- states/eachmode_string.go | 35 -------------------------------- states/module.go | 25 ++++++++++------------- states/resource.go | 31 ---------------------------- states/state_deepcopy.go | 1 - states/state_test.go | 1 - states/statefile/version4.go | 39 ++---------------------------------- states/sync.go | 4 ++-- 7 files changed, 15 insertions(+), 121 deletions(-) delete mode 100644 states/eachmode_string.go diff --git a/states/eachmode_string.go b/states/eachmode_string.go deleted file mode 100644 index 0dc73499a..000000000 --- a/states/eachmode_string.go +++ /dev/null @@ -1,35 +0,0 @@ -// Code generated by "stringer -type EachMode"; DO NOT EDIT. - -package states - -import "strconv" - -func _() { - // An "invalid array index" compiler error signifies that the constant values have changed. - // Re-run the stringer command to generate them again. - var x [1]struct{} - _ = x[NoEach-0] - _ = x[EachList-76] - _ = x[EachMap-77] -} - -const ( - _EachMode_name_0 = "NoEach" - _EachMode_name_1 = "EachListEachMap" -) - -var ( - _EachMode_index_1 = [...]uint8{0, 8, 15} -) - -func (i EachMode) String() string { - switch { - case i == 0: - return _EachMode_name_0 - case 76 <= i && i <= 77: - i -= 76 - return _EachMode_name_1[_EachMode_index_1[i]:_EachMode_index_1[i+1]] - default: - return "EachMode(" + strconv.FormatInt(int64(i), 10) + ")" - } -} diff --git a/states/module.go b/states/module.go index ddfada9ae..760625e05 100644 --- a/states/module.go +++ b/states/module.go @@ -51,10 +51,10 @@ func (ms *Module) ResourceInstance(addr addrs.ResourceInstance) *ResourceInstanc return rs.Instance(addr.Key) } -// SetResourceMeta updates the resource-level metadata for the resource +// SetResourceProvider updates the resource-level metadata for the resource // with the given address, creating the resource state for it if it doesn't // already exist. -func (ms *Module) SetResourceMeta(addr addrs.Resource, eachMode EachMode, provider addrs.AbsProviderConfig) { +func (ms *Module) SetResourceProvider(addr addrs.Resource, provider addrs.AbsProviderConfig) { rs := ms.Resource(addr) if rs == nil { rs = &Resource{ @@ -64,7 +64,6 @@ func (ms *Module) SetResourceMeta(addr addrs.Resource, eachMode EachMode, provid ms.Resources[addr.String()] = rs } - rs.EachMode = eachMode rs.ProviderConfig = provider } @@ -97,9 +96,7 @@ func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *R if obj == nil && rs != nil { // does the resource have any other objects? // if not then delete the whole resource - // When deleting the resource, ensure that its EachMode is NoEach, - // as a resource with EachList or EachMap can have 0 instances and be valid - if rs.EachMode == NoEach && len(rs.Instances) == 0 { + if len(rs.Instances) == 0 { delete(ms.Resources, addr.Resource.String()) return } @@ -115,7 +112,7 @@ func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *R // If we have no objects at all then we'll clean up. delete(rs.Instances, addr.Key) // Delete the resource if it has no instances, but only if NoEach - if rs.EachMode == NoEach && len(rs.Instances) == 0 { + if len(rs.Instances) == 0 { delete(ms.Resources, addr.Resource.String()) return } @@ -125,7 +122,7 @@ func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *R } if rs == nil && obj != nil { // We don't have have a resource so make one, which is a side effect of setResourceMeta - ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider) + ms.SetResourceProvider(addr.Resource, provider) // now we have a resource! so update the rs value to point to it rs = ms.Resource(addr.Resource) } @@ -134,8 +131,8 @@ func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *R if is == nil { // if we don't have a resource, create one and add to the instances is = rs.CreateInstance(addr.Key) - // update the resource meta because we have a new instance, so EachMode may have changed - ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider) + // update the resource meta because we have a new + ms.SetResourceProvider(addr.Resource, provider) } // Update the resource's ProviderConfig, in case the provider has updated rs.ProviderConfig = provider @@ -159,7 +156,7 @@ func (ms *Module) SetResourceInstanceCurrent(addr addrs.ResourceInstance, obj *R // the instance is left with no objects after this operation then it will // be removed from its containing resource altogether. func (ms *Module) SetResourceInstanceDeposed(addr addrs.ResourceInstance, key DeposedKey, obj *ResourceInstanceObjectSrc, provider addrs.AbsProviderConfig) { - ms.SetResourceMeta(addr.Resource, eachModeForInstanceKey(addr.Key), provider) + ms.SetResourceProvider(addr.Resource, provider) rs := ms.Resource(addr.Resource) is := rs.EnsureInstance(addr.Key) @@ -173,7 +170,7 @@ func (ms *Module) SetResourceInstanceDeposed(addr addrs.ResourceInstance, key De // If we have no objects at all then we'll clean up. delete(rs.Instances, addr.Key) } - if rs.EachMode == NoEach && len(rs.Instances) == 0 { + if len(rs.Instances) == 0 { // Also clean up if we only expect to have one instance anyway // and there are none. We leave the resource behind if an each mode // is active because an empty list or map of instances is a valid state. @@ -190,7 +187,7 @@ func (ms *Module) ForgetResourceInstanceAll(addr addrs.ResourceInstance) { } delete(rs.Instances, addr.Key) - if rs.EachMode == NoEach && len(rs.Instances) == 0 { + if len(rs.Instances) == 0 { // Also clean up if we only expect to have one instance anyway // and there are none. We leave the resource behind if an each mode // is active because an empty list or map of instances is a valid state. @@ -215,7 +212,7 @@ func (ms *Module) ForgetResourceInstanceDeposed(addr addrs.ResourceInstance, key // If we have no objects at all then we'll clean up. delete(rs.Instances, addr.Key) } - if rs.EachMode == NoEach && len(rs.Instances) == 0 { + if len(rs.Instances) == 0 { // Also clean up if we only expect to have one instance anyway // and there are none. We leave the resource behind if an each mode // is active because an empty list or map of instances is a valid state. diff --git a/states/resource.go b/states/resource.go index fc9f0b49a..0b6a45092 100644 --- a/states/resource.go +++ b/states/resource.go @@ -14,12 +14,6 @@ type Resource struct { // belongs to. Addr addrs.AbsResource - // EachMode is the multi-instance mode currently in use for this resource, - // or NoEach if this is a single-instance resource. This dictates what - // type of value is returned when accessing this resource via expressions - // in the Terraform language. - EachMode EachMode - // Instances contains the potentially-multiple instances associated with // this resource. This map can contain a mixture of different key types, // but only the ones of InstanceKeyType are considered current. @@ -173,31 +167,6 @@ func (i *ResourceInstance) findUnusedDeposedKey() DeposedKey { } } -// EachMode specifies the multi-instance mode for a resource. -type EachMode rune - -const ( - NoEach EachMode = 0 - EachList EachMode = 'L' - EachMap EachMode = 'M' -) - -//go:generate go run golang.org/x/tools/cmd/stringer -type EachMode - -func eachModeForInstanceKey(key addrs.InstanceKey) EachMode { - switch key.(type) { - case addrs.IntKey: - return EachList - case addrs.StringKey: - return EachMap - default: - if key == addrs.NoKey { - return NoEach - } - panic(fmt.Sprintf("don't know an each mode for instance key %#v", key)) - } -} - // DeposedKey is a 8-character hex string used to uniquely identify deposed // instance objects in the state. type DeposedKey string diff --git a/states/state_deepcopy.go b/states/state_deepcopy.go index 817e1c19d..5cd7708e1 100644 --- a/states/state_deepcopy.go +++ b/states/state_deepcopy.go @@ -88,7 +88,6 @@ func (rs *Resource) DeepCopy() *Resource { return &Resource{ Addr: rs.Addr, - EachMode: rs.EachMode, Instances: instances, ProviderConfig: rs.ProviderConfig, // technically mutable, but immutable by convention } diff --git a/states/state_test.go b/states/state_test.go index cd57757cb..9f37ec3a1 100644 --- a/states/state_test.go +++ b/states/state_test.go @@ -83,7 +83,6 @@ func TestState(t *testing.T) { Name: "baz", }.Absolute(addrs.RootModuleInstance), - EachMode: EachList, Instances: map[addrs.InstanceKey]*ResourceInstance{ addrs.IntKey(0): { Current: &ResourceInstanceObjectSrc{ diff --git a/states/statefile/version4.go b/states/statefile/version4.go index 0cb0ae9b0..b98f90f6d 100644 --- a/states/statefile/version4.go +++ b/states/statefile/version4.go @@ -95,27 +95,10 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { } } - var eachMode states.EachMode - switch rsV4.EachMode { - case "": - eachMode = states.NoEach - case "list": - eachMode = states.EachList - case "map": - eachMode = states.EachMap - default: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid resource metadata in state", - fmt.Sprintf("Resource %s has invalid \"each\" value %q in state.", rAddr.Absolute(moduleAddr), eachMode), - )) - continue - } - ms := state.EnsureModule(moduleAddr) // Ensure the resource container object is present in the state. - ms.SetResourceMeta(rAddr, eachMode, providerAddr) + ms.SetResourceProvider(rAddr, providerAddr) for _, isV4 := range rsV4.Instances { keyRaw := isV4.IndexKey @@ -272,7 +255,7 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) { // on the incoming objects. That behavior is useful when we're making // piecemeal updates to the state during an apply, but when we're // reading the state file we want to reflect its contents exactly. - ms.SetResourceMeta(rAddr, eachMode, providerAddr) + ms.SetResourceProvider(rAddr, providerAddr) } // The root module is special in that we persist its attributes and thus @@ -394,29 +377,11 @@ func writeStateV4(file *File, w io.Writer) tfdiags.Diagnostics { continue } - var eachMode string - switch rs.EachMode { - case states.NoEach: - eachMode = "" - case states.EachList: - eachMode = "list" - case states.EachMap: - eachMode = "map" - default: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Failed to serialize resource in state", - fmt.Sprintf("Resource %s has \"each\" mode %s, which cannot be serialized in state", resourceAddr.Absolute(moduleAddr), rs.EachMode), - )) - continue - } - sV4.Resources = append(sV4.Resources, resourceStateV4{ Module: moduleAddr.String(), Mode: mode, Type: resourceAddr.Type, Name: resourceAddr.Name, - EachMode: eachMode, ProviderConfig: rs.ProviderConfig.String(), Instances: []instanceObjectStateV4{}, }) diff --git a/states/sync.go b/states/sync.go index 4b82c69fb..d0923b150 100644 --- a/states/sync.go +++ b/states/sync.go @@ -197,12 +197,12 @@ func (s *SyncState) ResourceInstanceObject(addr addrs.AbsResourceInstance, gen G // SetResourceMeta updates the resource-level metadata for the resource at // the given address, creating the containing module state and resource state // as a side-effect if not already present. -func (s *SyncState) SetResourceMeta(addr addrs.AbsResource, eachMode EachMode, provider addrs.AbsProviderConfig) { +func (s *SyncState) SetResourceProvider(addr addrs.AbsResource, provider addrs.AbsProviderConfig) { s.lock.Lock() defer s.lock.Unlock() ms := s.state.EnsureModule(addr.Module) - ms.SetResourceMeta(addr.Resource, eachMode, provider) + ms.SetResourceProvider(addr.Resource, provider) } // RemoveResource removes the entire state for the given resource, taking with