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 launch_templates_with_managed_node_group example #1599

Conversation

PascalBourdier
Copy link
Contributor

@PascalBourdier PascalBourdier commented Sep 23, 2021

remove instance_type in aws_launch_template.default to avoid this error:

│ Error: error creating EKS Node Group (test-eks-lt-URZJOIL1:test-eks-lt-URZJOIL1-example2021092307395969650000000f): InvalidRequestException: Cannot specify instance types in launch template and API request
│ {
│   RespMetadata: {
│     StatusCode: 400,
│     RequestID: "bece44af-e7e0-4931-9725-4fb57a7de031"
│   },
│   ClusterName: "test-eks-lt-URZJOIL1",
│   Message_: "Cannot specify instance types in launch template and API request",
│   NodegroupName: "test-eks-lt-URZJOIL1-example2021092307395969650000000f"
│ }
│
│   with module.eks.module.node_groups.aws_eks_node_group.workers["example"],
│   on ../../modules/node_groups/main.tf line 1, in resource "aws_eks_node_group" "workers":
│    1: resource "aws_eks_node_group" "workers" {

replace it by instance_types in node_groups block of module.eks

add a custom_suffix in aws_iam_service_linked_role.autoscaling to avoid conflict like:

│ Error: Error creating service-linked role with name autoscaling.amazonaws.com: InvalidInput: Service role name AWSServiceRoleForAutoScaling has been taken in this account, please try a different suffix.
│     status code: 400, request id: e4ad4c1e-ad6e-4544-bed5-f3beeb148d29
│
│   with aws_iam_service_linked_role.autoscaling,
│   on disk_encryption_policy.tf line 2, in resource "aws_iam_service_linked_role" "autoscaling":
│    2: resource "aws_iam_service_linked_role" "autoscaling" {

PR o'clock

Description

Please explain the changes you made here and link to any relevant issues.

Checklist

Pascal Bourdier and others added 2 commits September 23, 2021 10:45
remove `instance_type` in `aws_launch_template.default` to avoid this error:
```
│ Error: error creating EKS Node Group (test-eks-lt-URZJOIL1:test-eks-lt-URZJOIL1-example2021092307395969650000000f): InvalidRequestException: Cannot specify instance types in launch template and API request
│ {
│   RespMetadata: {
│     StatusCode: 400,
│     RequestID: "bece44af-e7e0-4931-9725-4fb57a7de031"
│   },
│   ClusterName: "test-eks-lt-URZJOIL1",
│   Message_: "Cannot specify instance types in launch template and API request",
│   NodegroupName: "test-eks-lt-URZJOIL1-example2021092307395969650000000f"
│ }
│
│   with module.eks.module.node_groups.aws_eks_node_group.workers["example"],
│   on ../../modules/node_groups/main.tf line 1, in resource "aws_eks_node_group" "workers":
│    1: resource "aws_eks_node_group" "workers" {
```

replace it by `instance_types` in `node_groups` block of `module.eks`

add a `custom_suffix` in `aws_iam_service_linked_role.autoscaling` to avoid conflict like:
```
│ Error: Error creating service-linked role with name autoscaling.amazonaws.com: InvalidInput: Service role name AWSServiceRoleForAutoScaling has been taken in this account, please try a different suffix.
│     status code: 400, request id: e4ad4c1e-ad6e-4544-bed5-f3beeb148d29
│
│   with aws_iam_service_linked_role.autoscaling,
│   on disk_encryption_policy.tf line 2, in resource "aws_iam_service_linked_role" "autoscaling":
│    2: resource "aws_iam_service_linked_role" "autoscaling" {
```
Copy link
Contributor

@daroga0002 daroga0002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PascalBourdier thank you for your contribution 🎉

@antonbabenko I have tested so this can be merged, I dont think here we need make a release as examples will be updated over master. I will try to review few more PRs and then we can consolidate few smaller PRs into release

@antonbabenko antonbabenko changed the title fix: launch_templates_with_managed_node_group example fix: Fixed launch_templates_with_managed_node_group example Sep 24, 2021
@antonbabenko antonbabenko merged commit 13cb555 into terraform-aws-modules:master Sep 24, 2021
@antonbabenko
Copy link
Member

@daroga0002 I agree, we can merge multiple PRs and release once. Let me know when you think you are done reviewing them.

@iyenigul
Copy link

iyenigul commented Oct 4, 2021

@antonbabenko @PascalBourdier
The following resource at https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/examples/launch_templates_with_managed_node_groups/disk_encryption_policy.tf#L2
is always re-created when I apply terraform apply (terraform 1.0)

 resource "aws_iam_service_linked_role" "autoscaling" {
    aws_service_name = "autoscaling.amazonaws.com"
    description      = "Default Service-Linked Role enables access to AWS Services and Resources used or managed by Auto Scaling"
     custom_suffix    = "lt_with_managed_node_groups" # the full name is "AWSServiceRoleForAutoScaling_lt_with_managed_node_groups" < 64 characters
   } 

see the result.


Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # aws_iam_service_linked_role.autoscaling must be replaced
-/+ resource "aws_iam_service_linked_role" "autoscaling" {
      ~ arn              = "arn:aws:iam::id:role/aws-service-role/autoscaling.amazonaws.com/AWSServiceRoleForAutoScaling_lt_with_managed_node_groups" -> (known after apply)
      ~ create_date      = "2021-10-04T19:33:58Z" -> (known after apply)
      + custom_suffix    = "lt_with_managed_node_groups" # forces replacement
      ~ id               = "arn:aws:iam::id:role/aws-service-role/autoscaling.amazonaws.com/AWSServiceRoleForAutoScaling_lt_with_managed_node_groups" -> (known after apply)
      ~ name             = "AWSServiceRoleForAutoScaling_lt_with_managed_node_groups" -> (known after apply)
      ~ path             = "/aws-service-role/autoscaling.amazonaws.com/" -> (known after apply)
      ~ unique_id        = "AROATPZ442RUJ3BUOZH6F" -> (known after apply)
        # (2 unchanged attributes hidden)
    }

Plan: 1 to add, 0 to change, 1 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_iam_service_linked_role.autoscaling: Destroying... [id=arn:aws:iam::id:role/aws-service-role/autoscaling.amazonaws.com/AWSServiceRoleForAutoScaling_lt_with_managed_node_groups]
aws_iam_service_linked_role.autoscaling: Still destroying... [id=arn:aws:iam::id:role/aws-serv...utoScaling_lt_with_managed_node_groups, 10s elapsed]
aws_iam_service_linked_role.autoscaling: Destruction complete after 12s
aws_iam_service_linked_role.autoscaling: Creating...
aws_iam_service_linked_role.autoscaling: Creation complete after 1s [id=arn:aws:iam::id:role/aws-service-role/autoscaling.amazonaws.com/AWSServiceRoleForAutoScaling_lt_with_managed_node_groups]
Releasing state lock. This may take a few moments...

But it works fine if I remove custom_suffix or If I add

 lifecycle {
    ignore_changes = ["custom_suffix"]
  }

lisfo4ka pushed a commit to lisfo4ka/terraform-aws-eks that referenced this pull request Oct 12, 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.

4 participants