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

Add pre_exec_hooks #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add pre_exec_hooks #66

wants to merge 1 commit into from

Conversation

0Nel
Copy link
Collaborator

@0Nel 0Nel commented Feb 1, 2023

Functionality wise this works well. However I am unsure where to place this. I feel in the .docker_scripts directory it is too hidden. In the root directory it is to top level for its functionality. In the 02_devel_image directory it seems okay, but would need some further work to properly integrate/copy/apply it to the release images.

Maybe it could be time for a top level config directory?

@0Nel 0Nel requested a review from planthaber February 1, 2023 09:50
@planthaber
Copy link
Member

I don't see the need to evaluate a list of files in a directory. if someone has a lot of calls that should be seperated, the files can still be called from the main hook script

I'd say we directly create more hooks and also a hooks folder on top level_

hooks/pre_exec.bash
hooks/post_exec.bash

hooks/pre_stop.bash
hooks/post_stop.bash

hooks/pre_delete.bash
hooks/post_delete.bash

Any more possible hooks?

Where we'd have to make sure e.g. pre/post_delete gets called not only on ./delete.bash but also when the old container gets deleted because a newer image is present

The example above are hooks for scripts in this repo, we could also have docker command based hooks: pre/post[run, start, exec, stop, delete]

What we could do in terms of folder organization is to just document the possibility to have these hooks but not provide the (empty) files in the main repo, so the scripts check if the hook script exists and is not, nothing is executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants