Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Implement weaveexec signal proxy #523

Closed
wants to merge 4 commits into from

Conversation

awh
Copy link
Contributor

@awh awh commented Apr 8, 2015

Address #507 (weave and weave --local behave differently).

Introduce an intermediate signal proxying process that:

  • Runs as PID 1 inside the weaveexec container
  • Installs a SIGINT handler
  • Forks and execs the weave script
  • Redelivers inbound SIGINTs (received from docker run --sig-proxy=true) to its own process group, which includes the shell running the weave script and any subprocesses (such as curl) it is executing
  • Waits for child to exit and then exits with child's exit status

As a consequence we can disable the problematic docker pseudoterminal allocation without losing the ability to interrupt the script running in the weaveexec container.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.21%) to 37.25% when pulling 647cd85 on awh:handle_signals_without_tty into 86abc6a on zettio:master.

@awh
Copy link
Contributor Author

awh commented Apr 8, 2015

@bboreham @dpw @rade review at your convenience, thankyou!

@@ -10,6 +10,7 @@ DOCKERHUB_USER=zettio
WEAVE_VERSION=git-$(shell git rev-parse --short=12 HEAD)
WEAVER_EXE=weaver/weaver
WEAVEDNS_EXE=weavedns/weavedns
SIGPROXY_EXE=sigproxy/sigproxy

This comment was marked as abuse.


func installSignalHandler() chan os.Signal {
sc := make(chan os.Signal, 1)
signal.Notify(sc, syscall.SIGINT)

This comment was marked as abuse.

This comment was marked as abuse.

// required to access system-dependent exit information
if exitErr, ok := err.(*exec.ExitError); ok {
if waitStatus, ok := exitErr.Sys().(syscall.WaitStatus); ok {
os.Exit(waitStatus.ExitStatus())

This comment was marked as abuse.

This comment was marked as abuse.

} else {
os.Exit(0)
}
}

This comment was marked as abuse.

Simplify waitAndExit
Commented makefile
Support SIGTERM
Remove superfluous type assertion check
Inlined functions
Remove superfluous --sig-proxy argument to docker run
@rade
Copy link
Member

rade commented Apr 10, 2015

LGTM, but would like @dpw to take a look.

@dpw
Copy link
Contributor

dpw commented Apr 10, 2015

The long comment line in the makefile needs a M-q. Fine otherwise.

@dpw
Copy link
Contributor

dpw commented Apr 10, 2015

The long comment line in the makefile needs a M-q.

Even worse - it has trailing whitespace!

@rade rade closed this in cd81101 Apr 11, 2015
@rade rade modified the milestone: 0.10.0 Apr 18, 2015
@awh awh deleted the handle_signals_without_tty branch April 28, 2015 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants