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

fix: Fixed variable reference for snapshot_id #1634

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

mitom
Copy link
Contributor

@mitom mitom commented Oct 12, 2021

PR o'clock

Description

#1431 introduced a new option to set snapshot_id, however the variable it points at in https://github.com/terraform-aws-modules/terraform-aws-eks/pull/1431/files#diff-abc45b07ed4bab6d21313623302e9685703e3b4ad4a981fd57fe91c4c70095edR348 is a copy-paste from https://github.com/terraform-aws-modules/terraform-aws-eks/pull/1431/files#diff-20ed3fefbb3f6838784a6f0bbd11bbef818670a86cab676afefe1bed893f4d3dR528

Changed it to point at the right variable.

This is currently breaking our pipeline with the following error:

Error: Reference to undeclared resource on .terraform/modules/eks/workers.tf line 348, in resource "aws_launch_configuration" "workers":
  348:         block_device_mappings.value, 
A managed resource "block_device_mappings" "value" has not been declared in module.eks.

Please treat it as a hotfix.

Checklist

@daroga0002
Copy link
Contributor

seems this impacting aws_launch_configuration we need some additional check and testing before merging this.

@daroga0002
Copy link
Contributor

@mitom could you provide me EKS module config to replicate this problem (it will make testing faster)?

@mitom
Copy link
Contributor Author

mitom commented Oct 13, 2021

@daroga0002 Hey, yeah sure. You'll have to fill out the subnets and such to actually deploy it, but even without those being valid you can reproduce the error on just a plan:

module "eks" {
  source           = "terraform-aws-modules/eks/aws"
  version          = "17.21.0"
  cluster_name     = "test"
  cluster_version = "1.21"
  subnets          = ["subnet-12345"]
  write_kubeconfig = false
  vpc_id           = "vpc-123456"

  workers_group_defaults = {
    additional_ebs_volumes = [
      {
        block_device_name = "/dev/xvdb"
        volume_size = "30"
        volume_type = "gp3"
        encrypted = true
      }
    ]
  }
  worker_groups = [
      {
        instance_type = "m5.medium"
      }
    ]
}

To test the change works, you can add snapshot_id to the volume, it also shows up in the plan without deployment.

@daroga0002
Copy link
Contributor

@mitom thank you for your contribution 🎉

@antonbabenko lets merge this

@antonbabenko antonbabenko changed the title fix: wrong variable reference for snapshot_id fix: Fixed variable reference for snapshot_id Oct 18, 2021
@antonbabenko antonbabenko merged commit bc0988c into terraform-aws-modules:master Oct 18, 2021
@antonbabenko
Copy link
Member

@daroga0002 Please let me know when you have more PRs ready for merge and we can try to release a little bit bigger changes in a release.

spr-mweber3 pushed a commit to spring-media/terraform-aws-eks that referenced this pull request Dec 1, 2021
@github-actions
Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants