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

remove S6 settings which are no longer necessary / startup service improvements #166

Closed
wants to merge 6 commits into from

Conversation

wiedehopf
Copy link
Contributor

Some S6 settings for this container are changed from the default, this is no longer necessary, so remove them. This means the container will exit on its own instead of running into the usual 10 second timeout of docker stop / compose down / compose up.

Simple fix for the timezone environment variable warning. (color code printed instead of colored warning)

startup service:

  • don't print the whole path via s6wrap
  • start after legacy services (not necessary for this container, but useful if this is ever moved to the baseimage)
  • run scripts in /etc/s6-overlay/finish.d as down action (not necessary for this container but useful for reuse)

S6_BEHAVIOUR_IF_STAGE2_FAILS only affects v2 legacy services and init
scripts, thus it has no effect on the current code

S6_SERVICES_GRACETIME only applies to v2 legacy services and does not
apply to the current code

S6_KILL_GRACETIME (default 3000 milliseconds)
After all services are stopped, SIGTERM is sent to all remaining processes.
s6 then waits S6_KILL_GRACETIME and sends SIGKILL to all remaining
processes.
This gracetime is never skipped even if there are no remaining
processes.
After services are stopped, no processes should be remaining if the
services are started correctly, which is currently the case for this
container.
readsb / collectd now stop correctly as part of the services being stopped
and don't need this anymore to stop cleanly
properly define startup sequence by having startup.d depend on the base
bundle (s6 v3 recommended way of enforcing that ordering)
@wiedehopf
Copy link
Contributor Author

Need to fix something else .. will re-do that PR once the issue is fixed.

@wiedehopf wiedehopf closed this Mar 20, 2024
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.

1 participant