-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add support for EKS Auto Mode #1519
Conversation
`${name}-eksRole`, | ||
{ | ||
service: "eks.amazonaws.com", | ||
service: pulumi.interpolate`eks.${dnsSuffix}`, |
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.
Noticed this was not partition independent
Does the PR have any schema changes?Found 60 breaking changes: Resources
Types
|
/** | ||
* For IAM roles associated with EC2 instances that need access policies. Allows the nodes to join the cluster. | ||
*/ | ||
EC2: "EC2", |
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.
This was a huge PITA to discover. This new Access Entry type is not documented and required when using a custom node role with EKS Auto Mode or else the nodes cannot join the cluster.
} | ||
|
||
/** | ||
* The ServiceRole component creates an IAM role for a particular service and attaches to it a list of well-known | ||
* managed policies. | ||
*/ | ||
export class ServiceRole extends pulumi.ComponentResource { | ||
// The service role. | ||
public readonly role: pulumi.Output<aws.iam.Role>; |
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.
Previously this internal component exported a single output for the role, that depended on all policy attachments.
Because of that it becomes unknown when attaching new policies.
In order to upgrade an existing cluster to Auto Mode, you need to attach new policies. The Cluster's service role property triggers replacements, so the preview would've shown that the cluster gets replaced.
This new approach is a bit more careful. We expose am output for the fully resolved role (with all policies attached) and the direct role resource itself.
Now the consumers can pick and choose what they want.
@@ -22,7 +22,16 @@ const popped = pulumi.output(publicSubnetIds).apply(subnets => { | |||
const cluster = new eks.Cluster(`${projectName}`, { | |||
vpcId: vpc.vpcId, | |||
publicSubnetIds: popped, | |||
}); | |||
}, { transforms: [args => { |
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.
This test was not working correctly anymore. Previously changes to the subnets triggered replacements. this is not the case anymore. This test wants to test a cluster replacement so I forced it.
Background why this wasn't failing hard in the past:
EKS picks two of the provided subnets for its control plane ENIs. When updating them, they need to be in the same AZs as the original ones.
We have a 1/3 chance to pick the right ones (the VPC has 3 AZs) and we have 3 retries configured for the test. So we got lucky most of the times.
Not quite. You can enable this, it just doesn't make sense. And in general, the better approach is to move to have this hard codeded as |
Thanks a lot for the feedback @bryantbiggs! Generally I agree with your sentiment, but hardcoding it to false would be a breaking change that we cannot take until the next major version because users expect the addons to be installed ATM. I was thinking about an alternative until we can take the breaking change. We could just not expose that property and ignore any changes to it. When users enable EKS Auto Mode we set it to false. This would only have an effect during cluster creation. |
oh completely agree - this is what we did in the EKS module since the default value for the boostrap managed addons is with a TODO reminder to set this to a hardcoded value of |
While looking into this, I noticed that this test was never working. IPv6 cannot work with the current setup because the node role is missing
This seems to be an instance of pulumi/pulumi-aws#4759 which was believed to be a bug, but this behavior might actually make a regression. I'll look into it! Edit: opened #1523 to remove the wrong and untested IPv6 example |
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.
Looks good! Just some questions around the API and default values.
@@ -447,6 +454,13 @@ export function createCore( | |||
); | |||
} | |||
|
|||
if (args.autoMode?.enabled && !supportsAccessEntries(args.authenticationMode)) { | |||
throw new pulumi.ResourceError( |
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.
TIL!
nodejs/eks/cluster.ts
Outdated
// EKS Auto Mode needs "sts:TagSession" in addition to the default "sts:AssumeRole" | ||
assumeRoleActions: args.autoMode?.enabled | ||
? ["sts:AssumeRole", "sts:TagSession"] | ||
: undefined, |
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.
The comment makes it look like undefined
should be ["sts:AssumeRole"]
instead?
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.
just add both regardless of auto mode - more policies are moving to use session tags
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.
["sts:AssumeRole"]
is the default of assumeRoleActions
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.
Good to know @bryantbiggs, thanks!
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.
Changed it to add sts:TagSession
independent of EKS Auto Mode now
This PR has been shipped in release v3.5.0. |
> Today at re:Invent, AWS announced Amazon Elastic Kubernetes Service (Amazon EKS) Auto Mode, a new feature that fully automates compute, storage, and networking management for Kubernetes clusters. [AWS Announcement](https://aws.amazon.com/about-aws/whats-new/2024/12/amazon-eks-auto-mode/) [AWS Blog post](https://aws.amazon.com/blogs/aws/streamline-kubernetes-cluster-management-with-new-amazon-eks-auto-mode/) This adds a blog post about the new EKS Auto Mode feature AWS released and how users can leverage it with Pulumi. This is the PR for adding support to pulumi-eks: pulumi/pulumi-eks#1519
Proposed changes
This change adds support for EKS Auto Mode (see AWS docs). It's a new feature that fully automates compute, storage, and networking management for Kubernetes clusters.
To support EKS Auto Mode,
pulumi-aws
was upgraded to6.65.0
and the required options are exposed via the Cluster component's input properties.Right now, existing clusters cannot be upgraded to use Auto Mode when using the built-in node pools. This is due to an upstream bug pulumi/pulumi-aws#4885. Users can still use EKS Auto Mode by opting out of the built-in node pools and node role. They'll have to create a node role with the appropriate access entries and create the node classes and node pools themselves in this case.
An example of this can be found in the upgrade test.
Related issues (optional)
Closes #1509
Relates to pulumi/pulumi-aws#4885