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

Managed Node Group launch template disk size fix #1305

Merged
merged 11 commits into from
Aug 21, 2024

Conversation

flostadler
Copy link
Contributor

This fixes the handling of disk size changes when a custom launch template is created for the MNG.
It is based on the Community PR #1229 by @JustASquid, I just added some test fixes on top.

Fixes #1228

@flostadler flostadler requested review from a team, t0yv0 and corymhall August 20, 2024 17:23
@flostadler flostadler self-assigned this Aug 20, 2024
Copy link

Does the PR have any schema changes?

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

},
})

integration.ProgramTest(t, &test)
Copy link
Member

Choose a reason for hiding this comment

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

programTestWithExtraOptions will ignore delete failures

@@ -1742,6 +1742,9 @@ function createManagedNodeGroupInternal(
let launchTemplate: aws.ec2.LaunchTemplate | undefined;
if (args.kubeletExtraArgs || args.bootstrapExtraArgs || args.enableIMDSv2) {
launchTemplate = createMNGCustomLaunchTemplate(name, args, core, parent, provider);

// Disk size is specified in the launch template.
delete nodeGroupArgs.diskSize;
Copy link
Member

Choose a reason for hiding this comment

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

Not quite following control flow here, is diskSize deleted here coming from what the user supplied? Should we warn the user that the supplied value is a no-op in this case?

Copy link
Contributor Author

@flostadler flostadler Aug 20, 2024

Choose a reason for hiding this comment

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

It needs to be deleted because it will be set in the launch template. If you supply a custom launch template to the managed node group, the disk size needs to be set within the launch template and not on the node group


// Defined in test managed-ng-disk-size
var desiredSizeGB int64 = 50
assert.True(t, nodeEphemeralStorage.CmpInt64(desiredSizeGB*1_000_000_000) >= 0)
Copy link
Member

Choose a reason for hiding this comment

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

This looks reassuring!

@flostadler
Copy link
Contributor Author

The test failures are unrelated to this, the CI account is currently clogged up. Working on resolving this right now

@flostadler flostadler merged commit 453da6a into master Aug 21, 2024
46 checks passed
@flostadler flostadler deleted the community-contrib/mng-launch-template-disk-size-fix branch August 21, 2024 16:16
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v2.7.9.

flostadler added a commit that referenced this pull request Sep 4, 2024
This fixes the handling of disk size changes when a custom launch
template is created for the MNG.
It is based on the Community PR #1229 by @JustASquid, I just added some
test fixes on top.

Fixes #1228

---------

Co-authored-by: Daniel West <daniel.west.279@gmail.com>
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.

Cannot specify Managed Node Group DiskSize when a launch template is created
4 participants