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

Refactor exec of child process to trap, spawn, signal, and wait pattern #823

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

felddy
Copy link
Owner

@felddy felddy commented Oct 20, 2023

🗣 Description

This PR changes the way the child processes are managed by the entrypoint script.
Specifically it backs out some of the changes from #822

Instead of execing the launcher.sh it will now be spawned into the background and waited on.
TERM signals will be trapped and forwarded to the child while waiting.
This allows the DDOS bucket protection code to execute if needed.

It also reduces the sleep interval in the infinite loop to allow container shutdown to be more responsive
when sleeping.

See:

Closes #813

💭 Motivation and context

I realized that the DDOS protection in the entrypoint would not execute after the exec call.
I expect that there are deployments that have disabled caching that may still need this mitigation.

🧪 Testing

I tested locally that the signals are properly handled by the child processes.
I disable the CONTAINER_CACHE and killed the node process to verify that the entrypoint would
execute the infinite sleep.
Also CI.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • All new and existing tests pass.

The exec was inadvertently disabling the DDOS protection for
the FoundryVTT distribution servers (S3)
when the caching is disabled.
This will allow the container to exit more responsively when requested.
@felddy felddy force-pushed the improvement/signals branch from f41b235 to cfc4e64 Compare November 1, 2023 00:22
@felddy felddy enabled auto-merge November 1, 2023 00:22
@felddy felddy merged commit 5f8cd90 into develop Nov 1, 2023
37 checks passed
@felddy felddy deleted the improvement/signals branch November 1, 2023 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remaining config.json.lock folder after stopping the container
1 participant