Implement --containerd-extra-config-toml in bootstrap.sh #790
Conversation
This depends on awslabs/amazon-eks-ami#790 being merged and released.
|
yes please I need it as well |
|
Thanks for taking care of this. |
| if [[ -n "$CONTAINERD_EXTRA_CONFIG_TOML" ]]; then | ||
| echo "$CONTAINERD_EXTRA_CONFIG_TOML" >> /etc/eks/containerd/containerd-config.toml | ||
| fi |
There was a problem hiding this comment.
This block should be moved above L470, so that we have the ability to specify SANDBOX image in custom containerd config and get is replaced by the right pause container in the bootstrap.sh script.
There was a problem hiding this comment.
Makes sense, I'm going to change this and force-push.
codablock
force-pushed
the
extra-containerd-config
branch
from
79e95c6 to
d93ea1b
Compare
There was a problem hiding this comment.
Hey, my apologies it's taken us this long to get to this PR. Would you be open to us collaborating with you on this? I can help push changes to your fork if you're okay with it or take this over on a separate PR.
@cartermckinnon could you take a look at this PR too?
| @@ -461,7 +468,10 @@ fi | |||
| if [[ "$CONTAINER_RUNTIME" = "containerd" ]]; then | |||
| sudo mkdir -p /etc/containerd | |||
| sudo mkdir -p /etc/cni/net.d | |||
| sudo sed -i s,SANDBOX_IMAGE,$PAUSE_CONTAINER,g /etc/eks/containerd/containerd-config.toml | |||
| if [[ -n "$CONTAINERD_EXTRA_CONFIG_TOML" ]]; then | |||
| echo "$CONTAINERD_EXTRA_CONFIG_TOML" >> /etc/eks/containerd/containerd-config.toml | |||
There was a problem hiding this comment.
Should we make this echo "$CONTAINERD_EXTRA_CONFIG_TOML" > /etc/eks/containerd/containerd-config.toml to support the use case also described in #830 where an append may not be sufficient?
| @@ -75,6 +76,11 @@ while [[ $# -gt 0 ]]; do | |||
| shift | |||
| shift | |||
| ;; | |||
| --containerd-extra-config-toml) | |||
There was a problem hiding this comment.
Should we change this to be a pointer to a file rather than the entire toml content itself?
I know with --docker-config-json we've already gone the latter route, but using a file pointer instead has a nice advantage when using MIME multiparts for the userData. In an earlier MIME part we can write the entire config to disk at a specific location, and the subsequent part that calls this bootstrap script (like Managed Nodegroups / Karpenter) can hardcode that file name.
There was a problem hiding this comment.
I would prefer it as you described it, but decided to stick with the way --docker-config-json is implemented to avoid unexpected surprises. If enough EKS folks tell me that the file-pointer way is the preferred one, I'll go with it.
This flag allows to pass extra configuration for containerd, which is appended to /etc/containerd/config.toml. This allows to specify things like mirros/pull-through-caches without interfering with the EKS provided config.
codablock
force-pushed
the
extra-containerd-config
branch
from
d93ea1b to
de77bcc
Compare
|
@suket22 Thanks for looking into the PR. I would prefer if you take over with this PR, in whatever fashion (new PR or pushing to my branch) you prefer. Right now, I'd have to work on it without any ability to test out the changes, which might end up being much less effective than when you directly take over. |
|
Sounds good, thanks @codablock. I'll get on it and should have a PR soon. |
|
@suket22 / @codablock - is it possible to make whatever solution comes out of this PR compatible with managed node groups? Right now, with the way managed node groups inject the bootstrap script at the base of the user data file, users do not have access to all of the bootstrap script flags which leads to hacky workarounds. I know it might be beyond the scope of what you were envisioning, but I think if there is a path to cater to both self-managed and EKS managed node groups, that would be amazing. Something to consider, thank you! |
|
Alright we've got this merged as part of #929 and this should be make it as part of our next AMI release. |
This flag allows to pass extra configuration for containerd, which is
appended to /etc/containerd/config.toml. This allows to specify things
like mirros/pull-through-caches without interfering with the EKS provided
config.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.