eksctl-io / eksctl

The official CLI for Amazon EKS
https://eksctl.io
Other
4.92k stars 1.41k forks source link

Support kubeletExtraConfig in managed node groups #4459

Closed janeklb closed 1 year ago

janeklb commented 2 years ago

What feature/behaviour/change do you want?

Support kubeletExtraConfig in managed node groups

Why do you want this feature?

In order to specify extra kubelet configuration features, you need to use overrideBootstrap and use --kubelet-extra-args etc. This is rather cumbersome and so it would be preferable to use something like node groups' kubeletExtraConfig field for managed node groups.

Summary

aclevername commented 2 years ago

Thanks for opening the issue @janeklb. For future folks who didn't realise we already have this for unmanaged nodegroups atm :sweat_smile: https://eksctl.io/usage/schema/#nodeGroups-kubeletExtraConfig. The feature sounds reasonable to me :+1:

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Skarlso commented 2 years ago

We have this in our doc:

- Full control over the node bootstrapping process and customization of the kubelet are not supported. This includes the
following fields: `classicLoadBalancerNames`, `targetGroupARNs`, `clusterDNS` and `kubeletExtraConfig`.

I think there is a reason for that because managed nodegroups don't allow for this to be passed in?

Skarlso commented 2 years ago

I guess we could make something like, if it's defined, we set override, and pass in the necessary config. That could work, and would bring together the way we handle extra configure in eksctl with nodegroups.

janeklb commented 2 years ago

I guess we could make something like, if it's defined, we set override, and pass in the necessary config. That could work, and would bring together the way we handle extra configure in eksctl with nodegroups.

That would make kubelet config management (for managed node groups) a lot easier!

Skarlso commented 2 years ago

The only that will be required however, is that a custom AMI is set. I can't do anything unless that is done, because only then will the Managed Nodegroups not merge all the user data. @janeklb

Skarlso commented 2 years ago

Okay, so after rummaging in the code, the following will be done:

Skarlso commented 2 years ago

So... I can't make this work without https://github.com/awslabs/amazon-eks-ami/issues/873 being a thing in the bootstrap.sh of a managed nodegroup. Without that, anything I would do with the extra config is just overwritten. I tried doing something like this:

#!/bin/sh
set -ex
KUBELET_CONFIG=/etc/kubernetes/kubelet/kubelet-config.json

TMP_KUBE_CONF='/tmp/kubelet-conf-temp.json'
KUBELET_CONFIG_EXTRA='/tmp/kubelet-conf-extra.json'
echo '%s' > ${KUBELET_CONFIG_EXTRA}

echo "eksctl: merging user options into kubelet-config.json"
trap 'rm -f ${TMP_KUBE_CONF} ${KUBELET_CONFIG_EXTRA}' EXIT
jq -s '.[0] + .[1]' "${KUBELET_CONFIG}" "${KUBELET_CONFIG_EXTRA}" > "${TMP_KUBE_CONF}"
mv "${TMP_KUBE_CONF}" "${KUBELET_CONFIG}"
/etc/eks/bootstrap.sh %s

As a userdata script. Which works very well actually. Where %s is the passed in kube config. It does merge the two configs. But... when you call bootstrap at the end, they configure a few things automatically and overwrite some values in the configuration file which makes this thing obsolete.

I could have parsed the config flag and then translate each and every json value to a --flag option and pass that in, but that's simply just not feasible in the long run. eksctl would have to constantly keep this translation list up to date with values and kubelet flags as they change and evolve which leads to more headache then we care to take on.

If AWS deems this a nice feature to have, we can come back to this issue. Until then, sadly, this is blocked.

Skarlso commented 2 years ago

FWIW I submitted the necessary modification to the bootstrap script as a PR here.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 5 days with no activity.

MartinEmrich commented 1 year ago

Yep, we also could use kubeletExtraConfigs for managed nodegroups, to work around https://github.com/aws/containers-roadmap/issues/1847