pkg/systemd: don't require LISTEN_FDNAMES for socket activation

LISTEN_FDNAMES is optional, the docs for sd_listen_fds() says:

    This information is read from the $LISTEN_FDNAMES variable, which
    **may** contain a colon-separated list of names.

emphasis mine (indeed, the cited coreos code also suggests it is optional).

This actually results in bug, since the default
/contrib/systemd/system/podman.socket file doesn't set a
FileDescriptorName=. podman when run with this systemd configuration
*always* starts in unix socket mode since SocketActivated() will return
false because the name is missing.

The bug is a race with a very small window: between when podman does the
unlink() and when it re-binds the socket later in the code, requests made
during this time will fail since nothing is listening. There's another
small race when the service stops and systemd realizes it and starts
listening again.

However, small this window we managed to hit it :).

Let's fix this by ignoring LISTEN_FDNAMES. Since the code in
cmd/podman/system/service_abi.go:restService() ignores this value anyway
when setting up the socket activated stuff, there's no real loss here.

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
This commit is contained in:
Tycho Andersen 2021-05-20 13:01:21 -06:00
parent 21d80faa0b
commit 364e8a26da
2 changed files with 32 additions and 6 deletions

View File

@ -25,11 +25,5 @@ func SocketActivated() bool {
if err != nil || nfds == 0 {
return false
}
// "github.com/coreos/go-systemd/v22/activation" will use and validate this variable's
// value. We're just providing a fast fail
if _, found = os.LookupEnv("LISTEN_FDNAMES"); !found {
return false
}
return true
}

View File

@ -0,0 +1,32 @@
package systemd
import (
"fmt"
"os"
"testing"
"github.com/stretchr/testify/assert"
)
func TestSocketActivated(t *testing.T) {
assert := assert.New(t)
assert.False(SocketActivated())
// different pid
assert.NoError(os.Setenv("LISTEN_PID", "1"))
assert.False(SocketActivated())
// same pid no fds
assert.NoError(os.Setenv("LISTEN_PID", fmt.Sprintf("%d", os.Getpid())))
assert.NoError(os.Setenv("LISTEN_FDS", "0"))
assert.False(SocketActivated())
// same pid some fds
assert.NoError(os.Setenv("LISTEN_FDS", "1"))
assert.True(SocketActivated())
// FDNAME is ok too (but not required)
assert.NoError(os.Setenv("LISTEN_FDNAMES", "/meshuggah/rocks"))
assert.True(SocketActivated())
}