Resource for_each

[pselle]

Allow instances to be created according to a map or a set of strings. Closes #17179

locals {
  little_map = {
    a = 1
    b = 2
  }
}
resource "random_pet" "server" {
  for_each = little_map
}

locals {
  lil_strings = list("x", "y")
}
resource "random_pet" "server" {
  for_each = toset(lil_strings)
}

Things that won't work:

• null
• lists (that are a list and not a set)
• random types (string, number, w/e)
• sets that aren't of type string

[mildwonkey]

This is great!

count doesn't work with null, though I personally expected it to work, and I suspect we'll consider adding that in the future (in my mind, null count means: leave out the count expression altogether, but others might expect it to be equivalent to 0 - the ambiguity may explain why we don't allow null, and instead have the user write a conditional).

I think lists and sets (other than strings) should be supported before this gets released; I'm not as sure about how to implement sets (we'd either need to convert them to lists so we have an index, or have users use the tolist() function) but we should at least think about it. Lists and sets should have unified types (so no set of ["1", 2, true])

I'm also curious about the errors that aren't being surfaced to the user - I wouldn't mind spending some time looking into that!

[apparentlymart]

Looking good so far! 🎺

[Skaronator]

How are the resources managed in the state file? Is it still a array like with count or are the created resources actual mapped to their keys in the state file?

[pselle]

@Skaronator Here's a statefile showing instances created with for_each. https://gist.github.com/pselle/140c5f21c96b66f417796ed6ca000c1d, they are identified with the index_key inside the instances array of the given resource. This is the configuration that generated this:

locals {
  little_map = {
    a = 1
    b = 2
  }
}
resource "random_pet" "with_for_each" {
  for_each = local.little_map
}

resource "random_pet" "with_count" {
  count = 2
}

resource "random_pet" "no_iterator" {}

[Skaronator]

@pselle Ah thats great! So b wouldn't get recreated when I remove a from the map. (Since its now on Index 0 instead of 1)

[apparentlymart]

Looks good to me! 🎉

[apparentlymart]

The sentence fragment here made me trip over reading this sentence a bit. 😖

Most of our "modern" error messages try to start with a concise statement of the broad problem as the summary and then elaborate more in the detail. Since this is a situation where it's the combination of two things that is invalid rather than one thing, the usual "Invalid " summary won't work here, but maybe something like this?

Error: Invalid combination of "count" and "for_each"

  <source snippet>

The "count" and "for_each" meta-arguments are mutually-exclusive, because each one chooses
a different identifier type for the resource instances.

(This term "meta-arguments" has always felt a little weird to me, but that's what it's called throughout the documentation and so hopefully using that terminology in the error message will help users find further documentation about these features if they are unsure how to proceed.)

[apparentlymart]

Sorry I didn't catch this on the previous review, but it only just occurred to me while I was staring some similar code...

I think the References implementation of NodeAbstractResource must also be extended to include references from the for_each expression, similarly to how it includes references in the count expression:

terraform/terraform/node_resource_abstract.go

Lines 188 to 189 in 1a89165

	refs, _ := lang.ReferencesInExpr(c.Count)
	result = append(result, refs...)

Without this, Terraform wouldn't notice that a particular object is referenced in the for_each expression and thus create the dependency edge needed to ensure that we wait until all of the data is available before evaluating it.

I'm sorry if I missed something somewhere else in this diff that's catching that.

This sort of thing unfortunately tends to be quite awkward to test, because without it what we have is a race condition and thus not a deterministic test failure. We have two existing strategies for testing this:

• In graph_builder_plan_test.go we test the plan graph builder with some contrived inputs and check that the resulting graph has the expected shape. This is a good way to get a deterministic test, but these tests tend to be pretty sensitive to changes elsewhere because they run the whole plan graph builder (rather than just the reference-analysis code) and thus any other changes to the graph building logic tends to change the outcomes.
• In context_plan_test.go (and other context tests) we have a few cases where we use a mocked provider for one of the resources that intentionally has a short delay in the PlanResourceChange implementation, thus making it very likely that any race condition would be detected. This is nice because it can more directly test for the behavior we're looking for, but it's not so nice that it forces an artificial slowdown in the test and still isn't truly deterministic (if, e.g., something else on the system interferes and creates delays).

[pselle]

@apparentlymart Very good note -- I'll do some digging to check on this/make sure it works/fix it if it doesn't.

[apparentlymart]

Looking at this again with fresh eyes, I remember there is a third testing strategy out there:

• There's at least one test (which I encountered in 0.12 test update work) where it uses one of the sync package's primitives to force one resource to hang until after another one is processed. That's similar to the timer-based trick I mentioned before but without the risk of other days causing the test result to wobble. That approach does have the significant drawback that one of its failure modes is to deadlock the test run altogether, which is what had happened when I encountered that test. So... I dunno if I would add more tests like that, but I suppose it's an option if nothing else seems practical.

[i870495]

Hi,

Are we really able to use for_each inside resource. I am using terraform 12.4 but cannot use for_each to create multiple azure network interfaces.

Error :
rror: Reserved argument name in resource block

on main.tf line 81, in resource "azurerm_network_interface" "vm_instance_nic":
81: for_each = local.testmap

The name "for_each" is reserved for use in a future version of Terraform.

Thanks,
Balu Rajendran

[Skaronator]

@i870495 This feature hasn't been merged yet nor has it been released.

[i870495]

@Skaronator - Thank you !!!

[pselle]

@apparentlymart Re: references

The following is a simple example demonstrating referencing another resource in the expression passed to for_each -- I was able to see this working without issue

resource "random_pet" "with_for_each" {
  for_each =  {
    cool_pet = random_pet.no_iterator.id
  }
  separator = each.value
}

resource "random_pet" "no_iterator" {}

I've added a reference in the testdata used by Context2Plan; I'd be interested in more info on how to introspect there, reading some of the other tests, it's not very clear how to introspect (as when I started using checkVals, I started getting some values that seemed to be bleeding in from another test's schema definition).

I could of course add to the abstract node ref but without proof that it's necessary, I'm not inclined to do so.

[apparentlymart]

If the dependency is not present, Terraform will think it can work on both of those resource blocks concurrently, so it would be undefined which order they would resolve in. With resources that resolve essentially instantly, it's quite possible for them to complete in the right order just by accident, unfortunately.

One way to verify this without writing any further code would be to terraform plan that test configuration with TF_LOG=trace set and watch the logs about graph construction... the last graph construction in the log (building the plan graph) should include something like this:

2019/07/15 10:15:33 [TRACE] Completed graph transform *terraform.TransitiveReductionTransformer with new graph:
meta.count-boundary (EachMode fixup) - *terraform.NodeCountBoundary
  random_pet.with_for_each - *terraform.NodePlannableResource
provider.random - *terraform.NodeApplyableProvider
provider.random (close) - *terraform.graphNodeCloseProvider
  random_pet.with_for_each - *terraform.NodePlannableResource
random_pet.no_iterator - *terraform.NodePlannableResource
  provider.random - *terraform.NodeApplyableProvider
random_pet.with_for_each - *terraform.NodePlannableResource
  random_pet.no_iterator - *terraform.NodePlannableResource
root - terraform.graphNodeRoot
  meta.count-boundary (EachMode fixup) - *terraform.NodeCountBoundary
  provider.random (close) - *terraform.graphNodeCloseProvider
------
2019/07/15 10:15:33 [DEBUG] Starting graph walk: walkPlan

The important part of this is the dependencies for random_pet.with_for_each:

random_pet.with_for_each - *terraform.NodePlannableResource
  random_pet.no_iterator - *terraform.NodePlannableResource

This shows that (in the quick example configuration I wrote using count on my local tree) random_pet.with_for_each depends on random_pet.no_iterator, which is what I'd expect. I'd expect to see the same sort of thing with for_each in your example, if Terraform is correctly finding the dependency here.

(You will likely see multiple TransitiveReductionTransformer references like this in the logs. The one just before Starting graph walk: walkPlan is the main one to look at here, because planning is the phase where we'll expand out the for_each expression and thus the one where this dependency is needed.)

[pselle]

For PR visibility: added the change (and a test) to make sure edges created by references are caught.

[pselle]

@tmccombs Great changeset! You figured out what I hadn't as yet -- the bits of count that hadn't yet been paralleled in for_each (the Known situations). It made sense to me so I cherry-picked it here, if you'd like this to go in (also, sign the CLA please 💯 ). I'll also need to update the docs over here (after you confirm you're good with bringing your commit into this PR): https://www.terraform.io/docs/configuration/data-sources.html to reference this functionality, or can cherry-pick a commit for that if you'd like to do it.

[tmccombs]

Yes, please include my changes in this PR, I've signed the CLA. And thanks for catching the panic and extra file I left in there. If you want to update the data-source documentation, that would be great.

[defo89]

Apologize if it was clarified before but is it planned to add for_each expression support for modules (#17519 (comment)) in this PR as well? Thank you!

[pselle]

@defo89 It was not asked before, so your question is welcome! No, that is not planned (to add for_each for modules in this PR). As soon as I add docs for data sources, this will be merged and will go out in the next release of Terraform.

[brikis98]

Very excited to see this merged!

Two questions:

1. With count, you could not reference the output of a resource. Does for_each have the same limitation?
2. Is there any reason to use count now? Given all the issues it has with removing items from the middle of a list, would it always be safer to use for_each? Or are there cases where count is still preferred?

[Skaronator]

@brikis98

1. You can actually reference the output of a resource that uses a count. The resource is stored as an array and you can return the whole array of a attribute by doing this:

output "nat_ips" {
  value = aws_instance.nat.*.private_ip
}

And then you can select with [0] or element(aws_instance.nat.*.private_ip, 0) the first element.

2. for_each seems to be the preferred way since it doesn't need to recreate everything when you remove a element in the middle but there are probably some cases where count is totally fine. For example if you use numbers directly eg. (count = 2)

[tmccombs]

With count, you could not reference the output of a resource. Does for_each have the same limitation?

I'm not sure what you mean. with count you can access the computed attributes of a resource like resource_type.name[idx].attribute. The same method works for for_each, but with a string key instead of a number key.

Is there any reason to use count now?

The one reason I can think of is where you actually want to create n copies of a resource, with almost identical config.

[brikis98]

Ah, whoops, my first question was a bit ambiguous. What I meant is that the count parameter itself cannot contain a reference to the output of some other resource. E.g.:

resource "random_integer" "num_instances" {
  min = 1
  max = 3
}

resource "aws_instance" "example_3" {
  # This is not allowed
  count         = random_integer.num_instances.result
  ami           = "ami-0c55b159cbfafe1f0"
  instance_type = "t2.micro"
}

The example above will fail as I'm trying to reference a computed output in a count parameter, which is not allowed. Will that fail with the new for_each as well?

[Skaronator]

When you apply the random_integer resource fist then it would go through.
terraform apply -target=random_integer.num_instances

Not the best workaround but it works.

[mejedi]

This is exactly the feature I was looking for! What a coincidence that it got merged just now :)

Unfortunately it fails if self is used within a resource spawned with for_each:

variable "do_token" {}
variable "private_key" {}
variable "ssh_fingerprint" {}
provider "digitalocean" { token = var.do_token }

variable "compute_cluster" {
  default = [1,2,3,4,5] # makes it possible to destroy specific instances when scaling down
}

resource "digitalocean_droplet" "compute" {
  for_each = zipmap(var.compute_cluster, var.compute_cluster)

  image = "ubuntu-18-04-x64"
  name = "compute-${each.key}"
  region = "ams3"
  size = "s-1vcpu-1gb"
  ssh_keys = [ var.ssh_fingerprint ]

  connection {
    host = self.ipv4_address
    user = "root"
    type = "ssh"
    private_key = file(var.private_key)
  }

  provisioner "remote-exec" {
    inline = []
  }
}

on app.tf line 133, in resource "digitalocean_droplet" "compute":
 133:     host = self.ipv4_address

Because digitalocean_droplet.compute has "for_each" set, its attributes must
be accessed on specific instances.

For example, to correlate with indices of a referring resource, use:
    digitalocean_droplet.compute[each.key]

Implementing the suggestion, I get a different error:

Error: Reference to "each" in context without for_each

  on app.tf line 132, in resource "digitalocean_droplet" "compute":
 132:     host = digitalocean_droplet.compute[each.key].ipv4_address

The "each" object can be used only in "resource" blocks, and only when the
"for_each" argument is set.

Declaring the map inline, i.e.

resource "digitalocean_droplet" "compute" {
  for_each = { 1 = 1 }

results in a slightly different error:

Error: Missing resource instance key

  on app.tf line 132, in resource "digitalocean_droplet" "compute":
 132:     host = self.ipv4_address

Because digitalocean_droplet.compute has "for_each" set, its attributes must
be accessed on specific instances.

For example, to correlate with indices of a referring resource, use:
    digitalocean_droplet.compute[each.key]

Any suggestions highly appreciated.

[tmccombs]

Does self work with a resource with count?

…

On Sun, Jul 28, 2019, 15:53 Nick Zavaritsky ***@***.***> wrote: This is exactly the feature I was looking for! What a coincidence that it got merged just now :) Unfortunately it fails if self is used within a resource spawned with for_each: variable "do_token" {} variable "private_key" {} variable "ssh_fingerprint" {} provider "digitalocean" { token = var.do_token } variable "compute_cluster" { default = [1,2,3,4,5] # makes it possible to destroy specific instances when scaling down } resource "digitalocean_droplet" "compute" { for_each = zipmap(var.compute_cluster, var.compute_cluster) image = "ubuntu-18-04-x64" name = "compute-${each.key}" region = "ams3" size = "s-1vcpu-1gb" ssh_keys = [ var.ssh_fingerprint ] connection { host = self.ipv4_address user = "root" type = "ssh" private_key = file(var.private_key) } provisioner "remote-exec" { inline = [] } } on app.tf line 133, in resource "digitalocean_droplet" "compute": 133: host = self.ipv4_address Because digitalocean_droplet.compute has "for_each" set, its attributes must be accessed on specific instances. For example, to correlate with indices of a referring resource, use: digitalocean_droplet.compute[each.key] Implementing the suggestion, I get a different error: Error: Reference to "each" in context without for_each on app.tf line 132, in resource "digitalocean_droplet" "compute": 132: host = digitalocean_droplet.compute[each.key].ipv4_address The "each" object can be used only in "resource" blocks, and only when the "for_each" argument is set. Declaring the map inline, i.e. resource "digitalocean_droplet" "compute" { for_each = { 1 = 1 } results in a slightly different error: Error: Missing resource instance key on app.tf line 132, in resource "digitalocean_droplet" "compute": 132: host = self.ipv4_address Because digitalocean_droplet.compute has "for_each" set, its attributes must be accessed on specific instances. For example, to correlate with indices of a referring resource, use: digitalocean_droplet.compute[each.key] Any suggestions highly appreciated. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21922?email_source=notifications&email_token=AATMRHSXRRP4BP5SOSFUDKTQBYIMNA5CNFSM4H373Z7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD27HJHA#issuecomment-515798172>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATMRHUFHWVCOI773RCNSU3QBYIMNANCNFSM4H373Z7A> .

[mejedi]

@tmccombs

Does self work with a resource with count?

Yes it does! AFAIK self is the only option if it is necessary to use (current) resource properties in resource definition. If you attempt resource[count.index] instead, a circular dependency is reported, since terraform is unable to figure out that only the current resource is being referenced, not the whole set.

[tmccombs]

Sounds like a bug to me. The logic for self is probably just missing the foreach support.

…

On Sun, Jul 28, 2019, 17:26 Nick Zavaritsky ***@***.***> wrote: @tmccombs <https://github.com/tmccombs> Does self work with a resource with count? Yes it does! AFAIK ‘self’ is the only option if it is necessary to use (current) resource properties in resource definition. If you attempt ‘resource[count.index]’ instead, a circular dependency is reported, since terraform is unable to figure out that only the current resource is being referenced, not the whole set. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21922?email_source=notifications&email_token=AATMRHV3HDRMXVY6JCLDKGDQBYTJVA5CNFSM4H373Z7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD27IWMY#issuecomment-515803955>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATMRHWUUJ67LYFLQROIZSTQBYTJVANCNFSM4H373Z7A> .

[tmccombs]

I think this should fix it: #22230

[mejedi]

@tmccombs Thank you for awesome work, self now works like a charm!

Unfortunately, I am hitting further issues. I will give a brief summary below. Later I will provide a minimal terraform file to reproduce.

variable "compute_cluster" {
  type = list
  default = [
    "compute-1",
    "compute-2",
    "compute-3",
    "compute-4",
    "compute-5"
  ]
  description = "The list of compute nodes to deploy"
}
resource "digitalocean_droplet" "compute" {
  for_each = zipmap(var.compute_cluster, var.compute_cluster)
  ...
}
resource "null_resource" "update_compute_cluster_users" {
  triggers = {
    compute_cluster_ips = join(" ", values(digitalocean_droplet.compute).*.ipv4_address)
  }
  ...
}

I tried deploying new compute nodes, and it works. However if I remove one, say compute-2, a sane-looking plan is produced but later terraform says the plan is inconsistent and fails. Relevant excerpt from the plan:

  # null_resource.update_compute_cluster_users (deposed object 4a995044) will be destroyed
  - resource "null_resource" "update_compute_cluster_users" {
      - id       = "1618624293842542071" -> null
      - triggers = {
          - "compute_cluster_ips" = "10.133.250.194 10.133.250.42 10.133.250.36 10.133.251.208 10.133.250.119"
        } -> null
    }

  # null_resource.update_compute_cluster_users will be created
  + resource "null_resource" "update_compute_cluster_users" {
      + id       = (known after apply)
      + triggers = {
          + "compute_cluster_ips" = "10.133.250.194 10.133.250.36 10.133.251.208 10.133.250.119"
        }
    }

Error message:

Error: Provider produced inconsistent final plan

When expanding the plan for null_resource.update_compute_cluster_users to
include new values learned so far during apply, provider "null" produced an
invalid new value for .triggers["compute_cluster_ips"]: was
cty.StringVal("10.133.250.194 10.133.250.36 10.133.251.208 10.133.250.119"),
but now cty.StringVal("10.133.250.194 10.133.250.42 10.133.250.36
10.133.251.208 10.133.250.119").

This is a bug in the provider, which should be reported in the provider's own
issue tracker.

Used to work with count.

[mejedi]

A minimal terraform file to reproduce the issue with inconsistent plan:

variable "list" {
  default = [1, 2, 3, 4, 5]
}
resource "local_file" "file" {
  for_each = zipmap(var.list, var.list)
  filename = "/tmp/${each.key}"
}
resource "null_resource" "null" {
  lifecycle {
    create_before_destroy = true
  }
  triggers = {
    files = join(" ", values(local_file.file)[*].filename)
  }
}

To reproduce: terraform apply, remove an element from the list, terraform apply again.

If I remove lifecycle section, there's no error.

[teamterraform]

Hi @mejedi, thanks for the feedback! Can you please open a new issue with this reproduction case? It's easy to miss comments on closed issues and merged PRs. Thank you.

[brikis98]

Question: what is the equivalent of count.index if using for_each on a resource?

[ilyasotkov]

@brikis98 I was wondering the same, found the answer here: #21533 (comment)

[brikis98]

@ilyasotkov Got it, thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.