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

Fix ManagedNodeGroup ignoring custom AMI if no user data is configured #1562

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

flostadler
Copy link
Contributor

@flostadler flostadler commented Dec 24, 2024

When users specified a custom AMI for an EKS Managed Node Group, the setting was only taking effect if their configuration already triggered custom user data generation, otherwise it used the default AMI. To fix this, I modified the logic to generate custom user data whenever a custom AMI is configured. This ensures the node group launches successfully with proper bootstrap settings regardless of other configuration options (AWS EKS does not provide default user data if an AMI ID is specified in the launch template).

The previous E2E tests worked because they coincidentally used the same AMI ID as the default AMI. The updated test intentionally chooses an AMI for the previous minor version and verifies the EC2 instances are using this exact AMI.

Fixes #1550

When users configured a custom AMI for an MNG it only took effect if
their settings also led to custom user data being generated.
This change fixes that by also generating custom user data if users
set a custom AMI. This is necessary because EKS will not provide
default user data if an AMI ID is specified in the launch template.
@flostadler flostadler requested a review from a team December 24, 2024 12:08
@flostadler flostadler self-assigned this Dec 24, 2024
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

export const customAmiId = pulumi.all([cluster.eksCluster.version]).apply(([version]) => {
const versionParts = version.split('.');
const majorVersion = parseInt(versionParts[0], 10);
const minorVersion = parseInt(versionParts[1], 10) - 1;
Copy link
Contributor Author

@flostadler flostadler Dec 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would break if k8s rolled out a new major version, but there's no indications this will happen in the next years and would make this computation unnecessarily complex.
If they ever do release one we can fix up the test (it'll break and not silently succeed).

@flostadler flostadler requested a review from corymhall December 24, 2024 13:58
@flostadler flostadler merged commit e9b446a into master Jan 2, 2025
34 checks passed
@flostadler flostadler deleted the flostadler/fix-mng-with-custom-ami branch January 2, 2025 09:26
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v3.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Managed node group ignores provided AMI id
3 participants