Skip to content

Commit

Permalink
cmd: forward signals through userns wrapper
Browse files Browse the repository at this point in the history
as of 5ffa9bd ("Reexec stacker build/recursive-build inside a user
namespace") we have a wrapper process that is the one that actually
receives the signal from the shell, and promptly ignores it. let's forward
relevant signals to the child process...

perhaps we should commonize this somehow with the stuff that's in
container.go, but this is good enough for now.

it would also be good to figure out how to test some of this, but i played
around with bats, and various incantations of subshell job forking didn't
work for me. e.g.:

~/packages/stacker fix-ctrlc cat test/ctrlc.bats
load helpers

function setup() {
    stacker_setup
}

function teardown() {
    cleanup
}

@test "stacker responds to SIGTERM" {
    cat > stacker.yaml <<EOF
test:
    from:
        type: oci
        url: $CENTOS_OCI
    run: |
        sleep 100s
EOF
    (stacker build) &
    pid=$!

    # let it get some progress; maybe this is still in go code, mabye it's in
    # the container's sleep, but it's somewhere interesting at least
    sleep 5s
    kill -SIGINT $pid
    wait $pid
}

and this at least fixes the problem, so let's just add it.

Closes project-stacker#178

Signed-off-by: Tycho Andersen <[email protected]>
  • Loading branch information
tych0 committed Aug 10, 2021
1 parent 0beef3b commit e3c0495
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
27 changes: 26 additions & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,32 @@ func main() {
cmd = append(cmd[:2], cmd[1:]...)
cmd[1] = "--internal-userns"

stackerResult(container.MaybeRunInUserns(cmd))
forward := make(chan os.Signal)
signal.Notify(forward, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGINT)

c, err := container.MaybeRunInUserns(cmd)
if err = c.Start(); err != nil {
stackerResult(err)
}
done := make(chan bool)

go func() {
for {
select {
case <-done:
return
case sig := <-forward:
err = syscall.Kill(c.Process.Pid, sig.(syscall.Signal))
if err != nil {
log.Infof("failed to forward %v through userns wrapper: %v", sig, err)
}
}
}
}()

err = errors.WithStack(c.Wait())
done <- true
stackerResult(err)
}
return nil
}
Expand Down
15 changes: 7 additions & 8 deletions container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func resolveIdmapSet(user *user.User) (*idmap.IdmapSet, error) {
return idmapSet, nil
}

func runInUserns(idmapSet *idmap.IdmapSet, userCmd []string) error {
func runInUserns(idmapSet *idmap.IdmapSet, userCmd []string) (*exec.Cmd, error) {
if idmapSet == nil {
return errors.Errorf("no subuids!")
return nil, errors.Errorf("no subuids!")
}

args := []string{}
Expand All @@ -93,13 +93,12 @@ func runInUserns(idmapSet *idmap.IdmapSet, userCmd []string) error {
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

return errors.WithStack(cmd.Run())
return cmd, nil
}

// A wrapper which runs things in a userns if we're an unprivileged user with
// an idmap, or runs things on the host if we're root and don't.
func MaybeRunInUserns(userCmd []string) error {
func MaybeRunInUserns(userCmd []string) (*exec.Cmd, error) {
// TODO: we should try to use user namespaces when we're root as well.
// For now we don't.
if os.Geteuid() == 0 {
Expand All @@ -108,17 +107,17 @@ func MaybeRunInUserns(userCmd []string) error {
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return errors.WithStack(cmd.Run())
return cmd, nil
}

idmapSet, err := ResolveCurrentIdmapSet()
if err != nil {
return err
return nil, err
}

if idmapSet == nil {
if os.Geteuid() != 0 {
return errors.Errorf("no idmap and not root, can't run %v", userCmd)
return nil, errors.Errorf("no idmap and not root, can't run %v", userCmd)
}

}
Expand Down

0 comments on commit e3c0495

Please sign in to comment.