-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add support for custom Bottlerocket AMIs for MNG #8418
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
Conversation
Signed-off-by: Kush Upadhyay <[email protected]>
if userData == "" { | ||
return "", errors.New("generated unexpected empty TOML user-data from input") | ||
// Generate TOML for launch in this NodeGroup. | ||
data, err := bottlerocketSettingsTOML(b.clusterConfig, b.ng.NodeGroupBase, settings) |
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.
bottlerocketSettingsTOML
, similar to ProtectTOMLKeys
a few lines above, comes from bottlerocket.go and is used to set the kubernetes cluster settings, which is necessary for the nodes to join the cluster.
Without this function, I was seeing the following error in the kubelet logs:
err="failed to construct kubelet dependencies: unable to loadclient CA file /etc/kubernetes/pki/ca.crt: error creating pool from /etc/kubernetes/pki/ca.crt: data does not contain any valid RSA or ECDSA certificates"
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 passing verifies that these settings cannot be manually set thru userdata, only injected.
Expect(tree.HasPath([]string{"settings", "kubernetes", "node-labels"})).To(BeFalse()) | ||
Expect(tree.HasPath([]string{"settings", "kubernetes", "node-taints"})).To(BeFalse()) |
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.
These settings actually exist, but should not be set thru userdata: https://bottlerocket.dev/en/os/1.39.x/api/settings/kubernetes/#node-labels
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.
lgtm!
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.
Thanks for the contribution! 🚀
Issue
Close: #8419
Description
Currently, when creating MNGs using a custom Bottlerocket AMI with
eksctl
, we get the error:This PR adds support for custom Bottlerocket AMIs for MNG.
Also, it improves tests in
managed_bottlerocket_test.go
so they verify specific settings rather than entire strings 😁Testing
make test
eksctl
./eksctl create cluster -f <MANIFEST>
successfully creates cluster and MNG with custom AMIChecklist
README.md
, or theuserdocs
directory)area/nodegroup
) and kind (e.g.kind/improvement
)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯