fpco / terraform-aws-foundation

Establish a solid Foundation on AWS with these modules for Terraform
MIT License
203 stars 99 forks source link

Support multiple EBS volumes attached and persisted to single node ASG. #319

Open Magicloud opened 4 years ago

Magicloud commented 4 years ago

This change contains multiple changes to modules, to support having multiple persisted EBS attached to single node ASG.

The problem is that, since this is a breaking change, many variables/outputs are different. I think we should make them new modules to avoid breaking current code.

Magicloud commented 4 years ago

@ketzacoatl The change works, but not the final code (filling up description and so) since I'd like to hear your idea. Is it better to make another few modules instead of making breaking changes to original modules?

ketzacoatl commented 4 years ago

@Magicloud do we also want to create a copy of the previous single-node-asg module to allow users access to the previous workflow?

qrilka commented 4 years ago

@Magicloud what's the status of this? Do you plan to add README as requested above and add a module to use with the previous workflow?

Magicloud commented 4 years ago

Please add a README and Makefile with the example, and include in the README notes on how to use the example and run the tests.

Done.

qrilka commented 4 years ago

I tried to create an ASG with no extra volumes (passing []) got an error during apply:


Error: Invalid index

  on .terraform/modules/asg-a/modules/init-snippet-attach-ebs-volume/main.tf line 19, in data "template_file" "init_snippet":
  19:     device_path   = var.device_paths[count.index]
    |----------------
    | count.index is 0
    | var.device_paths is empty list of string

The given key does not identify an element in this collection value.
Magicloud commented 4 years ago

I tried to create an ASG with no extra volumes (passing []) got an error during apply:


Error: Invalid index

  on .terraform/modules/asg-a/modules/init-snippet-attach-ebs-volume/main.tf line 19, in data "template_file" "init_snippet":
  19:     device_path   = var.device_paths[count.index]
    |----------------
    | count.index is 0
    | var.device_paths is empty list of string

The given key does not identify an element in this collection value.

@qrilka Does this happen on existing resources? I tried from beginning, no problem.

qrilka commented 4 years ago

@Magicloud yes, probably that was the reason - I switched from master version to this branch

qrilka commented 4 years ago

So probably it could be a problem when upgrading modules (and when this will get merged)

Magicloud commented 4 years ago

@qrilka I see. Since the logic is changed totally, it could break.

qrilka commented 4 years ago

@Magicloud while using this branch for A/B switching demo I have found that tags cause unnecessary updates like the following:

  # module.asg-magenta.module.service-data.aws_ebs_volume.main[0] will be updated in-place
  ~ resource "aws_ebs_volume" "main" {
        arn                  = "arn:aws:ec2:us-west-2:xxxxx:volume/vol-xxxxx"
        availability_zone    = "us-west-2a"
        encrypted            = true
        id                   = "vol-xxxx"
        iops                 = 100
        kms_key_id           = "arn:aws:kms:us-west-2:xxxx:key/xxxx-ae63-4278-bba4-xxxx"
        multi_attach_enabled = false
        size                 = 20
      ~ tags                 = {
            "Name"             = "switching-demo-magenta-us-west-2a-a"
          - "cldy-instance-id" = "i-0b4f3320df090f7ca" -> null
        }
        type                 = "gp2"
    }
Magicloud commented 4 years ago

@qrilka Where does that tag come from?

qrilka commented 4 years ago

That's a good question but I have not idea and not sure if I will have time to find it out...

ketzacoatl commented 4 years ago

That tag is unrelated and we should not have blocked this PR on that. Unless there's reason to hold back, I think we should merge this PR.