diff --git a/README.md b/README.md index a41ae8c..65222b2 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,10 @@ as the virtual account `NT SERVICE\gpu-turnstile` (low-privilege, per-service, no password); the installer automatically grants it write access to the install and data directories — nothing else to do. +Re-running `--install-service` is safe: it stops a running service, +replaces the installed binary only if it changed, fixes the registration +only where it drifted, and restarts the service only if it was running. + ### Run natively on Linux (systemd) The same binary works on Linux. Install it as a systemd service as root: diff --git a/SPEC.md b/SPEC.md index 38bb387..c57f8e4 100644 --- a/SPEC.md +++ b/SPEC.md @@ -176,6 +176,12 @@ prompt instead of failing — the command relaunches itself elevated, waits for the child, and mirrors its exit code. The legacy form `gpu-turnstile service install|remove` does the same thing. +Re-running install on an already-registered service converges instead of +failing: a running service is stopped first, the installed binary copy is +refreshed only when the content differs, the registration (Windows service +config / systemd unit) is updated only where it drifted, and the service is +started again only if it was running before. + By default install creates the canonical layout and copies the binary into it (Windows: `%ProgramFiles%\gpu-turnstile\`, plus `%ProgramData%\gpu-turnstile\` for logs; Linux: `/var/lib/gpu-turnstile/` diff --git a/internal/service/service_linux.go b/internal/service/service_linux.go index c175525..b1cb644 100644 --- a/internal/service/service_linux.go +++ b/internal/service/service_linux.go @@ -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 { diff --git a/internal/service/service_windows.go b/internal/service/service_windows.go index a61f7d2..6da86b9 100644 --- a/internal/service/service_windows.go +++ b/internal/service/service_windows.go @@ -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)