compose_cleanup should run on pre-artifact hook

[arturopie]

Some sidecar containers, like a vnc-recorder, generate artifacts on INT signal. However, docker compose cleanup is running after buildkite upload job artifacts.

I think we should move compose_cleanup from pre-exit hook to pre-artifact.

Let me know if that makes sense.

[toolmantim]

Thanks for raising this! I'd have no problems moving the cleanup from pre-exit to pre-artifact, if you'd like to raise a pull request?

[EnricoMi]

@arturopie @toolmantim What is the outcome of the current hook order? Are no artifacts uploaded on non-zero exit code of the container? If so, then the proposed solution would solve my problem, if not, then I would have to raise a separate issue.

[arturopie]

@EnricoMi only if the artifact is generated when the container receives the INT signal during job clean up

[EnricoMi]

@toolmantim would that only mean to rename hooks/pre-exit to hooks/pre-artifact? There is only compose_cleanup happening in pre-exit.

[toote]

I don't think that this issue can be solved in this plugin. The cleanup script needs to run in the pre-exit hook so that it is (mostly) guaranteed to run even when jobs are cancelled. As mentioned in #288, moving it to pre-artifact will only be run when an artifact path is defined. That means it will probably break every other use case when artifact paths are not defined.

The one workaround I could think of is adding a repository post-command hook to stop the container in that particular repo to trigger the signal and generate the artifacts before everything else is run.