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

Fix killing of unresponsive containers by adding init process #369

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

mrnugget
Copy link
Contributor

@mrnugget mrnugget commented Nov 5, 2020

Fixes https://github.com/sourcegraph/sourcegraph/issues/15314

This adds the --init flag to the docker run command so that the
entrypoint process (bash in our case) is started by a init process.
That fixes the behaviour of the interrupt signal simply having no
effect.

From what I could gather that's because PID 1 (which is the
--entrypoint in a Docker container) is protected and cannot be killed.
But --init is given the PID1 is an init process, which forwards the
signal to our entrypoint and kills it, exiting afterwards.

Resources:

This adds the `--init` flag to the `docker run` command so that the
entrypoint process (`bash` in our case) is started by a init process.
That fixes the behaviour of the interrupt signal simply having no
effect.

From what I could gather that's because PID 1 (which is the
`--entrypoint` in a Docker container) is protected and cannot be killed.
But `--init` is given the PID1 is an init process, which forwards the
signal to our entrypoint and kills it, exiting afterwards.

Resources:

- https://stackoverflow.com/questions/45718967/how-do-you-kill-a-docker-containers-default-command-without-killing-the-entire-c
- https://unix.stackexchange.com/questions/457649/unable-to-kill-process-with-pid-1-in-docker-container
- https://stackoverflow.com/questions/31538314/stopping-docker-container-from-inside
@mrnugget mrnugget requested a review from a team November 5, 2020 15:57
Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call 1 800 555 5555 for Thorsten Ball container extermination services.

@mrnugget
Copy link
Contributor Author

mrnugget commented Nov 5, 2020

This is a campaign spec to reproduce the issue:

name: sleep
description: sleep

on:
  - repositoriesMatchingQuery: repo:automation-testing count:1

steps:
  - run: sleep 40
    container: alpine:3

changesetTemplate:
  title: SLEEP
  body: SLEEP
  branch: campaign/sleep
  commit:
    message: Sleep
  published: false

This couldn't be killed before with Ctrl-C, now it does.

Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I'm going to merge this once #368 lands so I can do a release.

@LawnGnome LawnGnome merged commit cad314d into main Nov 5, 2020
@LawnGnome LawnGnome deleted the mrn/fix-killing-of-containers branch November 5, 2020 18:22
scjohns pushed a commit that referenced this pull request Apr 24, 2023
This adds the `--init` flag to the `docker run` command so that the
entrypoint process (`bash` in our case) is started by a init process.
That fixes the behaviour of the interrupt signal simply having no
effect.

From what I could gather that's because PID 1 (which is the
`--entrypoint` in a Docker container) is protected and cannot be killed.
But `--init` is given the PID1 is an init process, which forwards the
signal to our entrypoint and kills it, exiting afterwards.

Resources:

- https://stackoverflow.com/questions/45718967/how-do-you-kill-a-docker-containers-default-command-without-killing-the-entire-c
- https://unix.stackexchange.com/questions/457649/unable-to-kill-process-with-pid-1-in-docker-container
- https://stackoverflow.com/questions/31538314/stopping-docker-container-from-inside
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.

Ctrl-c in src campaign preview|apply doesn't kill unresponsive containers
3 participants