Support running on demand via systemd #151
Conversation
fd0
force-pushed
the
add-socket-activation
branch
from
7aa41a3
to
b8058fe
Compare
cmd/rest-server/main.go
Outdated
| panic(err) | ||
| } | ||
|
|
||
| switch len(listeners) { |
There was a problem hiding this comment.
Perhaps make the systemd socket activation explicit with a --systemd switch?
There was a problem hiding this comment.
Why do you think that's needed? We can discover (from the environment) if we're run via systemd socket activation...
There was a problem hiding this comment.
In that case you will end up ignoring the address configuration and using systemd instead, which could surprise the user. Adding a --systemd flag also makes clear that some magic is happening here.
There was a problem hiding this comment.
On the other hand, rest-server does log that it is in systemd mode, so I guess this is fine,
fd0
force-pushed
the
add-socket-activation
branch
from
b8058fe
to
7f3ce33
Compare
cmd/rest-server/listener_windows.go
Outdated
| // findListener creates a listener. | ||
| func findListener(inetd bool, addr string) (listener net.Listener, err error) { | ||
| if inetd { | ||
| log.Print("inetd mode activated") |
There was a problem hiding this comment.
Can this even work on windows?
There was a problem hiding this comment.
Via cygwin maybe? Or WSL? It compiled, so I left it there...
fd0
changed the title
|
inetd mode is off: #126 (comment) |
fd0
force-pushed
the
add-socket-activation
branch
2 times, most recently
from
6921484
to
0f90ebf
Compare
fd0
force-pushed
the
add-socket-activation
branch
from
0f90ebf
to
f90205e
Compare
| // | ||
| // systemd-socket-activate -l 8080 ./rest-server | ||
| log.Printf("systemd socket activation mode") | ||
| return listeners[0], nil |
There was a problem hiding this comment.
I'm not sure who should do the check, but if the environmental flags don't match the passed in file descriptors, listerners[0] can be nil here and result in a panic later. Not a very nice to debug.
There was a problem hiding this comment.
Checking that listeners[0] is valid is probably a good idea before returning it. The expected behavior of findListener is that it either returns a listening socket or an error. Returning nil is thus wrong. Would you be willing to open a PR to check that listeners[0] != nil?
There was a problem hiding this comment.
I've opened coreos/go-systemd#405 for now, let's see if they are willing to fix the condition on their end.
What is the purpose of this change? What does it change?
This change allows
rest-serverto be run on demand by systemd (socket activation). The rationale for supporting both is as follows:rest-serverdoes not have to have the privileges to listen on a port.Was the change discussed in an issue or in the forum before?
Closes #126
Supersedes #127
Checklist
changelog/unreleased/that describes the changes for our users (template here)gofmton the code in all commits