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.
4 participants
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