xrootd: fix new daemon being terminated immediately after restart on log-level change

[brianaydemir]

Closes #3156.
Addresses #3158.

Fix for #3156 — new daemon terminated immediately after restart

Running pelican server set-logging-level with an XRootD-specific parameter (e.g., Logging.Cache.PssSetOpt) triggers a full XRootD restart via the "xrootd-logging" param callback. The problem was in how handleXrootdLoggingChange created its restart context:

ctx, cancel := context.WithTimeout(context.Background(),
    param.Xrootd_ShutdownTimeout.GetDuration()+time.Minute)
defer cancel()
_, err := restartXrootdFn(ctx, nil)

This short-lived timeout context flowed all the way through RestartXrootd → LaunchDaemons → daemon.LaunchDaemons → DaemonLauncher.Launch, where it was passed to exec.CommandContext (and cap.Launcher.Launch on Linux). When handleXrootdLoggingChange returned and defer cancel() fired, the Go runtime sent SIGTERM to the freshly-started daemon. The result was a second signal to a different PID seconds after the restart appeared to succeed, leaving XRootD dead.

Fix: store the long-lived server context in restartInfo at launch time. In RestartXrootd, use info.ctx for LaunchDaemons so the new daemon is bound to the server lifetime. The short-lived timeout context is now used only for the shutdown/wait phase.

Fix for #3158 — no graceful drain before restart

Previously RestartXrootd sent SIGTERM to old processes immediately, cutting off in-flight client transfers without notice and without advertising the impending downtime to the Director.

The Web UI Restart button already handles this via handleGracefulShutdown:

1. Set health status to StatusShuttingDown
2. Advertise to the Director (stops new redirects to this server)
3. Sleep for Xrootd_ShutdownTimeout (drain period for ongoing transfers)

Fix: add a preRestartHook func(ctx context.Context) field to restartInfo. RestartXrootd calls each hook before sending any signals. The launchers package injects closures that call handleGracefulShutdown with the appropriate server — the same code path used by the Web UI Restart button. No new imports in the xrootd package are needed.

Fix for post-restart 404 — server not re-advertised to Director after restart

The preRestartHook explicitly tells the Director the server is shutting down (via launcher_utils.Advertise with StatusShuttingDown), which causes the Director to remove the server from its routing table. Without a corresponding re-advertisement after the restart, the Director remained unaware the server was back — causing clients to receive 404 errors until the next periodic advertisement cycle fired.

Fix: add a symmetric postRestartHook func(ctx context.Context) field to restartInfo. RestartXrootd calls each hook after metrics StatusOK is set and all new daemons are running. The launchers package injects closures that call launcher_utils.Advertise immediately, so the Director knows the server is available again before RestartXrootd returns.

Changes

• xrootd/restart.go — add ctx, preRestartHook, and postRestartHook fields to restartInfo;
  update StoreRestartInfo; use info.ctx for LaunchDaemons; call
  preRestartHook before SIGTERM loop; call postRestartHook after StatusOK is set
• xrootd/restart_windows.go — update stub signature
• launchers/cache_serve.go — pass ctx, preRestartHook, and postRestartHook to StoreRestartInfo
• launchers/origin_serve.go — same for origin
• xrootd/restart_test.go — update call sites
• fed_test_utils/fed.go — set Xrootd_ShutdownTimeout to 0 in tests to prevent 1-minute drain sleep

---

Analysis and implementation by GitHub Copilot (copilot-swe-agent).

Formatting fixes by brianaydemir.

[brianaydemir]

Things I learn: The GitHub Copilot CLI is happy to create a PR, but can't set itself as the author of the PR.

That said, I did look at all of the changes on this PR and go through the exercise of convincing myself that they're at least plausibly correct. (I make no claim to fully understanding "contexts" in Go….)

[brianaydemir]

Do we want the long-lived context from info, or the short-lived one that was passed to RestartXrootd?

[brianaydemir]

Do we want the long-lived context from info, or the short-lived one that was passed to RestartXrootd?

[brianaydemir]

This sounds plausible to me, but will this interfere with tests outside of launchers and xrootd?