Merge pull request #24807 from hashicorp/jbardin/remove-each-mode

Remove EachMode from state
This commit is contained in:
James Bardin 2020-05-03 10:54:07 -04:00 committed by GitHub
commit 9debd341bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 43 additions and 215 deletions

View File

@ -259,6 +259,7 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module
current := resource{
Address: r.Addr.Instance(k).String(),
Index: k,
Type: resAddr.Type,
Name: resAddr.Name,
ProviderName: r.ProviderConfig.Provider.String(),
@ -276,10 +277,6 @@ func marshalResources(resources map[string]*states.Resource, module addrs.Module
)
}
if r.EachMode != states.NoEach {
current.Index = k
}
schema, _ := schemas.ResourceTypeConfig(
r.ProviderConfig.Provider,
resAddr.Mode,

View File

@ -193,7 +193,6 @@ func TestMarshalResources(t *testing.T) {
Name: "bar",
},
},
EachMode: states.NoEach,
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
addrs.NoKey: {
Current: &states.ResourceInstanceObjectSrc{
@ -237,7 +236,6 @@ func TestMarshalResources(t *testing.T) {
Name: "bar",
},
},
EachMode: states.EachList,
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
addrs.IntKey(0): {
Current: &states.ResourceInstanceObjectSrc{
@ -281,7 +279,6 @@ func TestMarshalResources(t *testing.T) {
Name: "bar",
},
},
EachMode: states.EachMap,
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
addrs.StringKey("rockhopper"): {
Current: &states.ResourceInstanceObjectSrc{
@ -325,7 +322,6 @@ func TestMarshalResources(t *testing.T) {
Name: "bar",
},
},
EachMode: states.NoEach,
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
addrs.NoKey: {
Deposed: map[states.DeposedKey]*states.ResourceInstanceObjectSrc{
@ -371,7 +367,6 @@ func TestMarshalResources(t *testing.T) {
Name: "bar",
},
},
EachMode: states.NoEach,
Instances: map[addrs.InstanceKey]*states.ResourceInstance{
addrs.NoKey: {
Deposed: map[states.DeposedKey]*states.ResourceInstanceObjectSrc{

View File

@ -279,10 +279,6 @@ func (c *StateMvCommand) Run(args []string) int {
ssFrom.ForgetResourceInstanceAll(addrFrom)
ssFrom.RemoveResourceIfEmpty(fromResourceAddr)
// since this is moving an instance, we can infer the target
// mode from the address.
toEachMode := eachModeForInstanceKey(addrTo.Resource.Key)
rs := stateTo.Resource(addrTo.ContainingResource())
if rs == nil {
// If we're moving to an address without an index then that
@ -291,15 +287,13 @@ func (c *StateMvCommand) Run(args []string) int {
// address covers both). If there's an index in the
// target then allow creating the new instance here.
resourceAddr := addrTo.ContainingResource()
stateTo.SyncWrapper().SetResourceMeta(
stateTo.SyncWrapper().SetResourceProvider(
resourceAddr,
toEachMode,
fromProviderAddr, // in this case, we bring the provider along as if we were moving the whole resource
)
rs = stateTo.Resource(resourceAddr)
}
rs.EachMode = toEachMode
rs.Instances[addrTo.Resource.Key] = is
}
default:
@ -372,20 +366,6 @@ func (c *StateMvCommand) Run(args []string) int {
return 0
}
func eachModeForInstanceKey(key addrs.InstanceKey) states.EachMode {
switch key.(type) {
case addrs.IntKey:
return states.EachList
case addrs.StringKey:
return states.EachMap
default:
if key == addrs.NoKey {
return states.NoEach
}
panic(fmt.Sprintf("don't know an each mode for instance key %#v", key))
}
}
// sourceObjectAddrs takes a single source object address and expands it to
// potentially multiple objects that need to be handled within it.
//
@ -415,11 +395,13 @@ func (c *StateMvCommand) sourceObjectAddrs(state *states.State, matched addrs.Ta
// terraform state mv aws_instance.foo aws_instance.bar[1]
// That wouldn't be allowed if aws_instance.foo had multiple instances
// since we can't move multiple instances into one.
if rs := state.Resource(addr); rs != nil && rs.EachMode == states.NoEach {
if rs := state.Resource(addr); rs != nil {
if _, ok := rs.Instances[addrs.NoKey]; ok {
ret = append(ret, addr.Instance(addrs.NoKey))
} else {
ret = append(ret, addr)
}
}
default:
ret = append(ret, matched)
}

View File

@ -97,9 +97,10 @@ func TestStateMv(t *testing.T) {
if diags.HasErrors() {
t.Fatal(diags.Err())
}
i := s.Resource(addr)
if i.EachMode != states.EachList {
t.Fatalf("expected each mode List, got %s", i.EachMode)
for key := range s.Resource(addr).Instances {
if _, ok := key.(addrs.IntKey); !ok {
t.Fatalf("expected each mode List, got key %q", key)
}
}
// change from list to map
@ -118,9 +119,10 @@ func TestStateMv(t *testing.T) {
if diags.HasErrors() {
t.Fatal(diags.Err())
}
i = s.Resource(addr)
if i.EachMode != states.EachMap {
t.Fatalf("expected each mode Map, got %s", i.EachMode)
for key := range s.Resource(addr).Instances {
if _, ok := key.(addrs.StringKey); !ok {
t.Fatalf("expected each mode map, found key %q", key)
}
}
// change from from map back to single
@ -139,9 +141,10 @@ func TestStateMv(t *testing.T) {
if diags.HasErrors() {
t.Fatal(diags.Err())
}
i = s.Resource(addr)
if i.EachMode != states.NoEach {
t.Fatalf("expected each mode NoEach, got %s", i.EachMode)
for key := range s.Resource(addr).Instances {
if key != addrs.NoKey {
t.Fatalf("expected no each mode, found key %q", key)
}
}
}
@ -179,13 +182,12 @@ func TestStateMv_resourceToInstance(t *testing.T) {
Module: addrs.RootModule,
},
)
s.SetResourceMeta(
s.SetResourceProvider(
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "test_instance",
Name: "bar",
}.Absolute(addrs.RootModuleInstance),
states.EachList,
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Module: addrs.RootModule,

View File

@ -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) + ")"
}
}

View File

@ -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.

View File

@ -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

View File

@ -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
}

View File

@ -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{

View File

@ -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{},
})

View File

@ -154,6 +154,6 @@ func TestFullInitialState() *states.State {
Provider: addrs.NewLegacyProvider(rAddr.ImpliedProvider()),
Module: addrs.RootModule,
}
childMod.SetResourceMeta(rAddr, states.EachList, providerAddr)
childMod.SetResourceProvider(rAddr, providerAddr)
return state
}

View File

@ -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

View File

@ -2637,7 +2637,7 @@ func TestContext2Apply_orphanResource(t *testing.T) {
Type: "test_thing",
Name: "one",
}.Absolute(addrs.RootModuleInstance)
s.SetResourceMeta(oneAddr, states.EachList, providerAddr)
s.SetResourceProvider(oneAddr, providerAddr)
s.SetResourceInstanceCurrent(oneAddr.Instance(addrs.IntKey(0)), &states.ResourceInstanceObjectSrc{
Status: states.ObjectReady,
AttrsJSON: []byte(`{}`),
@ -6111,9 +6111,7 @@ func TestContext2Apply_destroyWithModuleVariableAndCount(t *testing.T) {
//Test that things were destroyed
actual := strings.TrimSpace(state.String())
expected := strings.TrimSpace(`
<no state>
module.child:
<no state>`)
<no state>`)
if actual != expected {
t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
}
@ -6269,9 +6267,7 @@ func TestContext2Apply_destroyWithModuleVariableAndCountNested(t *testing.T) {
//Test that things were destroyed
actual := strings.TrimSpace(state.String())
expected := strings.TrimSpace(`
<no state>
module.child.child2:
<no state>`)
<no state>`)
if actual != expected {
t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
}

View File

@ -475,7 +475,7 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
return nil, diags.Err()
}
state.SetResourceMeta(n.Addr, states.EachList, n.ProviderAddr)
state.SetResourceProvider(n.Addr, n.ProviderAddr)
expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
case n.Config.ForEach != nil:
@ -487,11 +487,11 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
// This method takes care of all of the business logic of updating this
// while ensuring that any existing instances are preserved, etc.
state.SetResourceMeta(n.Addr, states.EachMap, n.ProviderAddr)
state.SetResourceProvider(n.Addr, n.ProviderAddr)
expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEach)
default:
state.SetResourceMeta(n.Addr, states.NoEach, n.ProviderAddr)
state.SetResourceProvider(n.Addr, n.ProviderAddr)
expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
}

View File

@ -4,7 +4,6 @@ import (
"fmt"
"os"
"path/filepath"
"strconv"
"sync"
"github.com/agext/levenshtein"
@ -803,43 +802,6 @@ func (d *evaluationStateData) getResourceSchema(addr addrs.Resource, providerAdd
return schema
}
// coerceInstanceKey attempts to convert the given key to the type expected
// for the given EachMode.
//
// If the key is already of the correct type or if it cannot be converted then
// it is returned verbatim. If conversion is required and possible, the
// converted value is returned. Callers should not try to determine if
// conversion was possible, should instead just check if the result is of
// the expected type.
func (d *evaluationStateData) coerceInstanceKey(key addrs.InstanceKey, mode states.EachMode) addrs.InstanceKey {
if key == addrs.NoKey {
// An absent key can't be converted
return key
}
switch mode {
case states.NoEach:
// No conversions possible at all
return key
case states.EachMap:
if intKey, isInt := key.(addrs.IntKey); isInt {
return addrs.StringKey(strconv.Itoa(int(intKey)))
}
return key
case states.EachList:
if strKey, isStr := key.(addrs.StringKey); isStr {
i, err := strconv.Atoi(string(strKey))
if err != nil {
return key
}
return addrs.IntKey(i)
}
return key
default:
return key
}
}
func (d *evaluationStateData) GetTerraformAttr(addr addrs.TerraformAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
switch addr.Name {