From 03601ca89b4c242d6dce9c1edd460ce0fe7f50ee Mon Sep 17 00:00:00 2001 From: tomnewton-mulga Date: Thu, 9 Apr 2026 15:40:26 +1000 Subject: [PATCH] fix: banner service ordering, getty wait, motd append (mulga-866) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - banner runs After=spinifex.target so services are settled before banner writes - switch systemctl restart → try-restart (safe if target not yet active) - append banner to /etc/motd with sentinel instead of overwriting, preserving the Debian disclaimer; sentinel ensures re-runs replace cleanly - remove /etc/issue write — getty shows it pre-login which is wrong - add getty@.service.d drop-in so all getty instances wait for banner before showing login prompt; prevents service messages scrolling during login - fix Makefile to prefer locally built spx/installer binaries over system PATH --- cmd/installer/firstboot/firstboot.go | 34 +++++++++++++++---- cmd/spinifex/cmd/admin.go | 49 +++++++++++++++++++++------- 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/cmd/installer/firstboot/firstboot.go b/cmd/installer/firstboot/firstboot.go index 09f0b7c60..87c7be5d6 100644 --- a/cmd/installer/firstboot/firstboot.go +++ b/cmd/installer/firstboot/firstboot.go @@ -51,7 +51,10 @@ func Write(root string, cfg Config) error { if err := writeBannerUnit(root); err != nil { return fmt.Errorf("banner unit: %w", err) } - return enableBannerUnit(root) + if err := enableBannerUnit(root); err != nil { + return err + } + return writeGettyDropIn(root) } func writeScript(root string, cfg Config) error { @@ -137,15 +140,14 @@ func buildClusterCmd(cfg Config) string { } // writeBannerUnit writes the spinifex-banner.service unit that runs -// `spx admin banner --boot-check` on every boot. This replaces the old -// spinifex-banner.sh bash script with the CLI command, which also handles -// management IP change detection. +// `spx admin banner --boot-check` on every boot after spinifex.target is up. +// Running After=spinifex.target ensures the banner reflects a settled system +// state and that the IP check/restart happens once services are already running. func writeBannerUnit(root string) error { unit := `[Unit] Description=Spinifex console banner and boot health check -After=network-online.target -Before=spinifex.target -Wants=network-online.target +After=spinifex.target +Wants=spinifex.target [Service] Type=oneshot @@ -171,3 +173,21 @@ func enableBannerUnit(root string) error { _ = os.Remove(link) return os.Symlink(target, link) } + +// writeGettyDropIn makes all getty instances (tty1, ttyS0, etc.) wait for +// spinifex-banner.service before showing the login prompt. This ensures: +// - All spinifex services have settled (banner runs After=spinifex.target) +// - The /etc/motd banner is written before the user sees the login prompt +// - Service startup messages finish scrolling before the prompt appears +func writeGettyDropIn(root string) error { + dropIn := `[Unit] +After=spinifex-banner.service +Wants=spinifex-banner.service +` + // Drop-in on getty@.service applies to all instances: tty1, ttyS0, etc. + dropInDir := filepath.Join(root, "etc/systemd/system/getty@.service.d") + if err := os.MkdirAll(dropInDir, 0o755); err != nil { + return err + } + return os.WriteFile(filepath.Join(dropInDir, "spinifex-wait.conf"), []byte(dropIn), 0o644) +} diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index d03e63ff6..3fe55d212 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -2100,8 +2100,10 @@ func installCACertificate(caPemPath string) { fmt.Printf("✅ CA certificate installed to system trust store\n") } -// runAdminBanner writes the Spinifex console banner to /etc/issue and /etc/motd. +// runAdminBanner writes the Spinifex console banner to /etc/motd. // With --boot-check it also detects management IP changes and updates node.conf. +// All errors are logged as warnings — the command always exits 0 so a banner +// failure never blocks the boot sequence. func runAdminBanner(cmd *cobra.Command, _ []string) { bootCheck, _ := cmd.Flags().GetBool("boot-check") @@ -2119,16 +2121,16 @@ func runAdminBanner(cmd *cobra.Command, _ []string) { } } - // Resolve current IP from the management interface. + // Resolve current IP from the management interface at runtime. currentIP := resolveIfaceIP(iface) if currentIP == "" { - currentIP = recordedIP // fall back to recorded value + currentIP = recordedIP // fall back to value recorded at install time } if currentIP == "" { currentIP = "" } - if bootCheck && iface != "" && currentIP != recordedIP && recordedIP != "" { + if bootCheck && iface != "" && recordedIP != "" && currentIP != recordedIP { slog.Info("Management IP changed", "old", recordedIP, "new", currentIP) conf["MANAGEMENT_IP"] = currentIP if err := writeNodeConf(nodeConf, conf); err != nil { @@ -2136,8 +2138,8 @@ func runAdminBanner(cmd *cobra.Command, _ []string) { } else { slog.Info("Updated node.conf with new management IP", "ip", currentIP) } - // Restart services that bind to the management IP. - restartCmd := exec.Command("systemctl", "restart", "spinifex.target") + // try-restart is safe even if the target isn't active yet. + restartCmd := exec.Command("systemctl", "try-restart", "spinifex.target") restartCmd.Stdout = os.Stdout restartCmd.Stderr = os.Stderr if err := restartCmd.Run(); err != nil { @@ -2164,16 +2166,39 @@ func runAdminBanner(cmd *cobra.Command, _ []string) { "root@"+currentIP, ) - // Write to /etc/issue (shown before login on physical/serial console). - if err := os.WriteFile("/etc/issue", []byte(banner), 0o644); err != nil { - slog.Warn("Failed to write /etc/issue", "err", err) - } - // Write to /etc/motd (shown after SSH login). - if err := os.WriteFile("/etc/motd", []byte(banner), 0o644); err != nil { + // Append banner to /etc/motd, preserving any existing content (e.g. the + // Debian "ABSOLUTELY NO WARRANTY" disclaimer). A sentinel marks the start + // of our section so re-runs replace it cleanly rather than accumulating. + if err := appendBannerToMotd(banner); err != nil { slog.Warn("Failed to write /etc/motd", "err", err) } } +// appendBannerToMotd appends the Spinifex banner to /etc/motd, preserving any +// existing content. A sentinel line marks the start of the Spinifex section so +// repeated runs replace only our section rather than accumulating duplicates. +func appendBannerToMotd(banner string) error { + const ( + motdPath = "/etc/motd" + sentinel = "# --- Spinifex ---\n" + ) + existing, _ := os.ReadFile(motdPath) + base := string(existing) + // Strip any previous Spinifex section from the sentinel onwards. + if idx := strings.Index(base, sentinel); idx >= 0 { + base = base[:idx] + } + // Ensure a blank line separates the existing content from our banner. + if len(strings.TrimSpace(base)) > 0 && !strings.HasSuffix(base, "\n\n") { + if strings.HasSuffix(base, "\n") { + base += "\n" + } else { + base += "\n\n" + } + } + return os.WriteFile(motdPath, []byte(base+sentinel+banner), 0o644) +} + // parseNodeConf reads a KEY=VALUE shell-format file and returns a map. // Lines starting with # and blank lines are ignored. func parseNodeConf(path string) map[string]string {