-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(eks): managed nodegroup support #6759
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
- minor fix
copy @eladb |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Co-Authored-By: Elad Ben-Israel <[email protected]>
Co-Authored-By: Elad Ben-Israel <[email protected]>
Co-Authored-By: Elad Ben-Israel <[email protected]>
Co-Authored-By: Elad Ben-Israel <[email protected]>
Co-Authored-By: Elad Ben-Israel <[email protected]>
Co-Authored-By: Elad Ben-Israel <[email protected]>
Co-Authored-By: Elad Ben-Israel <[email protected]>
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should add a BREAKING CHANGE notice in the commit message.
OK. In the commit message now. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@@ -312,7 +320,7 @@ export class Cluster extends Resource implements ICluster { | |||
* The auto scaling group that hosts the default capacity for this cluster. | |||
* This will be `undefined` if the default capacity is set to 0. | |||
*/ | |||
public readonly defaultCapacity?: autoscaling.AutoScalingGroup; | |||
public readonly defaultCapacity?: Nodegroup | autoscaling.AutoScalingGroup; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly this won’t work with jsii languages like Java so we don’t use unions.
Let’s just use defaultNodegroup for the nodegroup version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Let me fix it.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Now we have both |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
I am crying ! Have learned a lot from it. Thanks!! |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Hi @eladb I just noticed this blog post from EKS team: Upcoming Changes to IP Assignment for EKS Managed Node Groups
I am afraid users would be confused as we deploy the
However, this behavior, according to the blog post, will be changed starting April 20, 2020. And CDK doesn't have to do anything for it. By that time, all clusters created by CDK will by default have a managed nodegroup in private subnet and no public IP addresses will be automatically assigned. Do we need to note this somewhere? |
Hi @eladb I am trying to add a managed nodegroup to my cluster but I cannot find the eks.NodeGroup anywhere on the API documentation. Can you help me with how to create a managed node group with CDK in Python? https://github.com/aws/aws-cdk/tree/master/packages/%40aws-cdk/aws-eks |
Also, do I need to create an AutoScalingGroup to make sure I have a manged node group? |
@pahud can you help me figure out how to enable managed node groups in eks? |
@FarshadNiayesh I think we can't use it in Python until CDK 1.32.0 released(should be in the next few days). According to the document, we will have a new # create a cluster with default managed nodegroup
cluster = aws_eks.Cluster()
# add an extra manaded nodegroup
cluster.add_nodegroup() |
Thank you for your answer. @pahud I had another question. When I try to install helm charts in specific namespaces, I always get an error from the Cloudformation console when the stack is in "Create in progress" mode. Seems like the helm chart cannot create the specified namespace. Do you know how I can pre-create namespaces with eks.cluster() and then pass them to the eks.chart()? |
If you create Helm Charts with see the python reference here Given this thread is for managed nodegroup, let's discuss Helm Chart on gitter(awslabs/aws-cdk) or create another issue if necessary. |
Description
Nodegroup
resource fromAWS::EKS::Nodegroup
cluster.addNodegroup(id, {...})
which creates a default nodegroup for the clusterCommit Message
feat(aws-eks): managed nodegroup support (#6759 )
BREAKING CHANGE:
Cluster
now creates a default managed nodegroup as its default capacity. Set the new cluster propertydefaultCapacityType
toDefaultCapacityType.EC2
to preserveEC2
as its default capacity.Closes #5086
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license