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

(aws-eks): managed nodegroup for spot instances #11827

Closed
1 of 2 tasks
pahud opened this issue Dec 2, 2020 · 7 comments · Fixed by #11962
Closed
1 of 2 tasks

(aws-eks): managed nodegroup for spot instances #11827

pahud opened this issue Dec 2, 2020 · 7 comments · Fixed by #11962
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@pahud
Copy link
Contributor

pahud commented Dec 2, 2020

Amazon EKS now supports provisioning and managing EC2 Spot Instances in managed node groups
https://aws.amazon.com/tw/blogs/containers/amazon-eks-now-supports-provisioning-and-managing-ec2-spot-instances-in-managed-node-groups/

eksctl just introduced the new --spot flag which seems to create a new launch template with spot options and pass this template to create the spot-only nodegroup.

I was wondering how to create similar experience with aws-eks. The Nodegroup L2 construct actually can accept the launch template as the construct property, however, users need to bake their own LT with spot options before they can pass it to NodegroupProps. It doesn't make sense to add a new spot property for NodegroupProps as this is actually a high level abstraction, but making people easily create spot nodegroup with CDK is really helpful.

Some options:

  1. create aws-eks-patterns L3 and make SpotNodegroup a L3 construct
  2. let's create a 3rd party construct lib like cdk-eks-spot-nodegroup

I'd vote option 1.

@iliapolo wdyt?

ref: https://docs.aws.amazon.com/eks/latest/userguide/managed-node-groups.html

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@pahud pahud added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 2, 2020
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Dec 2, 2020
@iliapolo iliapolo added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 2, 2020
@iliapolo
Copy link
Contributor

iliapolo commented Dec 2, 2020

@pahud I actually think this should be part of the core library, its definitely an important capability and we should provide an experience as easy as --spot.

I think we can start with enabling this via the current launchTemplateSpec option but provide an easy way to create a compatible launch template.

const spots = new eks.SpotLaunchTemplateSpec(...);
new eks.Nodegroup(this, 'SpotNodeGroup', {
  ...,
  launchTemplateSpec: spots,
})

When we introduce proper ec2.LaunchTemplate, this will become:

const spots = new eks.SpotLaunchTemplate(...); // will now extend `ec2.LaunchTemplate`.
new eks.Nodegroup(this, 'SpotNodeGroup', {
  ...,
  launchTemplate: spots,
})

Thoughts?

@pahud
Copy link
Contributor Author

pahud commented Dec 2, 2020

@iliapolo Yes. That will be easier. Please leave this PR for me. I'll do it in the next few days.

@iliapolo
Copy link
Contributor

iliapolo commented Dec 3, 2020

@pahud Let's have another API discussion here before you actually start coding? still feels a little awkward to me even though the approach is solid.

@abierbaum
Copy link

@iliapolo @pahud FWIW, it would be great if the high level api for the cluster just allowed something similar to:

cluster.addNodegroupCapacity('spot-node-group', {
  instanceTypes: [new ec2.InstanceType('m5.large'), new ec2.InstanceType('m5.xlarge')],
  minSize: 4,
  spot: true,
});

Interacting with launch templates seems like an advanced concept that isn't needed for the simple case of just adding more capacity.

@pahud
Copy link
Contributor Author

pahud commented Dec 8, 2020

@pahud Let's have another API discussion here before you actually start coding? still feels a little awkward to me even though the approach is solid.

Hi @iliapolo I have yet to start coding. Please let me know what's in your mind.

RE @abierbaum I believe there would be some scenarios for the EKS with Spot instances integration:

  1. managed NG with CapacityType='spot'

CapacityTye is a new option in API/SDK, see Managed node group capacity types, however, it's not yet available in cloudformation(AWS::EKS::Nodegroup) at this moment. When we specify CapacityType='spot', behind the scene an AutoScaling Group with launch template of single instance type across all AZs will be provisoned.

  1. managed NG with `CapacityType='on-demand'

Same as above. But only on-demand single instance type.

  1. managed NG with multiple instance types and purchase options

According to Considerations for selecting a capacity type:

When deploying your node group with the Spot capacity type that's using a custom launch template, use the API to pass multiple instance types instead of passing a single instance type through the launch template. For more information about deploying a node group using a launch template, see Launch template support.

To achieve this, we'll need to bake the launch template for the nodegroup, which would be a little bit challenging. Good to hide it behind the construct.

Given this, I think we probably can design the Nodegroup API like this

cluster.addNodeGroup(scope, id, {
   capacityType:  CapacityType.SPOT,
   instanceTypes: InstanceType[],
})

or simply

cluster.addNodeGroup(scope, id, {
   spot: true,
   instanceTypes: InstanceType[],
})

And we create the nodegroup and bake the LT with custom resource. WDYT?

@pahud
Copy link
Contributor Author

pahud commented Dec 8, 2020

this is weird capacityType is mentioned in cfn nodegroup document but actually not available

If you specify Spot for capacityType, then we recommend specifying multiple values for instanceTypes. For more information, see Managed node group capacity types and Launch template support in the Amazon EKS User Guide.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html

If cfn can support this, that would be easier for implementation in cdk.
aws-cloudformation/cloudformation-coverage-roadmap#712

@mergify mergify bot closed this as completed in #11962 Dec 22, 2020
mergify bot pushed a commit that referenced this issue Dec 22, 2020
This PR adds the `CapacityType` support and allows users to create Spot managed node groups for Amazon EKS.

1. The `CapacityType` attribute is supported by cloudformation but not yet documented. We tentatively use addPropertyOverride() to enable it.
2. `instanceType` will be deprecated and we introduced the new `instanceTypes`
3. `instanceTypes` with different CPU architectures will throw an error.
4. `amiType` is still optional, however, when specified, incorrect `amiType` will throw the error.
5. According to the [document](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-instancetypes), we are allowed to specify instance type(s) in either `instanceTypes` property or launch template but not both. As we can't check the content of the launch template passed in, we allow `instanceTypes` and launch template both specified and encourage to use `instanceTypes` when possible.


## Sample

```ts
cluster.addNodegroupCapacity('extra-ng-spot', {
  instanceTypes: [
    new ec2.InstanceType('c5.large'),
    new ec2.InstanceType('c5a.large'),
    new ec2.InstanceType('c5d.large'),
  ],
  minSize: 3,
  capacityType: eks.CapacityType.SPOT,
});
```

Closes #11827 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

flochaz pushed a commit to flochaz/aws-cdk that referenced this issue Jan 5, 2021
This PR adds the `CapacityType` support and allows users to create Spot managed node groups for Amazon EKS.

1. The `CapacityType` attribute is supported by cloudformation but not yet documented. We tentatively use addPropertyOverride() to enable it.
2. `instanceType` will be deprecated and we introduced the new `instanceTypes`
3. `instanceTypes` with different CPU architectures will throw an error.
4. `amiType` is still optional, however, when specified, incorrect `amiType` will throw the error.
5. According to the [document](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-eks-nodegroup.html#cfn-eks-nodegroup-instancetypes), we are allowed to specify instance type(s) in either `instanceTypes` property or launch template but not both. As we can't check the content of the launch template passed in, we allow `instanceTypes` and launch template both specified and encourage to use `instanceTypes` when possible.


## Sample

```ts
cluster.addNodegroupCapacity('extra-ng-spot', {
  instanceTypes: [
    new ec2.InstanceType('c5.large'),
    new ec2.InstanceType('c5a.large'),
    new ec2.InstanceType('c5d.large'),
  ],
  minSize: 3,
  capacityType: eks.CapacityType.SPOT,
});
```

Closes aws#11827 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants