-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(docker): /container-init.d for advanced initialization #6577
Conversation
README updated with example per request of @olizilla |
7354767
to
2b3c534
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.
I really like how flexible this approach is. ❤️ 👍
Agree on the main selling point: hook dir removes the need for things like #8326 and allows people to implement custom initialization without burdening the upstream project with additional maintenance surface.
(The mentioned postgresql/docker-entrypoint.sh is very similar in that it executes only during initialization)
Remaining work
- Rebase on top of latest master (done)
- Add regression test
- as this is something people will leverage in their infra, there should be a test in
test/sharness/t0300-docker-image.sh
(or a separate file, if thats easier) that confirms that a script mounted into/container-init.d
is executed. It could executeipfs bootstrap add
and then we could check ifBootstrap
section in the config cot updated.- @Caian will you have time to push this over the finish line, or can this be picked up by someone else?
- as this is something people will leverage in their infra, there should be a test in
Next step is to add tests. |
Adding sharness tests for this will take a bit of time, as the current Docker tests are broken and need to be rewritten. |
2022-04-01:
|
@thattommyhall : it would be great if you can give review of this as as a user of the functionality. |
@thattommyhall : did you meean to close this? |
License: MIT Signed-off-by: Caian Benedicto <caianbene@gmail.com>
License: MIT Signed-off-by: Caian Benedicto <caianbene@gmail.com>
License: MIT Signed-off-by: Caian Benedicto <caianbene@gmail.com>
License: MIT Signed-off-by: Caian Benedicto <caianbene@gmail.com>
License: MIT Signed-off-by: Caian Benedicto <caianbene@gmail.com>
Fixed in init script: - Added some missing quotes around expansions - Fixed INIT_ARGS to not pass any args if IPFS_PROFILE isn't specified - Use printf instead of "echo -e" - Only run scripts in top-level of init dir - Handle filenames correctly when finding init scripts (by using find + xargs)
b31c311
to
ed202bc
Compare
cleans up containers and images (useful when run on developer machine)
README.md
Outdated
@@ -366,6 +366,48 @@ Basic proof of 'ipfs working' locally: | |||
# QmT78zSuBmuS4z925WZfrqQ1qHaJ56DQaTfyMUF7F8ff5o | |||
ipfs cat <that hash> | |||
|
|||
To perform a custom initialization and configuration step, one can mount `.sh` scripts to a `/container-init.d` directory, or mount the entire directory. These scripts are executed if they have `+x` permission, otherwise they are just sourced. Executed scripts can exit with error and abort the initialization process. The scripts may also depend on environment variables set by the `docker run` command or in `docker-compose.yml`. The custom initialization step is **only** performed during IPFS initialization, **after** `ipfs init`, **after** after the swarm key is copied to the IPFS data directory, and **before** the daemon is started. This also enables custom containers to layer extra initialization steps to the base image by adding more scripts to `/container-init.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.
@guseggert @thattommyhall my understanding is that we already run scripts from /container-init.d
every time, so we just need to document that + emphasize that scripts should be written in idempotent way.
Would below work?
To perform a custom initialization and configuration step, one can mount `.sh` scripts to a `/container-init.d` directory, or mount the entire directory. These scripts are executed if they have `+x` permission, otherwise they are just sourced. Executed scripts can exit with error and abort the initialization process. The scripts may also depend on environment variables set by the `docker run` command or in `docker-compose.yml`. The custom initialization step is **only** performed during IPFS initialization, **after** `ipfs init`, **after** after the swarm key is copied to the IPFS data directory, and **before** the daemon is started. This also enables custom containers to layer extra initialization steps to the base image by adding more scripts to `/container-init.d`. | |
To perform a custom initialization or configuration step, one can mount `.sh` scripts to a `/container-init.d` directory, or mount the entire directory. These scripts are executed if they have `+x` permission, otherwise they are just sourced. Executed scripts can exit with error and abort the initialization process. The scripts may also depend on environment variables set by the `docker run` command or in `docker-compose.yml`. **The custom initialization steps from `/container-init.d` are run every time, and should be idempotent.** When run the first time during IPFS initialization, custom scripts run **after** `ipfs init`, **after** the swarm key is copied to the IPFS data directory, and **before** the daemon is started for the very first time. This also enables custom containers to layer extra initialization steps to the base image by adding more scripts to `/container-init.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.
Yeah it's a good distinction to make, but the prose of this paragraph bothers me a little, let me rewrite it.
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.
I also don't think this advanced stuff is appropriate for the "getting started" section of the readme. And that section is also not docker-specific
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.
I'm going to just remove this and add it to the IPFS Docker documentation , which is already linked to from the readme.
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.
Filled ipfs/ipfs-docs#1114
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.
Filled ipfs/ipfs-docs#1114
There is already IPFS Docker documentation where this should live: https://docs.ipfs.io/how-to/run-ipfs-inside-docker/
I've removed the documentation and will open a PR in https://github.com/ipfs/ipfs-docs w/ docs. |
* Add initialization directory support to Docker image * Add sharness test, fix bugs in init script Fixed in init script: - Added some missing quotes around expansions - Fixed INIT_ARGS to not pass any args if IPFS_PROFILE isn't specified - Use printf instead of "echo -e" - Only run scripts in top-level of init dir - Handle filenames correctly when finding init scripts (by using find + xargs) * chore: docker cleanup cleans up containers and images (useful when run on developer machine) * remove container init documentation from README There is already IPFS Docker documentation where this should live: https://docs.ipfs.io/how-to/run-ipfs-inside-docker/ Co-authored-by: Caian <caian@ggaunicamp.com> Co-authored-by: Marcin Rataj <lidel@lidel.org> Co-authored-by: Gus Eggert <gus@gus.dev>
started a repo of interesting examples of this https://github.com/ipfs-shipyard/go-ipfs-docker-examples |
This is a proposal to help solve advanced initialization in container environments being discussed in several issues. It follows the model used by postgres container: https://hub.docker.com/_/postgres
This PR adds support for an optional
/container-init.d
directory in the container that can be mounted with custom configuration scripts. These scripts are executed if they have +x permission, otherwise they are just sourced.This model also allows passing environment variables to the custom scripts when launching the container.
This grants flexibility to and delegates the responsability of bulding complex initializations to the users and administrators.
License: MIT
Signed-off-by: Caian Benedicto caianbene@gmail.com