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

@@ -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
 }

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()
 	}

notify/notify.go

@@ -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()
-}

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 {
+	_, err = conn.Write([]byte(payload))
 	return err
 }
-	return nil
-}
 
-// 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
+// Stopping notifies systemd that caddy is stopping.
+func Stopping() error {
+	return sdNotify("STOPPING=1")
 }
-	if err := sdNotify(val, "STOPPING=1"); err != nil {
-		return err
+
+// Status sends systemd an updated status message.
+func Status(msg string) error {
+	return sdNotify("STATUS=" + msg)
 }
-	return nil
+
+// 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")

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 }

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 }