Support bootstrap env from file

[stevehipwell]

#844

Description of changes:

Support providing environment variables to bootstrap.sh via an optional file (/etc/profile.d/bootstrap-env.sh) that is sourced if it exists. This solves the problem of no variable persistence between cloud-init steps which currently stops MNGs providing bootstrap overrides. To support adding KUBELET_EXTRA_ARGS to MNGs if KUBELET_EXTRA_ARGS is set before the args are processed it will be persisted to KUBELET_EXTRA_ARGS_ENV which will be appended to any value pass in via the args.

From non-MNG nodes this functionality can be used to add dynamic node labels based on instance metadata amongst other things.

This pattern has been in use for while with no issues, but this official change will make it safer as it removes the need to modify bootstrap.sh.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[stevehipwell]

@cartermckinnon could you take a look at this please?

[suket22]

I really like the flexibility this change gives, and I see how this would be super beneficial today for managed nodegroups (and Karpenter) since it helps communicate info across the MIME multi-parts.

My main concern is the maintainability of this going forward. @bwagner5 @cartermckinnon and I have been discussing rewriting the bootstrap script - potentially a Go binary that we install in the AMI. If we were to do that, ensuring that /etc/profile.d/bootstrap-env.sh will continue to be honored will be difficult.

This might limit our ability to modify Managed Nodegroups to use a new bootstrap binary since nodegroups already reliant on this file bootstrap-env.sh and this semantic will break. It's less of a problem in Karpenter which is still early-stage and the API can be versioned.

[suket22]

The alternative I've been thinking of is that the bootstrap script supports accepting two filePaths : one that points to the kubelet configuration, and one to the container runtime configuration. The latter now exists for containerd.

If we were to re-write the bootstrapping in a different language / as a binary, it might be easier to continue to honor these configuration files rather than honoring all bash environment variables as defined in the original bootstrap script.

I'm making an assumption here that overriding kubelet configuration, and container runtime configuration are the only use cases. I don't think most folks are looking to override the API Server endpoint / Cluster certificate etc. Having said that, if someone's looking to do that, they should probably just bootstrap the whole thing themselves.

[stevehipwell]

@suket22 I also spoke to @bwagner5 at KubeCon about a Go binary to replace bootstrap.sh; and I think this should be the way forward especially if it can use the Bottlerocket configuration for consistency.

That said, I think you'll have to offer a switch between the new binary and old shell bootstrapping; so I don't think it matters if more work is done to the current logic as long as there is a way to do the same with the new binary. The two approaches would be fundamentally different, but this change would make it clearer how the current pattern can be used to do anything with the MNG that you can also do with a straight ASG before the binary is available.

[stevehipwell]

@suket22 I missed your second reply. Many people are making other changes such as adding custom CAs to the OS and adding OCI credentials to kubelet. The configuration options in Bottlerocket are a good starting point, but remember that it has init containers for the real edge cases; say dynamic node labels based on the subnet the node starts in.

[suket22]

That said, I think you'll have to offer a switch between the new binary and old shell bootstrapping; so I don't think it matters if more work is done to the current logic as long as there is a way to do the same with the new binary

Agreed, I just don't know how we can offer the switch via the Managed Nodegroups API. I don't think we're going to be adding a new EKS API parameter for this. I think the best CX would be for us to switch over to a newer binary without anyone with an existing nodegroup having to change anything.

Many people are making other changes such as adding custom CAs to the OS and adding OCI credentials to kubelet

Fair point. I'd assume this is very niche though right? I think as a managed service we need to draw the line somewhere. At this level of customization, it probably makes sense to take complete ownership of the bootstrapping (like how custom AMIs + MNG does it).

[suket22]

Playing devil's advocate, there's no way enforce backwards compatibility regardless :(
If someone's using Managed Nodegroups and is editing the /etc/eks/bootstrap.sh file on disk directly in their MIME part, all their edits become useless if Managed Nodegroups starts calling a different binary.
So maybe a switch is something we'd need either way.

[stevehipwell]

Agreed, I just don't know how we can offer the switch via the Managed Nodegroups API. I don't think we're going to be adding a new EKS API parameter for this. I think the best CX would be for us to switch over to a newer binary without anyone with an existing nodegroup having to change anything.

I assume the MNG AWS API gets the officially supported options added to it which feed the default configuration for the binary. Additional configuration should be able to be set via a LT so the binary should support merging multiple configuration files.

Fair point. I'd assume this is very niche though right? I think as a managed service we need to draw the line somewhere. At this level of customization, it probably makes sense to take complete ownership of the bootstrapping (like how custom AMIs + MNG does it).

I don't think OCI credentials are niche, they're pretty fundamental to a production ready K8s cluster. At a minimum and off the top of my head for a MNG I'd expect to be able to set labels, taints, max pods, kube reserved & OCI credentials (OCI credentials are currently static R/O but moving towards dynamic).

[cartermckinnon]

Closing along with #875 .