terraform-aws-eks-node-group icon indicating copy to clipboard operation
terraform-aws-eks-node-group copied to clipboard

Support attribute-based instance type selection

Open nitrocode opened this issue 3 years ago • 10 comments

what

  • Expose instance_requirements
  • This is a WIP

why

  • Support attribute-based instance type selection

references

  • Potentially closes (or will eventually close) https://github.com/cloudposse/terraform-aws-eks-node-group/issues/95
  • block description https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template#instance_requirements
  • full block definition https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template#instance-requirements
  • t3 info https://aws.amazon.com/ec2/instance-types/t3/
  • Perhaps we want to match https://github.com/terraform-aws-modules/terraform-aws-autoscaling/blob/e14d875cc2ee8a5c2882ed41df0d8135cbcf530b/main.tf#L159 ?

test

module "eks_node_group" {
  # Use source to point to this branch
  # NOTE: this branch is experimental and will be removed upon merge
  source = "git::https://github.com/cloudposse/terraform-aws-eks-node-group?ref=instance_requirements"

  # should select t3.medium (untested)
  instance_requirements = {
    memory_mib = {
      min = 4
      max = 4
    }
    memory_gib_per_vcpu = {
      min = 0.5
      max = 0.5
    }
    vcpu_count = {
      min = 2
      max = 2
    }
    cpu_manufacturers    = ["amazon-web-services"]
    instance_generations = ["current"]
  }
  # ...
}

nitrocode avatar Jun 07 '22 14:06 nitrocode

/test all

nitrocode avatar Jun 07 '22 15:06 nitrocode

I think the provider version needs to be bumped to>= 4.14.0 because that's when instance_requirements was added to launch templates.

Also, maybe instance_requirements should be part of the create_before_destroy pet name?

jtdoepke avatar Jun 07 '22 20:06 jtdoepke

/test all

nitrocode avatar Jun 08 '22 20:06 nitrocode

Also, maybe instance_requirements should be part of the create_before_destroy pet name?

I don't think the instance_requirements should be part of that because then this will auto create the node group which would destroy the existing one. The instance_requirements will only affect the launch template and then we can approach rotating the node group safely outside of terraform.

nitrocode avatar Jun 08 '22 20:06 nitrocode

/test test/bats

nitrocode avatar Jun 08 '22 21:06 nitrocode

/test test/bats

nitrocode avatar Jun 08 '22 21:06 nitrocode

/test test/bats

nitrocode avatar Jun 08 '22 21:06 nitrocode

/test all

nitrocode avatar Jun 08 '22 21:06 nitrocode

This pull request is now in conflict. Could you fix it @nitrocode? 🙏

mergify[bot] avatar Aug 02 '22 18:08 mergify[bot]

@jtdoepke see the above test in the PR description. Please test this out if you have time. :)

nitrocode avatar Aug 08 '22 20:08 nitrocode

This PR won't work, EKS managed node groups are not compatible with instance_requirements. What will happen here is that AWS/EKS will spawn t3.medium nodes only

awoimbee avatar Jun 19 '23 16:06 awoimbee