Skip to content

[supervisor] Properly terminate background tasks #2654

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

Closed
csweichel opened this issue Jan 4, 2021 · 5 comments
Closed

[supervisor] Properly terminate background tasks #2654

csweichel opened this issue Jan 4, 2021 · 5 comments
Assignees
Milestone

Comments

@csweichel
Copy link
Contributor

When a Gitpod task starts a process in the background (e.g. sudo docker-up &) that process might not be subject to the same termination rules that other children of supervisor are.

When supervisor is asked to shut down (receives SIGINT/SIGTERM) it sends SIGINT to its terminals followed by a SIGKILL a few seconds later.

We should investigate what happens to background processes started by supervisor's task in this scenario.
Do they receive the same signal regimen, are they SIGKILL'ed or reparented to some supervisor process?

@csweichel csweichel added this to the January 2021 milestone Jan 4, 2021
@csweichel csweichel added the roadmap item: root & docker https://bit.ly/2XlgB5p label Jan 8, 2021
@csweichel csweichel self-assigned this Jan 8, 2021
@csweichel
Copy link
Contributor Author

csweichel commented Jan 13, 2021

The motivation for this issue is running docker-up during prebuilds. Prebuilds finish when all their tasks finish, which means we can't just run sudo docker-up in a prebuild. Instead, we have a few options:

sudo docker-up &

  • 👍 comes built-in with the bash, it's nothing we have to implement
  • 👍 it's somewhat "intuitive", i.e. it's what more experienced users would do once they know that all tasks have to terminate
  • 👎 bash does not forward sigint/sigterm to background processes but kills them hard. This prevents orderly shutdown of the background task.

discover background tasks and gracefully terminate

We'd have the supervisor inspect the process hierarchy, detect background tasks and terminate them with the same shutdown procedure it uses for terminals (sigterm followed by sigkill).

gp daemonize

We'd introduce a gp daemonize (e.g. gp daemonize -- sudo docker-up) command which would fork the child and then exit, making sure the child is reparented to the PID1 process beforehand.

  • 👍 adds a neat runtime-level annotation to "background tasks"
  • 👎 it's unclear how this would actually work, i.e. if/how we can do that reparenting
  • 👎 it's something unusual, i.e. not something other environments/systems use

background tasks in .gitpod.yml

We'd introduce a special background task, or possibly a marker on the tasks whereby prebuilds would no longer wait for those tasks to complete.

  • 👍 it's a very explicit mechanism and clearly communicates how prebuild tasks behave
  • 👎 it's fairly intrusive and requires changes to quite a few bits within Gitpod (config, supervisor)
  • 👎 semantics aren't immediately clear to users. How would a background task differ from an init task? If we wanted to annotate existing tasks instead, how would we go about that? A task is defined by its command only.

@weitzman
Copy link

So is it now possible to use dockerd in prebuild?

@weitzman
Copy link

Given that there is no new command in the PR mentioned here, perhaps sudo docker-up & option was chosen? Also, is this live now on gitpod.io?

@akosyakov
Copy link
Member

Yes, it should be on prod.

@shaal
Copy link
Contributor

shaal commented Mar 1, 2021

👍
Thank you! I tested it on a branch and it works!
sudo docker-up & allow running docker commands during the prebuild process.

pavan-tri pushed a commit to trilogy-group/gitpod that referenced this issue Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants