Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify service manager when start up is completed #1148

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/yggdrasil/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ func main() {
}
}

if _, err = notifyStartupCompleted(); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first bool return value is never used, so why have it at all?

log.Warnln("Error while sending start up notification:", err)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarantee at this point (in the main goroutine) that startup tasks in other goroutines have been finished, since they don't synchronise.

So I think you are just racing against tun(4) setup just like -chuser is already racing against the control socket setup over at #927 (comment)

// Block until we are told to shut down.
<-ctx.Done()

Expand Down
50 changes: 50 additions & 0 deletions cmd/yggdrasil/notify_startup_linux.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//go:build linux
// +build linux

package main

import (
"net"
"os"
)

// Notify systemd daemon when start up is completed.
// Required to ensure that dependent services are started only after TUN interface is ready.
//
// One of the following is returned:
// (false, nil) - notification not supported (i.e. `notifySocketEnv` is unset)
// (false, err) - notification supported, but failure happened (e.g. error connecting to `notifySocketEnv` or while sending data)
// (true, nil) - notification supported, data has been sent
//
// Based on `SdNotify` from [`coreos/go-systemd`](https://github.com/coreos/go-systemd/blob/7d375ecc2b092916968b5601f74cca28a8de45dd/daemon/sdnotify.go#L56)
func notifyStartupCompleted() (bool, error) {
const (
notifyReady = "READY=1"
notifySocketEnv = "NOTIFY_SOCKET"
)

socketAddr := &net.UnixAddr{
Name: os.Getenv(notifySocketEnv),
Net: "unixgram",
}

// `notifySocketEnv` not set
if socketAddr.Name == "" {
return false, nil
}

os.Unsetenv(notifySocketEnv)

conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr)
// Error connecting to `notifySocketEnv`
if err != nil {
return false, err
}
defer conn.Close()

if _, err = conn.Write([]byte(notifyReady)); err != nil {
return false, err
}

return true, nil
}
8 changes: 8 additions & 0 deletions cmd/yggdrasil/notify_startup_other.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//go:build !linux
// +build !linux

package main

func notifyStartupCompleted() (bool, error) {
return false, nil
}
4 changes: 4 additions & 0 deletions contrib/systemd/yggdrasil.service
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ After=network-online.target
After=yggdrasil-default-config.service

[Service]
Type=notify
# Allow forked off processes to send notifications.
# Uncomment if e.g. yggdrasil is started by a script.
#NotifyAccess=all
Group=yggdrasil
ProtectHome=true
ProtectSystem=true
Expand Down
1 change: 1 addition & 0 deletions contrib/systemd/yggdrasil.service.debian
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ After=network-online.target
After=yggdrasil-default-config.service

[Service]
Type=notify
Group=yggdrasil
ProtectHome=true
ProtectSystem=strict
Expand Down