eksctl icon indicating copy to clipboard operation
eksctl copied to clipboard

Support kubeletExtraConfig in managed node groups

Open janeklb opened this issue 3 years ago • 9 comments

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

  • We want to improve the UX by unifying the behaviour for both managed and unmanaged nodegroup as suggested here "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."
  • MNGs have override settings that allow us to pass through, so we need to parse the config and set these overrides
  • We also need to do some work around the bootstrapping script so we allow these overrides passed to the kubelet
  • First Spike this to understand how much work involved so we can make a decision to go for this feature
  • Timebox : 1-2 days

janeklb avatar Nov 15 '21 11:11 janeklb

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:

aclevername avatar Nov 16 '21 17:11 aclevername

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] avatar Dec 17 '21 01:12 github-actions[bot]

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 avatar Dec 17 '21 08:12 Skarlso

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.

Skarlso avatar Dec 17 '21 08:12 Skarlso

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!

janeklb avatar Dec 23 '21 12:12 janeklb

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 avatar Feb 28 '22 09:02 Skarlso

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

  • if kubeletExtraConfig is provided together with ami, we automatically attach the following ovveride script:
    overrideBootstrapCommand: |
      #!/bin/bash
      /etc/eks/bootstrap.sh my-cluster-name \
        --kubelet-extra-args <parsedargshere> \
  • if kubeletExtraConfig is provided together with ami AND overrideBootstrapCommand kubeletExtraConfig will be ignored. If you are already providing overrideBootstrapCommand might as well add kubelet-extra-args. :)
  • if kubeletExtraConfig is provided without ami we will error. That's invalid as per my previous comment

Skarlso avatar Feb 28 '22 11:02 Skarlso

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 avatar Mar 01 '22 17:03 Skarlso

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

Skarlso avatar Mar 02 '22 08:03 Skarlso

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] avatar Oct 29 '22 02:10 github-actions[bot]

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

github-actions[bot] avatar Nov 03 '22 02:11 github-actions[bot]

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

MartinEmrich avatar Nov 04 '22 07:11 MartinEmrich