Skip to content

Commit 6bc52c9

Browse files
Luap99Luap99
committed
pkg/rootless: correctly handle proxy signals on reexec
There are quite a lot of places in podman were we have some signal handlers, most notably libpod/shutdown/handler.go. However when we rexec we do not want any of that and just send all signals we get down to the child obviously. So before we install our signal handler we must first reset all others with signal.Reset(). Also while at it fix a problem were the joinUserAndMountNS() code path would not forward signals at all. This code path is used when you have running containers but the pause process was killed. Fixes containers#16091 Given that signal handlers run in different goroutines parallel it would explain why it flakes sometimes in CI. However to my understanding this flake can only happen when the pause process is dead before we run the podman command. So the question still is what kills the pause process? Signed-off-by: Paul Holzinger <[email protected]>
1 parent 47ac6c4 commit 6bc52c9

File tree

4 files changed

+126
-58
lines changed

4 files changed

+126
-58
lines changed

‎pkg/rootless/rootless_linux.go

+13-11
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,7 @@ func joinUserAndMountNS(pid uint, pausePid string) (bool, int, error) {
184184
return false, -1, fmt.Errorf("cannot re-exec process to join the existing user namespace")
185185
}
186186

187-
ret := C.reexec_in_user_namespace_wait(pidC, 0)
188-
if ret < 0 {
189-
return false, -1, errors.New("waiting for the re-exec process")
190-
}
191-
192-
return true, int(ret), nil
187+
return waitAndProxySignalsToChild(pidC)
193188
}
194189

195190
// GetConfiguredMappings returns the additional IDs configured for the current user.
@@ -395,7 +390,6 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (_ boo
395390
if ret < 0 {
396391
return false, -1, errors.New("waiting for the re-exec process")
397392
}
398-
399393
return true, 0, nil
400394
}
401395

@@ -418,6 +412,10 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (_ boo
418412
return false, -1, errors.New("setting up the process")
419413
}
420414

415+
return waitAndProxySignalsToChild(pidC)
416+
}
417+
418+
func waitAndProxySignalsToChild(pid C.int) (bool, int, error) {
421419
signals := []os.Signal{}
422420
for sig := 0; sig < numSig; sig++ {
423421
if sig == int(unix.SIGTSTP) {
@@ -426,24 +424,28 @@ func becomeRootInUserNS(pausePid, fileToRead string, fileOutput *os.File) (_ boo
426424
signals = append(signals, unix.Signal(sig))
427425
}
428426

427+
// Disable all existing signal handlers, from now forward everything to the child and let
428+
// it deal with it. All we do is to wait and propagate the exit code from the child to our parent.
429+
gosignal.Reset()
429430
c := make(chan os.Signal, len(signals))
430431
gosignal.Notify(c, signals...)
431-
defer gosignal.Reset()
432432
go func() {
433433
for s := range c {
434434
if s == unix.SIGCHLD || s == unix.SIGPIPE {
435435
continue
436436
}
437437

438-
if err := unix.Kill(int(pidC), s.(unix.Signal)); err != nil {
438+
if err := unix.Kill(int(pid), s.(unix.Signal)); err != nil {
439439
if err != unix.ESRCH {
440-
logrus.Errorf("Failed to propagate signal to child process %d: %v", int(pidC), err)
440+
logrus.Errorf("Failed to propagate signal to child process %d: %v", int(pid), err)
441441
}
442442
}
443443
}
444444
}()
445445

446-
ret := C.reexec_in_user_namespace_wait(pidC, 0)
446+
ret := C.reexec_in_user_namespace_wait(pid, 0)
447+
// child exited reset our signal proxy handler
448+
gosignal.Reset()
447449
if ret < 0 {
448450
return false, -1, errors.New("waiting for the re-exec process")
449451
}

‎test/system/032-sig-proxy.bats

+2-47
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,9 @@
11
#!/usr/bin/env bats
22

33
load helpers
4+
load helpers.sig-proxy
45

5-
# Command to run in each of the tests.
6-
SLEEPLOOP='trap "echo BYE;exit 0" INT;echo READY;while :;do sleep 0.1;done'
7-
8-
# Main test code: wait for container to exist and be ready, send it a
9-
# signal, wait for container to acknowledge and exit.
10-
function _test_sigproxy() {
11-
local cname=$1
12-
local kidpid=$2
13-
14-
# Wait for container to appear
15-
local timeout=10
16-
while :;do
17-
sleep 0.5
18-
run_podman '?' container exists $cname
19-
if [[ $status -eq 0 ]]; then
20-
break
21-
fi
22-
timeout=$((timeout - 1))
23-
if [[ $timeout -eq 0 ]]; then
24-
run_podman ps -a
25-
die "Timed out waiting for container $cname to start"
26-
fi
27-
done
28-
29-
# Now that container exists, wait for it to declare itself READY
30-
wait_for_ready $cname
31-
32-
# Signal, and wait for container to exit
33-
kill -INT $kidpid
34-
timeout=20
35-
while :;do
36-
sleep 0.5
37-
run_podman logs $cname
38-
if [[ "$output" =~ BYE ]]; then
39-
break
40-
fi
41-
timeout=$((timeout - 1))
42-
if [[ $timeout -eq 0 ]]; then
43-
run_podman ps -a
44-
die "Timed out waiting for BYE from container"
45-
fi
46-
done
47-
48-
run_podman rm -f -t0 $cname
49-
}
50-
51-
# Each of the tests below does some setup, then invokes the above helper.
6+
# Each of the tests below does some setup, then invokes the helper from helpers.sig-proxy.bash.
527

538
@test "podman sigproxy test: run" {
549
# We're forced to use $PODMAN because run_podman cannot be backgrounded

‎test/system/550-pause-process.bats

+61
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#
55

66
load helpers
7+
load helpers.sig-proxy
78

89
function _check_pause_process() {
910
pause_pid=
@@ -77,3 +78,63 @@ function _check_pause_process() {
7778
assert "$output" == "$tmpdir_userns" \
7879
"podman with tmpdir2 should use the same userns created using a tmpdir"
7980
}
81+
82+
# https://github.com/containers/podman/issues/16091
83+
@test "rootless reexec with sig-proxy" {
84+
skip_if_not_rootless "pause process is only used as rootless"
85+
skip_if_remote "system migrate not supported via remote"
86+
87+
# Use podman system migrate to stop the currently running pause process
88+
run_podman system migrate
89+
90+
# We're forced to use $PODMAN because run_podman cannot be backgrounded
91+
$PODMAN run -i --name c_run $IMAGE sh -c "$SLEEPLOOP" &
92+
local kidpid=$!
93+
94+
_test_sigproxy c_run $kidpid
95+
96+
# our container exits 0 so podman should too
97+
wait $kidpid || die "podman run exited $? instead of zero"
98+
}
99+
100+
101+
@test "rootless reexec with sig-proxy when rejoining userns from container" {
102+
skip_if_not_rootless "pause process is only used as rootless"
103+
skip_if_remote "unshare not supported via remote"
104+
105+
# System tests can execute in contexts without XDG; in those, we have to
106+
# skip the pause-pid-file checks.
107+
if [[ -z "$XDG_RUNTIME_DIR" ]]; then
108+
skip "\$XDG_RUNTIME_DIR not defined"
109+
fi
110+
local pause_pid_file="$XDG_RUNTIME_DIR/libpod/tmp/pause.pid"
111+
112+
# First let's run a container in the background to keep the userns active
113+
local cname1=c1_$(random_string)
114+
run_podman run -d --name $cname1 $IMAGE top
115+
116+
run_podman unshare readlink /proc/self/ns/user
117+
userns="$output"
118+
119+
# check for pause pid and then kill it
120+
_check_pause_process
121+
kill -9 $pause_pid
122+
123+
# Now again directly start podman run and make sure it can forward signals
124+
# We're forced to use $PODMAN because run_podman cannot be backgrounded
125+
local cname2=c2_$(random_string)
126+
$PODMAN run -i --name $cname2 $IMAGE sh -c "$SLEEPLOOP" &
127+
local kidpid=$!
128+
129+
_test_sigproxy $cname2 $kidpid
130+
131+
# our container exits 0 so podman should too
132+
wait $kidpid || die "podman run exited $? instead of zero"
133+
134+
# Check that podman joined the same userns as it tries to use the one
135+
# from the running podman process in the background.
136+
run_podman unshare readlink /proc/self/ns/user
137+
assert "$output" == "$userns" "userns before/after kill is the same"
138+
139+
run_podman rm -f -t0 $cname1
140+
}

‎test/system/helpers.sig-proxy.bash

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# -*- bash -*-
2+
#
3+
# BATS helpers for sig-proxy functionality
4+
#
5+
6+
# Command to run in each of the tests.
7+
SLEEPLOOP='trap "echo BYE;exit 0" INT;echo READY;while :;do sleep 0.1;done'
8+
9+
# Main test code: wait for container to exist and be ready, send it a
10+
# signal, wait for container to acknowledge and exit.
11+
function _test_sigproxy() {
12+
local cname=$1
13+
local kidpid=$2
14+
15+
# Wait for container to appear
16+
local timeout=10
17+
while :;do
18+
sleep 0.5
19+
run_podman '?' container exists $cname
20+
if [[ $status -eq 0 ]]; then
21+
break
22+
fi
23+
timeout=$((timeout - 1))
24+
if [[ $timeout -eq 0 ]]; then
25+
run_podman ps -a
26+
die "Timed out waiting for container $cname to start"
27+
fi
28+
done
29+
30+
# Now that container exists, wait for it to declare itself READY
31+
wait_for_ready $cname
32+
33+
# Signal, and wait for container to exit
34+
kill -INT $kidpid
35+
timeout=20
36+
while :;do
37+
sleep 0.5
38+
run_podman logs $cname
39+
if [[ "$output" =~ BYE ]]; then
40+
break
41+
fi
42+
timeout=$((timeout - 1))
43+
if [[ $timeout -eq 0 ]]; then
44+
run_podman ps -a
45+
die "Timed out waiting for BYE from container"
46+
fi
47+
done
48+
49+
run_podman rm -f -t0 $cname
50+
}

0 commit comments

Comments
 (0)