feat: Add support for spot instances node groups.

[psoares]

PR o'clock

Description

Please explain the changes you made here and link to any relevant issues.
This is to implement feature request #1107.
It allows creating nodegroups with spot instances, as described in https://aws.amazon.com/about-aws/whats-new/2020/12/amazon-eks-support-ec2-spot-instances-managed-node-groups/

example usage:

module "eks" {
  // [...]
  node_groups = {
    spot-inst-managed = {
      name             = "spot-managed-${var.cluster_name}"
      desired_capacity = 2
      max_capacity     = 20
      min_capacity     = 2
      instance_type    = ["m5.large", "m5a.large", "m5d.large", "m5ad.large"]
      capacity_type    = "SPOT"
    }

Checklist

• CI tests are passing
• README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

[zmingxie]

Could you also update the example here with the new capacity_type option?
https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/examples/launch_templates_with_managed_node_groups/main.tf

This will be easier for future references

[brannondorsey]

Managed spot instance node pools add support for mixed instance types. Would it be possible to make the instance_type accept a list of instance types in addition to a string?

On second thought, that's exactly what it looks like this PR does ✅

[psoares]

@brannondorsey I'm updating the documentation to make it obvious.
@zmingxie I added an example. Let me know if it looks ok.

[brannondorsey]

Thanks for that docs change 😸

We're currently using this module and have had to make our managed spot instance node groups out of band without Terraform. I appreciate this small PR which will make our lives much easier. 🙏

[brannondorsey]

Any idea when this might land in a new release? We're looking to use this functionality this week and are contemplating if we should just vendor this branch into our code base or wait for a new release of the module with these changes included.

[psoares]

What's the process to get a PR reviewed here?

[worldofgeese]

What's the process to get a PR reviewed here?

Looks like @barryib has write access. Pinging him for you.

[psoares]

What's the process to get a PR reviewed here?

Looks like @barryib has write access. Pinging him for you.
Merci @barryib!

[worldofgeese]

Maybe if I try @dpiddockcmp also

[duboisf]

Hi @max-rocket-internet! Would it be possible to get a review for this PR pretty please?

[anon892]

When will this PR get merged?

[psoares]

When will this PR get merged?

This is my first PR... I have no experience on how long it usually takes to get a review.

[anon892]

@psoares, have you please tested your fix that it works?

[duboisf]

Tested it 👍

[psoares]

Tested it 👍

Thank you 😉
Any idea why the merge is still blocked?

[duboisf]

Yes we need one of the maintainers to approve the PR

[antimack]

also interested in this, any estimations?

[darkhelmet]

How will this work with a launch template and wanting to pick multiple instance types? Do I create a launch template per instance type and then a node group per launch template?

Basically, I have a launch template, and I want a spot instance node group of "any of these instance types" (which it looks like your example supports, if you don't specify a launch template).

Maybe that's a separate thing? I can leave the instance type off the launch template, but then the node group module ignores the instance type I specify on the group itself. Should that be the case?

Edit: Looking at the AWS help when creating a new node group I see When using a launch template do not specify instanceTypes, diskSize or remoteAccess in the node group configuration. These properties must be configured within the launch template itself. which suggests it's doing things properly.

[psoares]

How will this work with a launch template and wanting to pick multiple instance types? Do I create a launch template per instance type and then a node group per launch template?

Basically, I have a launch template, and I want a spot instance node group of "any of these instance types" (which it looks like your example supports, if you don't specify a launch template).

Maybe that's a separate thing? I can leave the instance type off the launch template, but then the node group module ignores the instance type I specify on the group itself. Should that be the case?

Edit: Looking at the AWS help when creating a new node group I see When using a launch template do not specify instanceTypes, diskSize or remoteAccess in the node group configuration. These properties must be configured within the launch template itself. which suggests it's doing things properly.

I've been scratching my head on this one for a few days. I even opened #1192. I'll let you know if I find a way. So far I've been able to do a launch template, but not override it's parameters with a launch configuration to handle multiple instance types...

[barryib]

I just merged #1129 and it adds the SPOT support for managed node groups.

@psoares can we close this PR as it duplicates #1129 ?

[psoares]

Closing since the feature was implemented and merged through #1129.

[psoares]

@barryib, should we reopen this PR to fix #1211 ?

[barryib]

@barryib, should we reopen this PR to fix #1211 ?

Nope. Thanks and sorry for the delay.

[github-actions]

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.