Make service install converge: stop running service, refresh binary/config only on change, restart only if it was running

2026-09-21 08:30:55 +02:00
parent da02457fd5
commit 1394a76eea
4 changed files with 227 additions and 54 deletions
@@ -6,6 +6,7 @@
 package service

 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io"
@@ -126,6 +127,12 @@ func copyFile(src, dst string, mode os.FileMode) error {
 // copyBin=false the current executable location and config path are
 // registered as-is instead. Needs root.
 //
+// Re-running install converges an existing unit instead of failing: it is
+// stopped first if active, the installed binary is replaced only when the
+// content differs, the unit file is rewritten (followed by daemon-reload)
+// only when it changed, and the service is started again only if it was
+// active before.
+//
 // The binary lives in the StateDirectory rather than /usr/local/sbin on
 // purpose: replacing a running binary needs write access to its
 // *directory*, and granting the sandboxed service user write access to a
@@ -139,6 +146,16 @@ func Install(configPath string, copyBin bool) error {
 	if abs, absErr := filepath.Abs(exe); absErr == nil {
 		exe = abs
 	}
+	unit := Name + ".service"
+	_, statErr := os.Stat(unitPath)
+	fresh := os.IsNotExist(statErr)
+	wasRunning := exec.Command("systemctl", "is-active", "--quiet", unit).Run() == nil
+	if wasRunning {
+		if out, err := exec.Command("systemctl", "stop", unit).CombinedOutput(); err != nil {
+			return fmt.Errorf("systemctl stop (run as root): %w (%s)", err, out)
+		}
+	}
+
 	cfg := etcConfig
 	if copyBin {
 		if err := os.MkdirAll(stateDir, 0o755); err != nil {
@@ -146,8 +163,10 @@ func Install(configPath string, copyBin bool) error {
 		}
 		installedExe := filepath.Join(stateDir, Name)
 		if exe != installedExe {
-			if err := copyFile(exe, installedExe, 0o755); err != nil {
-				return fmt.Errorf("install binary to %s: %w", installedExe, err)
+			if same, _ := sameFileContent(exe, installedExe); !same {
+				if err := copyFile(exe, installedExe, 0o755); err != nil {
+					return fmt.Errorf("install binary to %s: %w", installedExe, err)
+				}
 			}
 		}
 		exe = installedExe
@@ -161,18 +180,51 @@ func Install(configPath string, copyBin bool) error {
 			cfg = abs
 		}
 	}
-	if err := os.WriteFile(unitPath, []byte(renderUnit(exe, cfg)), 0o644); err != nil {
-		return fmt.Errorf("write %s (run as root): %w", unitPath, err)
+	rendered := renderUnit(exe, cfg)
+	if old, _ := os.ReadFile(unitPath); string(old) != rendered {
+		if err := os.WriteFile(unitPath, []byte(rendered), 0o644); err != nil {
+			return fmt.Errorf("write %s (run as root): %w", unitPath, err)
+		}
+		if out, err := exec.Command("systemctl", "daemon-reload").CombinedOutput(); err != nil {
+			return fmt.Errorf("systemctl daemon-reload: %w (%s)", err, out)
+		}
 	}
-	if out, err := exec.Command("systemctl", "daemon-reload").CombinedOutput(); err != nil {
-		return fmt.Errorf("systemctl daemon-reload: %w (%s)", err, out)
+	if fresh {
+		if out, err := exec.Command("systemctl", "enable", "--now", unit).CombinedOutput(); err != nil {
+			return fmt.Errorf("systemctl enable --now: %w (%s)", err, out)
+		}
+		return nil
 	}
-	if out, err := exec.Command("systemctl", "enable", "--now", Name+".service").CombinedOutput(); err != nil {
-		return fmt.Errorf("systemctl enable --now: %w (%s)", err, out)
+	if exec.Command("systemctl", "is-enabled", "--quiet", unit).Run() != nil {
+		if out, err := exec.Command("systemctl", "enable", unit).CombinedOutput(); err != nil {
+			return fmt.Errorf("systemctl enable: %w (%s)", err, out)
+		}
+	}
+	if wasRunning {
+		if out, err := exec.Command("systemctl", "start", unit).CombinedOutput(); err != nil {
+			return fmt.Errorf("systemctl start: %w (%s)", err, out)
+		}
 	}
 	return nil
 }

+// sameFileContent reports whether two files hold identical bytes. A missing
+// destination is simply "different".
+func sameFileContent(a, b string) (bool, error) {
+	ba, err := os.ReadFile(a)
+	if err != nil {
+		return false, err
+	}
+	bb, err := os.ReadFile(b)
+	if os.IsNotExist(err) {
+		return false, nil
+	}
+	if err != nil {
+		return false, err
+	}
+	return bytes.Equal(ba, bb), nil
+}
+
 // Remove stops and disables the service and deletes the unit file and the
 // installed binary. The config file in /etc is left in place (user data).
 func Remove() error {
@@ -8,6 +8,7 @@
 package service

 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -109,6 +110,12 @@ func (h *handler) Execute(_ []string, requests <-chan svc.ChangeRequest, status
 // config file if it lives elsewhere. The grants must come after
 // CreateService: the virtual account's SID only exists once the service is
 // registered.
+//
+// Re-running install on an existing service converges instead of failing:
+// the service is stopped first if running (so the binary can be replaced),
+// the installed binary is refreshed only when the content differs, the
+// registration is updated only where it drifted, and the service is
+// started again only if it was running before.
 func Install(configPath string, copyBin bool) error {
 	exe, err := os.Executable()
 	if err != nil {
@@ -123,14 +130,38 @@ func Install(configPath string, copyBin bool) error {
 		}
 	}

+	m, err := mgr.Connect()
+	if err != nil {
+		return fmt.Errorf("connect to service manager (run as administrator): %w", err)
+	}
+	defer m.Disconnect()
+
+	// An existing service is converged, not an error. Stop it first so the
+	// binary copy can be replaced, and remember whether to start it again.
+	var s *mgr.Service
+	wasRunning := false
+	if existing, openErr := m.OpenService(Name); openErr == nil {
+		s = existing
+		defer s.Close()
+		if st, qErr := s.Query(); qErr == nil &&
+			(st.State == svc.Running || st.State == svc.StartPending) {
+			wasRunning = true
+			if err := stopAndWait(s); err != nil {
+				return err
+			}
+		}
+	}
+
 	installDir, _ := installDirs()
 	if copyBin && !strings.EqualFold(filepath.Dir(exe), installDir) {
 		if err := os.MkdirAll(installDir, 0o755); err != nil {
 			return fmt.Errorf("create %s: %w", installDir, err)
 		}
 		installedExe := filepath.Join(installDir, "gpu-turnstile.exe")
-		if err := copyFile(exe, installedExe); err != nil {
-			return fmt.Errorf("copy binary to %s: %w", installedExe, err)
+		if same, _ := sameFileContent(exe, installedExe); !same {
+			if err := copyFile(exe, installedExe); err != nil {
+				return fmt.Errorf("copy binary to %s: %w", installedExe, err)
+			}
 		}
 		exe = installedExe
 		targetCfg := filepath.Join(installDir, "gpu-turnstile.env")
@@ -141,44 +172,138 @@ func Install(configPath string, copyBin bool) error {
 			configPath = targetCfg
 		}
 	}
-
-	m, err := mgr.Connect()
-	if err != nil {
-		return fmt.Errorf("connect to service manager (run as administrator): %w", err)
-	}
-	defer m.Disconnect()
-
 	binPath := fmt.Sprintf(`"%s" -config "%s"`, exe, configPath)
-	s, err := m.CreateService(Name, binPath, mgr.Config{
-		StartType:        mgr.StartAutomatic,
-		DisplayName:      "gpu-turnstile",
-		Description:      "GPU arbitration proxy for Ollama and ComfyUI",
-		ServiceStartName: virtualAccount,
-	})
-	if err != nil {
-		return fmt.Errorf("create service: %w", err)
-	}
-	defer s.Close()

-	restart := mgr.RecoveryAction{Type: mgr.ServiceRestart, Delay: 5 * time.Second}
-	if err := s.SetRecoveryActions([]mgr.RecoveryAction{restart, restart, restart}, 24*60*60); err != nil {
+	if s == nil {
+		s, err = m.CreateService(Name, binPath, mgr.Config{
+			StartType:        mgr.StartAutomatic,
+			DisplayName:      "gpu-turnstile",
+			Description:      "GPU arbitration proxy for Ollama and ComfyUI",
+			ServiceStartName: virtualAccount,
+		})
+		if err != nil {
+			return fmt.Errorf("create service: %w", err)
+		}
+		defer s.Close()
+
+		if err := ensureRecovery(s); err != nil {
+			s.Delete() // roll back so a retry starts clean
+			return err
+		}
+		if err := grantAll(exe, configPath); err != nil {
+			s.Delete() // roll back so a retry starts clean
+			return err
+		}
+		// Best effort: start now instead of waiting for the next boot. A
+		// missing config (no consumer URLs) fails the start; the service stays
+		// registered and can be started once the config exists.
+		s.Start()
+		return nil
+	}
+
+	// Existing service: update the registration only where it drifted.
+	cur, err := s.Config()
+	if err != nil {
+		return fmt.Errorf("query service config: %w", err)
+	}
+	const displayName = "gpu-turnstile"
+	const description = "GPU arbitration proxy for Ollama and ComfyUI"
+	if cur.BinaryPathName != binPath ||
+		cur.StartType != mgr.StartAutomatic ||
+		cur.ServiceStartName != virtualAccount ||
+		cur.DisplayName != displayName ||
+		cur.Description != description {
+		upd := cur
+		upd.BinaryPathName = binPath
+		upd.StartType = mgr.StartAutomatic
+		upd.ServiceStartName = virtualAccount
+		upd.DisplayName = displayName
+		upd.Description = description
+		if err := s.UpdateConfig(upd); err != nil {
+			return fmt.Errorf("update service config: %w", err)
+		}
+	}
+	if err := ensureRecovery(s); err != nil {
+		return err
+	}
+	// Idempotent: re-assert the virtual account's ACLs (granting an
+	// existing ACE is a no-op).
+	if err := grantAll(exe, configPath); err != nil {
+		return err
+	}
+	if wasRunning {
+		if err := s.Start(); err != nil {
+			return fmt.Errorf("start service: %w", err)
+		}
+	}
+	return nil
+}
+
+// ensureRecovery makes sure the service restarts 5s after a failure — the
+// mechanism that also brings up a staged update. The current actions are
+// queried first so an up-to-date service is left untouched.
+func ensureRecovery(s *mgr.Service) error {
+	want := mgr.RecoveryAction{Type: mgr.ServiceRestart, Delay: 5 * time.Second}
+	actions, err := s.RecoveryActions()
+	onFailure, flagErr := s.RecoveryActionsOnNonCrashFailures()
+	if err == nil && flagErr == nil && onFailure && len(actions) == 3 {
+		ok := true
+		for _, a := range actions {
+			if a.Type != want.Type || a.Delay != want.Delay {
+				ok = false
+			}
+		}
+		if ok {
+			return nil
+		}
+	}
+	if err := s.SetRecoveryActions([]mgr.RecoveryAction{want, want, want}, 24*60*60); err != nil {
 		return fmt.Errorf("set recovery actions: %w", err)
 	}
 	if err := s.SetRecoveryActionsOnNonCrashFailures(true); err != nil {
 		return fmt.Errorf("set failure actions flag: %w", err)
 	}
-
-	if err := grantAll(exe, configPath); err != nil {
-		s.Delete() // roll back so a retry starts clean
-		return err
-	}
-	// Best effort: start now instead of waiting for the next boot. A
-	// missing config (no consumer URLs) fails the start; the service stays
-	// registered and can be started once the config exists.
-	s.Start()
 	return nil
 }

+// stopAndWait stops the service and waits up to 30s for the stopped state.
+func stopAndWait(s *mgr.Service) error {
+	if _, err := s.Control(svc.Stop); err != nil {
+		return fmt.Errorf("stop service: %w", err)
+	}
+	deadline := time.Now().Add(30 * time.Second)
+	for {
+		st, err := s.Query()
+		if err != nil {
+			return fmt.Errorf("query service: %w", err)
+		}
+		if st.State == svc.Stopped {
+			return nil
+		}
+		if time.Now().After(deadline) {
+			return fmt.Errorf("service did not stop within 30s")
+		}
+		time.Sleep(300 * time.Millisecond)
+	}
+}
+
+// sameFileContent reports whether two files hold identical bytes. A missing
+// destination is simply "different".
+func sameFileContent(a, b string) (bool, error) {
+	ba, err := os.ReadFile(a)
+	if err != nil {
+		return false, err
+	}
+	bb, err := os.ReadFile(b)
+	if os.IsNotExist(err) {
+		return false, nil
+	}
+	if err != nil {
+		return false, err
+	}
+	return bytes.Equal(ba, bb), nil
+}
+
 // copyFile copies src to dst (0755 on the new file).
 func copyFile(src, dst string) error {
 	in, err := os.Open(src)
@@ -374,22 +499,8 @@ func RestartIfRunning() (bool, error) {
 	if st.State != svc.Running && st.State != svc.StartPending {
 		return false, nil
 	}
-	if _, err := s.Control(svc.Stop); err != nil {
-		return false, fmt.Errorf("stop service: %w", err)
-	}
-	deadline := time.Now().Add(30 * time.Second)
-	for {
-		st, err = s.Query()
-		if err != nil {
-			return false, fmt.Errorf("query service: %w", err)
-		}
-		if st.State == svc.Stopped {
-			break
-		}
-		if time.Now().After(deadline) {
-			return false, fmt.Errorf("service did not stop within 30s")
-		}
-		time.Sleep(300 * time.Millisecond)
+	if err := stopAndWait(s); err != nil {
+		return false, err
 	}
 	if err := s.Start(); err != nil {
 		return false, fmt.Errorf("start service: %w", err)