cleanup created docker containers

[mikz]

Cancelled or timeouted workflow would keep the docker container running. Closes game-ci/unity-test-runner#197

This has two parts:

Part one. The entrypoints.

runs.post: GitHub Action metadata allow running something after the action (regardless of a failure, crash, timeout, ...). https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runspost

However, it needs to be a .js file and it can't be configured to pass any arguments.

There already was index.js used as the main entrypoint. The build process of this file uses typescript compiler and ncc to pack all dependencies into one .js file. And ncc has no way of generating multiple files in one go, so the only solution would be to run ncc twice and generate two independent files.

That would be quite unfortunate, wasting time and storage. So I rather came up with a new entrypoint that symlinked from two locations. And this new entrypoint understands how it was executed, so it can run the correct behaviour. This makes it easy to add runs.pre if needed.

This new entrypoint is in index.ts. The original index.ts is now in main.ts.

Part two. The signals. I've tried:

• try/catch/finally around the await Docker.run. Catch and finally are not executed when process receives SIGINT. See the discussion in: https://github.com/nodejs/node/discussions/29480
• New AbortController and AbortSignal. Great concept, but the action.exec does not support it. So it can't be aborted.
• Doing cleanup on process.on('exit'). Unfortunately you can't really do async stuff from there, so can't really run the docker rm command to delete the container.
• Using process.on('SIGINT'). For some reason that wasn't really executing for me. I'd not put my hand in fire for this, but I assume because it was in the signal handler it does something special, or would heed to be scheduled for later with setTimeout(0).

Evaluating all these I came to a conclusion that it is fragile and just relying on a runs.post is much better and safer.

I've not tested this on Windows, so that probably needs to happen. I've tried running locally with act and it worked. Worked also on my hosted linux runner.

[github-actions[bot]]

[webbertakken]

Great explanation! Thanks for the elaborate details, it will be very helpful for merging this.

Evaluating all these I came to a conclusion that it is fragile and just relying on a runs.post is much better and safer.

Ok, based on your research I would agree. Let's go for your approach.

Using process.on('SIGINT'). For some reason that wasn't really executing for me. I'd not put my hand in fire for this, but I assume because it was in the signal handler it does something special, or would heed to be scheduled for later with setTimeout(0).

As I understand it, that's probably because an event listener doesn't stop the event from bubbling and you only have a few milliseconds to do any cleanup.

[mikz]

I've updated it to clear up things brought in the review.

I've also added some type annotations to the new code, so it explicitly handles default values. IMO that is important in case this ever gets executed outside of Github Action Runner (like unit tests, or trying it locally).

[webbertakken]

LGTM. This can be merged once it's tested.

[mikz]

I'm running this on my hosted runner with concurrency limited cancellable workflows and it works well. Don't have Windows pipeline to try it on.