From 59286d2c7ede928cc24916abdf6c0fb9ee12b826 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 2 Sep 2022 09:23:51 -0600 Subject: [PATCH] notify: Don't send ready after error (fix #5003) Also simplify the notify package quite a bit. Also move stop notification into better place. Add ability to send status or error. --- admin.go | 5 --- caddy.go | 26 +++++++++++--- notify/notify.go | 30 ----------------- notify/notify_linux.go | 73 ++++++++++++++++++++-------------------- notify/notify_other.go | 8 +++-- notify/notify_windows.go | 12 +++++-- 6 files changed, 71 insertions(+), 83 deletions(-) delete mode 100644 notify/notify.go diff --git a/admin.go b/admin.go index a9432bf5..36cc2f82 100644 --- a/admin.go +++ b/admin.go @@ -40,7 +40,6 @@ import ( "sync" "time" - "github.com/caddyserver/caddy/v2/notify" "github.com/caddyserver/certmagic" "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" @@ -1020,10 +1019,6 @@ func handleStop(w http.ResponseWriter, r *http.Request) error { } } - if err := notify.NotifyStopping(); err != nil { - Log().Error("unable to notify stopping to service manager", zap.Error(err)) - } - exitProcess(context.Background(), Log().Named("admin.api")) return nil } diff --git a/caddy.go b/caddy.go index 6964f77c..4a82d22c 100644 --- a/caddy.go +++ b/caddy.go @@ -102,20 +102,32 @@ func Run(cfg *Config) error { // if it is different from the current config or // forceReload is true. func Load(cfgJSON []byte, forceReload bool) error { - if err := notify.NotifyReloading(); err != nil { - Log().Error("unable to notify reloading to service manager", zap.Error(err)) + if err := notify.Reloading(); err != nil { + Log().Error("unable to notify service manager of reloading state", zap.Error(err)) } + // after reload, notify system of success or, if + // failure, update with status (error message) + var err error defer func() { - if err := notify.NotifyReadiness(); err != nil { - Log().Error("unable to notify readiness to service manager", zap.Error(err)) + if err != nil { + if notifyErr := notify.Error(err, 0); notifyErr != nil { + Log().Error("unable to notify to service manager of reload error", + zap.Error(err), + zap.String("reload_err", err.Error())) + } + return + } + if err := notify.Ready(); err != nil { + Log().Error("unable to notify to service manager of ready state", zap.Error(err)) } }() - err := changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, "", forceReload) + err = changeConfig(http.MethodPost, "/"+rawConfigKey, cfgJSON, "", forceReload) if errors.Is(err, errSameConfig) { err = nil // not really an error } + return err } @@ -664,6 +676,10 @@ func Validate(cfg *Config) error { // Errors are logged along the way, and an appropriate exit // code is emitted. func exitProcess(ctx context.Context, logger *zap.Logger) { + if err := notify.Stopping(); err != nil { + Log().Error("unable to notify service manager of stopping state", zap.Error(err)) + } + if logger == nil { logger = Log() } diff --git a/notify/notify.go b/notify/notify.go deleted file mode 100644 index bca80c1f..00000000 --- a/notify/notify.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2015 Matthew Holt and The Caddy Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package notify - -// NotifyReadiness notifies process manager of readiness. -func NotifyReadiness() error { - return notifyReadiness() -} - -// NotifyReloading notifies process manager of reloading. -func NotifyReloading() error { - return notifyReloading() -} - -// NotifyStopping notifies process manager of stopping. -func NotifyStopping() error { - return notifyStopping() -} diff --git a/notify/notify_linux.go b/notify/notify_linux.go index 1e8da742..3457a5a6 100644 --- a/notify/notify_linux.go +++ b/notify/notify_linux.go @@ -17,7 +17,7 @@ package notify import ( - "io" + "fmt" "net" "os" "strings" @@ -26,9 +26,13 @@ import ( // The documentation about this IPC protocol is available here: // https://www.freedesktop.org/software/systemd/man/sd_notify.html -func sdNotify(path, payload string) error { +func sdNotify(payload string) error { + if socketPath == "" { + return nil + } + socketAddr := &net.UnixAddr{ - Name: path, + Name: socketPath, Net: "unixgram", } @@ -38,45 +42,40 @@ func sdNotify(path, payload string) error { } defer conn.Close() - if _, err := io.Copy(conn, strings.NewReader(payload)); err != nil { - return err - } - return nil + _, err = conn.Write([]byte(payload)) + return err } -// notifyReadiness notifies systemd that caddy has finished its +// Ready notifies systemd that caddy has finished its // initialization routines. -func notifyReadiness() error { - val, ok := os.LookupEnv("NOTIFY_SOCKET") - if !ok || val == "" { - return nil - } - if err := sdNotify(val, "READY=1"); err != nil { - return err - } - return nil +func Ready() error { + return sdNotify("READY=1") } -// notifyReloading notifies systemd that caddy is reloading its config. -func notifyReloading() error { - val, ok := os.LookupEnv("NOTIFY_SOCKET") - if !ok || val == "" { - return nil - } - if err := sdNotify(val, "RELOADING=1"); err != nil { - return err - } - return nil +// Reloading notifies systemd that caddy is reloading its config. +func Reloading() error { + return sdNotify("RELOADING=1") } -// notifyStopping notifies systemd that caddy is stopping. -func notifyStopping() error { - val, ok := os.LookupEnv("NOTIFY_SOCKET") - if !ok || val == "" { - return nil - } - if err := sdNotify(val, "STOPPING=1"); err != nil { - return err - } - return nil +// Stopping notifies systemd that caddy is stopping. +func Stopping() error { + return sdNotify("STOPPING=1") } + +// Status sends systemd an updated status message. +func Status(msg string) error { + return sdNotify("STATUS=" + msg) +} + +// Error is like Status, but sends systemd an error message +// instead, with an optional errno-style error number. +func Error(err error, errno int) error { + collapsedErr := strings.ReplaceAll(err.Error(), "\n", " ") + msg := fmt.Sprintf("STATUS=%s", collapsedErr) + if errno > 0 { + msg += fmt.Sprintf("\nERRNO=%d", errno) + } + return sdNotify(msg) +} + +var socketPath, _ = os.LookupEnv("NOTIFY_SOCKET") diff --git a/notify/notify_other.go b/notify/notify_other.go index 7fc618b7..dbe9bdb9 100644 --- a/notify/notify_other.go +++ b/notify/notify_other.go @@ -16,6 +16,8 @@ package notify -func notifyReadiness() error { return nil } -func notifyReloading() error { return nil } -func notifyStopping() error { return nil } +func Ready() error { return nil } +func Reloading() error { return nil } +func Stopping() error { return nil } +func Status(_ string) error { return nil } +func Error(_ error, _ int) error { return nil } diff --git a/notify/notify_windows.go b/notify/notify_windows.go index a551fc7e..5666a4c2 100644 --- a/notify/notify_windows.go +++ b/notify/notify_windows.go @@ -24,7 +24,7 @@ func SetGlobalStatus(status chan<- svc.Status) { globalStatus = status } -func notifyReadiness() error { +func Ready() error { if globalStatus != nil { globalStatus <- svc.Status{ State: svc.Running, @@ -34,16 +34,22 @@ func notifyReadiness() error { return nil } -func notifyReloading() error { +func Reloading() error { if globalStatus != nil { globalStatus <- svc.Status{State: svc.StartPending} } return nil } -func notifyStopping() error { +func Stopping() error { if globalStatus != nil { globalStatus <- svc.Status{State: svc.StopPending} } return nil } + +// TODO: not implemented +func Status(_ string) error { return nil } + +// TODO: not implemented +func Error(_ error, _ int) error { return nil }