Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When I change count to for_each, the resource will rebuild ? #22375

Open
lyc0221 opened this issue Aug 7, 2019 · 9 comments

Comments

@lyc0221
Copy link

commented Aug 7, 2019

Hi there,

terraform version: 0.12.6
aws version: v2.22.0

new main.tf

resource "aws_instance" "this" {
  for_each = {
    1: ""
  }

  ami                    = var.ami
  instance_type          = var.instance_type
  subnet_id              = var.subnet_id

  tags = merge(
    var.tags,
    {
      "Name" = format("%s-%s-%s-%s", var.service, var.environment, var.region, each.key)
    },
  )
  ......

}

result:

  # module.ec2.aws_instance.this[1] will be destroyed
  - resource "aws_instance" "this" {
      - ami                          = "ami-**********" -> null
      ..........
    }

  # module.ec2.aws_instance.this["1"] will be created
  + resource "aws_instance" "this" {
      + ami                          = "ami-*********"
      ..........
    }

How can the resources not be rebuilt?

Looking forward to your reply!

Thanks

@lyc0221 lyc0221 changed the title When I create multiple aws instances, from count to for_each, the resources are rebuilt? When I change count to for_each, the resource will rebuild ? Aug 7, 2019
@grimm26

This comment has been minimized.

Copy link

commented Aug 8, 2019

Just curious, what is the point of that change?

@teamterraform

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2019

Hi @lyc0221,

This works exactly as if you were renaming a resource. The method used for refactoring in this case is to move the resource within the state to match the new identifiers from the config.

terraform state mv 'aws_instance.this[1]' 'aws_instance.this["1"]'

Since this may be a more common request as users refactor their configurations to take advantage of new new features in 0.12, we may want to cover the use case in the documentation, and perhaps add some more tooling as well.

Thanks

@lyc0221

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

Just curious, what is the point of that change?

@grimm26
aws_instance.this[1] is count index, and aws_instance.this["1"] is for_each index

If your resources are created with count in version 0.11, upgrade0.12 changes to create with for_each, and the resources will be recreated, but you don't want to recreate, so you have to change the index

@lyc0221

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

@teamterraform

terraform state list

data.aws_ami.ubuntu_xenial
data.aws_subnet_ids.all
data.aws_vpc.default
null_resource.provisioning
module.ec2.data.template_file.userdata
module.ec2.aws_instance.this
module.security_group.aws_security_group.this
module.security_group.aws_security_group_rule.egress_rules
module.security_group.aws_security_group_rule.ingress_rules[0]
module.security_group.aws_security_group_rule.ingress_rules[1]
module.security_group.aws_security_group_rule.ingress_rules[2]

terraform state mv 'module.ec2.aws_instance.this' 'module.ec2.aws_instance.this["1"]'

Move "module.ec2.aws_instance.this" to "module.ec2.aws_instance.this[\"1\"]"

Error: Invalid target address

Cannot move to module.ec2.aws_instance.this["1"]: module.ec2.aws_instance.this
does not exist in the current state.

Why can I not move to module.ec2.aws_instance.this that is exist ?

@grimm26

This comment has been minimized.

Copy link

commented Aug 9, 2019

Just curious, what is the point of that change?

@grimm26
aws_instance.this[1] is count index, and aws_instance.this["1"] is for_each index

If your resources are created with count in version 0.11, upgrade0.12 changes to create with for_each, and the resources will be recreated, but you don't want to recreate, so you have to change the index

Right, what does that gain you? I get huge benefits by using for_each with a map, but over a list like that? Not so much.

@teamterraform teamterraform added bug core and removed enhancement labels Aug 9, 2019
@teamterraform

This comment has been minimized.

Copy link
Collaborator

commented Aug 9, 2019

Thanks for the update @lyc0221,

The issue here is that you're not moving directly from count to for_each, the resource module.ec2.aws_instance.this is still a single instance in the state. The interchangeability of having no count and count = 1 is a remnant of earlier versions of terraform, but they are now handled more consistently internally to make the configuration easier to reason about. I think we may even want to have plan show a change similar to your original for_each example when changing to count = 1 to further enforce this consistency.

You can work around this for now by changing to count = 1 first, and applying even though there is no plan shown. This will change the resource representation in the state to have an index which can be addressed. Once that is saved in the state, the state mv command will be able to move the resource to use string index.

The state command should probably be able to handle moving single resources directly to and from indexed resources to aid in this type of refactoring.

@lyc0221

This comment has been minimized.

Copy link
Author

commented Aug 12, 2019

@teamterraform I've tried by count with v0.12.6(the resource was created with v0.11.14 first) , but it'll change the resource(Equivalent to recreate), so I can't do that with this way. any other ways ?

@bastiandg

This comment has been minimized.

Copy link

commented Aug 12, 2019

We have a very similar issue, while trying to migrate count based states and modules from terraform 0.11 to 0.12 with for_each.

Example resource

resource "random_id" "resource" {
  count       = 1
  byte_length = 8
}

State list after apply with terraform 0.11:

$ terraform state list
random_id.resource

Migrating this state with terraform 0.12 gives an error:

$ terraform state mv random_id.resource "random_id.resource[0]"
Move "random_id.resource" to "random_id.resource[0]" #tf 0.12

Error: Invalid target address

Cannot move to random_id.resource[0]: random_id.resource does not exist in the
current state.

It works though with terraform 0.11:

$ terraform state mv random_id.resource "random_id.resource[0]" #tf 0.11
Moved random_id.resource to random_id.resource[0]

But even after that move from a standalone element to a list moving the state to the for_each construct doesn't work:

$ terraform state mv 'random_id.resource[0]' 'random_id.resource["zero"]'
Move "random_id.resource[0]" to "random_id.resource[\"zero\"]"

Error: Invalid target address

Cannot move to random_id.resource["zero"]: random_id.resource does not exist
in the current state.

The issue only occurs on single element lists.

When migrating a list with multiple elements there is no issue:

resource "random_id" "resource" {
  count       = 2
  byte_length = 8
}
$ terraform state list
random_id.resource[0]
random_id.resource[1]
$ terraform state mv 'random_id.resource[0]' 'random_id.resource["zero"]'
Move "random_id.resource[0]" to "random_id.resource[\"zero\"]"
Successfully moved 1 object(s).

As we have a significant number of modules with a lot of resources using count we want to migrate, applying manually with count = 1 is not really feasible. Having a different of way migrating single resources into list under terraform 0.12, would also be immensely helpful to us.

@bastiandg

This comment has been minimized.

Copy link

commented Aug 15, 2019

We found a dirty workaround to this issue. After an investigation on how a single resource differs from a list of resources, we figured out that it's simply the index_key in the state.

The index_key is located under .resources[$INDEX].instances[0]

Here is a tiny script that makes the fix with jq:

state-mv.sh

module="$1"
resource_type="$2"
index_key="$3"
state_path="$4"
out_path="$5"

jq ".resources[
		.resources | map(.type == \"$resource_type\" and .module == \"$module\") | index(true)
	].instances[0] +=
	{\"index_key\": \"$index_key\"}" \
	"$state_path" > "$out_path"

@lyc0221 In your case the following will replace the apparently existing key with the string representation and print the fixed state to stdout:

jq '.resources[ .resources | map(.type == "aws_instance" and .module == "module.ec2") | index(true) ].instances[0].index_key = "1"' terraform.tfstate

Use with caution, I give no guarantee that it won't break something :) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.