-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow containerd
config imports
#1630
Conversation
containerd
config imports
/ci |
@cartermckinnon roger that! I've dispatched a workflow. 👍 |
We need a user guide section about this, this should be the preferred way of amending the EKS containerd defaults on AL2. |
@ndbaker1 do we create the directory? (is |
updated the user guide 👍 PTAL
we weren't, just added this to |
d54bb8e
to
eac0985
Compare
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.
Think the wording can be tightened up and most of the details left in the code.
@cartermckinnon the workflow that you requested has completed. 🎉
|
thanks @ndbaker1 |
files/bootstrap.sh
Outdated
@@ -555,6 +555,7 @@ if [[ "$CONTAINER_RUNTIME" = "containerd" ]]; then | |||
fi | |||
|
|||
sudo mkdir -p /etc/containerd | |||
sudo mkdir -p /etc/containerd/config.d |
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.
Do we need this if it's created at build time?
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.
bottom line, probably not, but its still a contract set at bootstrap time so i figure its safe to have in both 🤔
/ci let's see what the latest rev looks like... |
@cartermckinnon roger that! I've dispatched a workflow. 👍 |
@cartermckinnon the workflow that you requested has completed. 🎉
|
I changed the base branch to |
3bbcd08
to
68ccf4c
Compare
thanks for the reminder! rebased now @cartermckinnon |
Could we get an eye on this 😃 it would be nice to get a release with this fix so that we can resolve upstream issues. |
/ci |
/ci |
@ndbaker1 the workflow that you requested has completed. 🎉
|
68ccf4c
to
70bd0e1
Compare
Issue #, if available:
#1628
Description of changes:
adding an import path for containerd to read for merging config tomls: https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md#complete-configuration
avoid the need for users to directly edit the containerd config toml.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Testing Done
See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.