agent: notify systemd after JoinLAN (#2121) #3164
Conversation
| // Notify sends a message to the init daemon. It is common to ignore the error. | ||
| func (n *Notifier) Notify(state string) error { | ||
| addr := &net.UnixAddr{ | ||
| Name: os.Getenv("NOTIFY_SOCKET"), |
There was a problem hiding this comment.
Should this be fleshed out into a normal config value that's also settable via CLI arg/config file?
There was a problem hiding this comment.
I don't think so since the name of the env var is fixed when the type is notify. I've previously missed that READY=1 is the value we are supposed to send when the service is up so it may make sense to define some constants.
https://www.freedesktop.org/software/systemd/man/sd_notify.html
There was a problem hiding this comment.
Right, I didn't realize systemd was setting that env var. This seems fine then.
agent/agent.go
Outdated
| @@ -1188,6 +1198,9 @@ func (a *Agent) JoinLAN(addrs []string) (n int, err error) { | |||
| a.logger.Printf("[INFO] agent: (LAN) joining: %v", addrs) | |||
| n, err = a.delegate.JoinLAN(addrs) | |||
| a.logger.Printf("[INFO] agent: (LAN) joined: %d Err: %v", n, err) | |||
| if err == nil && a.joinLANNotifier != nil { | |||
| a.joinLANNotifier.Notify("READY=1") | |||
There was a problem hiding this comment.
Do you want to log any non-nil error that's not NotfyNoSocket (since presumably that will be returned when not running under systemd? We could debug-level log for that one.
There was a problem hiding this comment.
yeah, might make sense. I'll also move the READY=1 into the systemd package since that is a magic value.
| type Notifier struct{} | ||
|
|
||
| // Notify sends a message to the init daemon. It is common to ignore the error. | ||
| func (n *Notifier) Notify(state string) error { |
There was a problem hiding this comment.
This should get documented some place, maybe on https://www.consul.io/docs/agent/basics.html (Running and Stopping).
There was a problem hiding this comment.
Ah, docs ... Reading is so '90s :) Will do
| @@ -1188,6 +1198,9 @@ func (a *Agent) JoinLAN(addrs []string) (n int, err error) { | |||
| a.logger.Printf("[INFO] agent: (LAN) joining: %v", addrs) | |||
| n, err = a.delegate.JoinLAN(addrs) | |||
| a.logger.Printf("[INFO] agent: (LAN) joined: %d Err: %v", n, err) | |||
| if err == nil && a.joinLANNotifier != nil { | |||
There was a problem hiding this comment.
I never knew about this before, but there is a callback you can hook into to get notified when a server is known about, this might be a little later/safer time to notify since at that point you will know that a server is around to start to handle requests - https://github.com/hashicorp/consul/blob/master/agent/consul/client.go#L289-L292. There's a hook on here already so you'd need to chain to that one, and that will get called multiple times, so you'd need to deal with that, but it's an interesting place to trigger this. It also looks like this doesn't get hooked on servers, just clients.
There was a problem hiding this comment.
The question is then whether that provides additional value. If you have to bring up your servers for this to work then JoinLAN should do the same thing, shouldn't it?
There was a problem hiding this comment.
It won't fire the event if you script your join from some outside thing that does consul join or uses the /v1/agent APIs, for example. I'd be ok leaving this as-is as long as we document that this event only comes out if you provide a join configuration to Consul when you start it, since this is a lot simpler to understand like this vs. the hook.
There was a problem hiding this comment.
Lets leave as is since this is simple and should cover the common use case. We can make it more complex if necessary.
This patch adds support for notifying systemd via the NOTIFY_SOCKET by sending 'READY=1' to the socket after a successful JoinLAN. Fixes #2121
magiconair
force-pushed
the
issue-2121-systemd-notif
branch
from
8b065ff to
fff1bac
Compare
This patch adds support for notifying systemd via the
NOTIFY_SOCKET by sending 'READY=1' to the socket after
a successful JoinLAN.
Fixes #2121