Skip to content
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

Merged
merged 9 commits into from
Apr 12, 2022

Conversation

Caian
Copy link
Contributor

@Caian Caian commented Aug 16, 2019

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

@Caian Caian changed the title Feat/container init scripts Docker initscript: Add optional init directory for advanced initialization Aug 16, 2019
README.md Outdated Show resolved Hide resolved
@Caian
Copy link
Contributor Author

Caian commented Sep 2, 2019

README updated with example per request of @olizilla

@Caian Caian requested a review from olizilla October 15, 2019 17:58
@lidel lidel requested review from lidel and guseggert and removed request for lanzafame, guseggert and olizilla September 3, 2021 19:02
@lidel lidel changed the title Docker initscript: Add optional init directory for advanced initialization feat(docker): /container-init.d for advanced initialization Sep 3, 2021
@lidel lidel force-pushed the feat/container-init-scripts branch 2 times, most recently from 7354767 to 2b3c534 Compare September 3, 2021 19:48
Copy link
Member

@lidel lidel left a 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 execute ipfs bootstrap add and then we could check if Bootstrap 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?

@BigLep BigLep added this to the go-ipfs 0.11 milestone Sep 10, 2021
@lidel lidel added help wanted Seeking public contribution on this issue need_tests labels Sep 24, 2021
@lidel lidel removed their assignment Sep 24, 2021
@BigLep
Copy link
Contributor

BigLep commented Oct 1, 2021

Next step is to add tests.

bin/container_daemon Outdated Show resolved Hide resolved
bin/container_daemon Outdated Show resolved Hide resolved
@guseggert
Copy link
Contributor

Adding sharness tests for this will take a bit of time, as the current Docker tests are broken and need to be rewritten.

@guseggert guseggert mentioned this pull request Nov 23, 2021
80 tasks
@BigLep
Copy link
Contributor

BigLep commented Apr 1, 2022

2022-04-01:

  • Needs review (@thattommyhall for ergonomics of someone using this, @lidel for sharness)
  • Is getting hit by docker flakiness (separate work item).

@BigLep
Copy link
Contributor

BigLep commented Apr 1, 2022

@thattommyhall : it would be great if you can give review of this as as a user of the functionality.

@BigLep
Copy link
Contributor

BigLep commented Apr 1, 2022

@thattommyhall : did you meean to close this?

@BigLep BigLep reopened this Apr 1, 2022
@lidel lidel self-assigned this Apr 11, 2022
Caian and others added 7 commits April 11, 2022 15:22
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)
@lidel lidel force-pushed the feat/container-init-scripts branch from b31c311 to ed202bc Compare April 11, 2022 13:22
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`.
Copy link
Member

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?

Suggested change
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`.

Copy link
Contributor

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.

Copy link
Contributor

@guseggert guseggert Apr 11, 2022

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

Copy link
Contributor

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidel lidel requested a review from guseggert April 11, 2022 15:20
There is already IPFS Docker documentation where this should live:

https://docs.ipfs.io/how-to/run-ipfs-inside-docker/
@guseggert
Copy link
Contributor

guseggert commented Apr 11, 2022

I've removed the documentation and will open a PR in https://github.com/ipfs/ipfs-docs w/ docs.
Update: filled ipfs/ipfs-docs#1114

@lidel lidel merged commit 63b0025 into ipfs:master Apr 12, 2022
xrazis pushed a commit to xrazis/kubo that referenced this pull request Apr 14, 2022
* 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>
@thattommyhall
Copy link
Member

started a repo of interesting examples of this https://github.com/ipfs-shipyard/go-ipfs-docker-examples

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue need_tests
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants