fix: exit non-zero when a spinifex service fails to start (#791)

* fix: exit non-zero when a spinifex service fails to start

Every service start/stop subcommand in cmd/spinifex/cmd/service.go used
Run funcs that printed an error and returned, which exits 0. Systemd's
Restart=on-failure never fires on a zero exit, so a fatal startup error
(seen live: viperblockd's volume lease bucket lookup timing out during a
cold JetStream start) left the node permanently down until a manual
systemctl restart.

Converts every fatal error path across predastore, viperblock, qemunbd,
nats, spinifex, awsgw, spinifex-ui, vpcd, northstar and qmp-collector to
RunE returning an error, following the RunE + SilenceErrors/SilenceUsage
pattern the northstar start command already used in this file. root.go's
Execute() already prints the returned error to stderr and calls
os.Exit(1), so this centralizes the exit-code decision instead of
repeating fmt.Fprintln+os.Exit at each call site. Pure status subcommands
with no fallible operations are left as Run.

Adds a subprocess re-exec test proving a fatal predastore start now
exits 1 end to end, plus in-process RunE tests covering each service's
start command.

* test: cover the service startup validation branches

cmd/spinifex/cmd/service.go

@@ -130,9 +130,11 @@ func derivePredastoreBind(clusterConfig *config.ClusterConfig) (predastoreBind,
 }
 
 var predastoreStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the predastore service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the predastore service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		// Add your start logic here
 		fmt.Println("Starting predastore service...")
 
@@ -148,13 +150,11 @@ var predastoreStartCmd = &cobra.Command{
 		if cfgFile := viper.GetString("config"); cfgFile != "" {
 			clusterConfig, err := config.LoadConfig(cfgFile)
 			if err != nil {
-				fmt.Println("Error loading cluster config file:", err)
-				return
+				return fmt.Errorf("load cluster config file: %w", err)
 			}
 			bind, err := derivePredastoreBind(clusterConfig)
 			if err != nil {
-				fmt.Println("Error deriving predastore bind config:", err)
-				return
+				return fmt.Errorf("derive predastore bind config: %w", err)
 			}
 			if !viper.IsSet("predastore-host") {
 				host = bind.Host
@@ -170,36 +170,31 @@ var predastoreStartCmd = &cobra.Command{
 		// Required, no default: the host id selects the [[host]] of the
 		// predastore topology this process runs, and nothing can guess it.
 		if hostID <= 0 {
-			fmt.Println("Host ID is not set (--host-id, SPINIFEX_PREDASTORE_HOST_ID or host_id in spinifex.toml)")
-			return
+			return fmt.Errorf("host id is not set (--host-id, SPINIFEX_PREDASTORE_HOST_ID or host_id in spinifex.toml)")
 		}
 
 		configPath := viper.GetString("predastore-config-path")
 
 		if configPath == "" {
-			fmt.Println("Config path is not set")
-			return
+			return fmt.Errorf("config path is not set")
 		}
 
 		tlsCert := viper.GetString("predastore-tls-cert")
 
 		if tlsCert == "" {
-			fmt.Println("TLS cert is not set")
-			return
+			return fmt.Errorf("TLS cert is not set")
 		}
 
 		tlsKey := viper.GetString("predastore-tls-key")
 
 		if tlsKey == "" {
-			fmt.Println("TLS key is not set")
-			return
+			return fmt.Errorf("TLS key is not set")
 		}
 
 		encryptionKeyFile := viper.GetString("predastore-encryption-key-file")
 
 		if encryptionKeyFile == "" {
-			fmt.Println("Encryption key file is not set")
-			return
+			return fmt.Errorf("encryption key file is not set")
 		}
 
 		defer initTelemetry("predastore", false)()
@@ -218,23 +213,24 @@ var predastoreStartCmd = &cobra.Command{
 		})
 
 		if err != nil {
-			fmt.Println("Error starting predastore service:", err)
-			return
+			return fmt.Errorf("create predastore service: %w", err)
 		}
 
 		if _, err := service.Start(); err != nil {
-			fmt.Println("Error starting predastore service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start predastore service: %w", err)
 		}
 
 		fmt.Println("Predastore service started", service)
+		return nil
 	},
 }
 
 var predastoreStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the predastore service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the predastore service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping predastore service...")
 
 		// Only the pid directory matters here: Stop finds the running process
@@ -244,17 +240,15 @@ var predastoreStopCmd = &cobra.Command{
 		})
 
 		if err != nil {
-			fmt.Println("Error stopping predastore service:", err)
-			return
+			return fmt.Errorf("create predastore service: %w", err)
 		}
 
 		if err = service.Stop(); err != nil {
-			fmt.Println("Error stopping predastore service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop predastore service: %w", err)
 		}
 
 		fmt.Println("Predastore service stopped")
-
+		return nil
 	},
 }
 
@@ -269,16 +263,17 @@ var predastoreStatusCmd = &cobra.Command{
 
 // Repeat for viperblock.
 var viperblockStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the viperblock service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the viperblock service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting viperblock service...")
 
 		cfgFile := viper.GetString("config")
 
 		if cfgFile == "" {
-			fmt.Println("Config file is not set")
-			return
+			return fmt.Errorf("config file is not set")
 		}
 
 		fmt.Println("Loading config from:", cfgFile)
@@ -286,8 +281,7 @@ var viperblockStartCmd = &cobra.Command{
 		// TODO: Support ENV vars, CLI, otherwise revert to config.LoadConfig()
 		clusterConfig, err := config.LoadConfig(cfgFile)
 		if err != nil {
-			fmt.Println("Error loading config file:", err)
-			return
+			return fmt.Errorf("load config file: %w", err)
 		}
 		nodeConfig := clusterConfig.Nodes[clusterConfig.Node]
 
@@ -345,14 +339,14 @@ var viperblockStartCmd = &cobra.Command{
 		if pluginPath == "" {
 			err := fmt.Errorf("plugin-path must be defined")
 			slog.Error(err.Error())
-			os.Exit(1)
+			return err
 		}
 
 		// Check plugin path exists
 		if _, err := os.Stat(pluginPath); os.IsNotExist(err) {
 			err := fmt.Errorf("plugin-path does not exist: %s", pluginPath)
 			slog.Error(err.Error())
-			os.Exit(1)
+			return err
 		}
 
 		// Resolve sharded WAL setting: default false unless explicitly set to true
@@ -397,41 +391,40 @@ var viperblockStartCmd = &cobra.Command{
 		})
 
 		if err != nil {
-			fmt.Println("Error starting viperblock service:", err)
-			return
+			return fmt.Errorf("create viperblock service: %w", err)
 		}
 
 		_, err = service.Start()
 
 		if err != nil {
-			fmt.Println("Error starting viperblock service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start viperblock service: %w", err)
 		}
 
 		fmt.Println("Viperblock service started")
+		return nil
 	},
 }
 
 var viperblockStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the viperblock service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the viperblock service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping viperblock service...")
 
 		service, err := service.New("viperblock", &viperblockd.Config{})
 
 		if err != nil {
-			fmt.Println("Error stopping viperblock service:", err)
-			return
+			return fmt.Errorf("create viperblock service: %w", err)
 		}
 
 		if err = service.Stop(); err != nil {
-			fmt.Println("Error stopping viperblock service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop viperblock service: %w", err)
 		}
 
 		fmt.Println("Viperblock service stopped")
-
+		return nil
 	},
 }
 
@@ -452,24 +445,24 @@ var qemunbdCmd = &cobra.Command{
 // viperblock, so the two must never run against the same NATS cluster
 // together.
 var qemunbdStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the qemunbd service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the qemunbd service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting qemunbd service...")
 
 		cfgFile := viper.GetString("config")
 
 		if cfgFile == "" {
-			fmt.Println("Config file is not set")
-			return
+			return fmt.Errorf("config file is not set")
 		}
 
 		fmt.Println("Loading config from:", cfgFile)
 
 		clusterConfig, err := config.LoadConfig(cfgFile)
 		if err != nil {
-			fmt.Println("Error loading config file:", err)
-			return
+			return fmt.Errorf("load config file: %w", err)
 		}
 		nodeConfig := clusterConfig.Nodes[clusterConfig.Node]
 
@@ -503,40 +496,40 @@ var qemunbdStartCmd = &cobra.Command{
 		})
 
 		if err != nil {
-			fmt.Println("Error starting qemunbd service:", err)
-			return
+			return fmt.Errorf("create qemunbd service: %w", err)
 		}
 
 		_, err = service.Start()
 
 		if err != nil {
-			fmt.Println("Error starting qemunbd service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start qemunbd service: %w", err)
 		}
 
 		fmt.Println("qemunbd service started")
+		return nil
 	},
 }
 
 var qemunbdStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the qemunbd service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the qemunbd service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping qemunbd service...")
 
 		service, err := service.New("qemunbd", &qemunbdd.Config{})
 
 		if err != nil {
-			fmt.Println("Error stopping qemunbd service:", err)
-			return
+			return fmt.Errorf("create qemunbd service: %w", err)
 		}
 
 		if err = service.Stop(); err != nil {
-			fmt.Println("Error stopping qemunbd service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop qemunbd service: %w", err)
 		}
 
 		fmt.Println("qemunbd service stopped")
+		return nil
 	},
 }
 
@@ -550,9 +543,11 @@ var qemunbdStatusCmd = &cobra.Command{
 
 // Repeat for nats.
 var natsStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the nats service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the nats service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting nats service...")
 
 		port := viper.GetInt("nats-port")
@@ -575,37 +570,37 @@ var natsStartCmd = &cobra.Command{
 		})
 
 		if err != nil {
-			fmt.Println("Error starting nats service:", err)
-			return
+			return fmt.Errorf("create nats service: %w", err)
 		}
 
 		if _, err = service.Start(); err != nil {
-			fmt.Println("Error starting nats service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start nats service: %w", err)
 		}
 		fmt.Println("NATS service started")
+		return nil
 	},
 }
 
 var natsStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the nats service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the nats service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping nats service...")
 
 		service, err := service.New("nats", &nats.Config{})
 
 		if err != nil {
-			fmt.Println("Error stopping nats service:", err)
-			return
+			return fmt.Errorf("create nats service: %w", err)
 		}
 
 		if err = service.Stop(); err != nil {
-			fmt.Println("Error stopping nats service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop nats service: %w", err)
 		}
 
 		fmt.Println("Nats service stopped")
+		return nil
 	},
 }
 
@@ -619,23 +614,23 @@ var natsStatusCmd = &cobra.Command{
 
 // Repeat for spinifex.
 var spinifexStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the spinifex service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the spinifex service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting spinifex service...")
 
 		cfgFile := viper.GetString("config")
 
 		if cfgFile == "" {
-			fmt.Println("Config file is not set")
-			return
+			return fmt.Errorf("config file is not set")
 		}
 
 		// TODO: Support ENV vars, CLI, otherwise revert to config.LoadConfig()
 		clusterConfig, err := config.LoadConfig(cfgFile)
 		if err != nil {
-			fmt.Println("Error loading config file:", err)
-			return
+			return fmt.Errorf("load config file: %w", err)
 		}
 		nodeConfig := clusterConfig.Nodes[clusterConfig.Node]
 
@@ -663,8 +658,7 @@ var spinifexStartCmd = &cobra.Command{
 		svc, err := service.New("spinifex", clusterConfig)
 
 		if err != nil {
-			fmt.Println("Error starting spinifex service:", err)
-			return
+			return fmt.Errorf("create spinifex service: %w", err)
 		}
 
 		// Set config path for cluster manager
@@ -673,32 +667,33 @@ var spinifexStartCmd = &cobra.Command{
 		}
 
 		if _, err = svc.Start(); err != nil {
-			fmt.Println("Error starting spinifex service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start spinifex service: %w", err)
 		}
 		fmt.Println("Spinifex service started")
+		return nil
 	},
 }
 
 var spinifexStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the spinifex service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the spinifex service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping spinifex service...")
 
 		service, err := service.New("spinifex", &config.ClusterConfig{})
 
 		if err != nil {
-			fmt.Println("Error stopping spinifex service:", err)
-			return
+			return fmt.Errorf("create spinifex service: %w", err)
 		}
 
 		if err = service.Stop(); err != nil {
-			fmt.Println("Error stopping spinifex service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop spinifex service: %w", err)
 		}
 
 		fmt.Println("Spinifex service stopped")
+		return nil
 	},
 }
 
@@ -713,16 +708,17 @@ var spinifexStatusCmd = &cobra.Command{
 // AWS GW
 
 var awsgwStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the awsgw service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the awsgw service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting awsgw service...")
 
 		cfgFile := viper.GetString("config")
 
 		if cfgFile == "" {
-			fmt.Println("Config file is not set")
-			return
+			return fmt.Errorf("config file is not set")
 		}
 
 		fmt.Println("Loading config from:", cfgFile)
@@ -730,8 +726,7 @@ var awsgwStartCmd = &cobra.Command{
 		// TODO: Support ENV vars, CLI, otherwise revert to config.LoadConfig()
 		clusterConfig, err := config.LoadConfig(cfgFile)
 		if err != nil {
-			fmt.Println("Error loading config file:", err)
-			return
+			return fmt.Errorf("load config file: %w", err)
 		}
 		nodeConfig := clusterConfig.Nodes[clusterConfig.Node]
 
@@ -771,37 +766,37 @@ var awsgwStartCmd = &cobra.Command{
 		service, err := service.New("awsgw", clusterConfig)
 
 		if err != nil {
-			fmt.Println("Error starting awsgw service:", err)
-			return
+			return fmt.Errorf("create awsgw service: %w", err)
 		}
 
 		if _, err = service.Start(); err != nil {
-			fmt.Println("Error starting awsgw service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start awsgw service: %w", err)
 		}
 		fmt.Println("AWSGW service started")
+		return nil
 	},
 }
 
 var awsgwStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the awsgw service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the awsgw service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping awsgw service...")
 
 		service, err := service.New("awsgw", &config.ClusterConfig{})
 
 		if err != nil {
-			fmt.Println("Error stopping awsgw service:", err)
-			return
+			return fmt.Errorf("create awsgw service: %w", err)
 		}
 
 		if err = service.Stop(); err != nil {
-			fmt.Println("Error stopping awsgw service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop awsgw service: %w", err)
 		}
 
 		fmt.Println("AWSGW service stopped")
+		return nil
 	},
 }
 
@@ -814,9 +809,11 @@ var awsgwStatusCmd = &cobra.Command{
 }
 
 var spinifexUIStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the spinifex-ui service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the spinifex-ui service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting spinifex-ui service...")
 
 		port := viper.GetInt("spinifex-ui-port")
@@ -836,57 +833,58 @@ var spinifexUIStartCmd = &cobra.Command{
 		})
 
 		if err != nil {
-			fmt.Println("Error starting spinifex-ui service:", err)
-			return
+			return fmt.Errorf("create spinifex-ui service: %w", err)
 		}
 
 		if _, err = svc.Start(); err != nil {
-			fmt.Println("Error starting spinifex-ui service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start spinifex-ui service: %w", err)
 		}
 		fmt.Println("spinifex-ui service started")
+		return nil
 	},
 }
 
 var spinifexUIStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the spinifex-ui service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the spinifex-ui service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping spinifex-ui service...")
 
 		svc, err := service.New("spinifex-ui", &spinifexui.Config{})
 
 		if err != nil {
-			fmt.Println("Error stopping spinifex-ui service:", err)
-			return
+			return fmt.Errorf("create spinifex-ui service: %w", err)
 		}
 
 		if err = svc.Stop(); err != nil {
-			fmt.Println("Error stopping spinifex-ui service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop spinifex-ui service: %w", err)
 		}
 		fmt.Println("spinifex-ui service stopped")
+		return nil
 	},
 }
 
 var spinifexUIStatusCmd = &cobra.Command{
-	Use:   "status",
-	Short: "Get status of the spinifex-ui service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "status",
+	Short:         "Get status of the spinifex-ui service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		svc, err := service.New("spinifex-ui", &spinifexui.Config{})
 
 		if err != nil {
-			fmt.Println("Error getting spinifex-ui service status:", err)
-			return
+			return fmt.Errorf("create spinifex-ui service: %w", err)
 		}
 
 		status, err := svc.Status()
 		if err != nil {
-			fmt.Println("Error getting spinifex-ui service status:", err)
-			return
+			return fmt.Errorf("get spinifex-ui service status: %w", err)
 		}
 
 		fmt.Println("spinifex-ui service status:", status)
+		return nil
 	},
 }
 
@@ -912,26 +910,26 @@ func checkLegacyWanBridgeKey(node, cfgFile string) error {
 }
 
 var vpcdStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the vpcd service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the vpcd service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting vpcd service...")
 
 		cfgFile := viper.GetString("config")
 		if cfgFile == "" {
-			fmt.Println("Config file is not set")
-			return
+			return fmt.Errorf("config file is not set")
 		}
 
 		clusterConfig, err := config.LoadConfig(cfgFile)
 		if err != nil {
-			fmt.Println("Error loading config file:", err)
-			return
+			return fmt.Errorf("load config file: %w", err)
 		}
 
 		if err := checkLegacyWanBridgeKey(clusterConfig.Node, cfgFile); err != nil {
 			slog.Error(err.Error())
-			os.Exit(1)
+			return err
 		}
 
 		nodeConfig := clusterConfig.Nodes[clusterConfig.Node]
@@ -998,35 +996,35 @@ var vpcdStartCmd = &cobra.Command{
 			NATExemptCIDRs:          clusterConfig.Network.NATExemptCIDRs,
 		})
 		if err != nil {
-			fmt.Println("Error starting vpcd service:", err)
-			return
+			return fmt.Errorf("create vpcd service: %w", err)
 		}
 
 		if _, err = svc.Start(); err != nil {
-			fmt.Println("Error starting vpcd service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start vpcd service: %w", err)
 		}
 		fmt.Println("vpcd service started")
+		return nil
 	},
 }
 
 var vpcdStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the vpcd service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the vpcd service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping vpcd service...")
 
 		svc, err := service.New("vpcd", &vpcd.Config{})
 		if err != nil {
-			fmt.Println("Error stopping vpcd service:", err)
-			return
+			return fmt.Errorf("create vpcd service: %w", err)
 		}
 
 		if err = svc.Stop(); err != nil {
-			fmt.Println("Error stopping vpcd service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop vpcd service: %w", err)
 		}
 		fmt.Println("vpcd service stopped")
+		return nil
 	},
 }
 
@@ -1161,22 +1159,23 @@ func resolveNorthstarConfigPath(nodePath, override string) string {
 }
 
 var northstarStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the northstar service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the northstar service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping northstar service...")
 
 		svc, err := service.New("northstar", &northstar.Config{})
 		if err != nil {
-			fmt.Println("Error stopping northstar service:", err)
-			return
+			return fmt.Errorf("create northstar service: %w", err)
 		}
 
 		if err = svc.Stop(); err != nil {
-			fmt.Println("Error stopping northstar service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop northstar service: %w", err)
 		}
 		fmt.Println("northstar service stopped")
+		return nil
 	},
 }
 
@@ -1194,20 +1193,20 @@ var qmpCollectorCmd = &cobra.Command{
 }
 
 var qmpCollectorStartCmd = &cobra.Command{
-	Use:   "start",
-	Short: "Start the qmp-collector service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "start",
+	Short:         "Start the qmp-collector service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Starting qmp-collector service...")
 
 		cfgFile := viper.GetString("config")
 		if cfgFile == "" {
-			fmt.Println("Config file is not set")
-			return
+			return fmt.Errorf("config file is not set")
 		}
 		clusterConfig, err := config.LoadConfig(cfgFile)
 		if err != nil {
-			fmt.Println("Error loading config file:", err)
-			return
+			return fmt.Errorf("load config file: %w", err)
 		}
 		nodeConfig := clusterConfig.Nodes[clusterConfig.Node]
 
@@ -1221,33 +1220,33 @@ var qmpCollectorStartCmd = &cobra.Command{
 			NodeName:   clusterConfig.Node,
 		})
 		if err != nil {
-			fmt.Println("Error starting qmp-collector service:", err)
-			return
+			return fmt.Errorf("create qmp-collector service: %w", err)
 		}
 		if _, err = svc.Start(); err != nil {
-			fmt.Println("Error starting qmp-collector service:", err)
-			os.Exit(1)
+			return fmt.Errorf("start qmp-collector service: %w", err)
 		}
 		fmt.Println("qmp-collector service started")
+		return nil
 	},
 }
 
 var qmpCollectorStopCmd = &cobra.Command{
-	Use:   "stop",
-	Short: "Stop the qmp-collector service",
-	Run: func(cmd *cobra.Command, args []string) {
+	Use:           "stop",
+	Short:         "Stop the qmp-collector service",
+	SilenceErrors: true,
+	SilenceUsage:  true,
+	RunE: func(cmd *cobra.Command, args []string) error {
 		fmt.Println("Stopping qmp-collector service...")
 
 		svc, err := service.New("qmp-collector", &qmpcollector.Config{})
 		if err != nil {
-			fmt.Println("Error stopping qmp-collector service:", err)
-			return
+			return fmt.Errorf("create qmp-collector service: %w", err)
 		}
 		if err = svc.Stop(); err != nil {
-			fmt.Println("Error stopping qmp-collector service:", err)
-			os.Exit(1)
+			return fmt.Errorf("stop qmp-collector service: %w", err)
 		}
 		fmt.Println("qmp-collector service stopped")
+		return nil
 	},
 }
 

cmd/spinifex/cmd/service_exit_test.go

@@ -0,0 +1,480 @@
+package cmd
+
+// Drives the unexported *cobra.Command vars (predastoreStartCmd, etc.)
+// directly via RunE, so it must stay in-package.
+//test:in-package
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// blockedDir returns a path whose parent component is a regular file, so
+// os.MkdirAll fails with ENOTDIR. Start() calls WritePidFileTo before any
+// real infra, so this forces a fast, local, in-process failure there.
+func blockedDir(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	blocker := filepath.Join(dir, "blocker")
+	require.NoError(t, os.WriteFile(blocker, []byte("x"), 0o600))
+	return filepath.Join(blocker, "sub")
+}
+
+// minimalClusterToml satisfies config.LoadConfig's validation with nothing
+// service-specific set, so each case below only has to layer on the one
+// field it needs to reach its target branch.
+const minimalClusterToml = `
+version = "1.0"
+epoch = 1
+node = "node1"
+
+[nodes.node1]
+node = "node1"
+region = "us-east-1"
+az = "us-east-1a"
+`
+
+// writeMalformedToml writes a syntactically invalid TOML file so
+// config.LoadConfig fails at viper.ReadInConfig rather than at validation.
+func writeMalformedToml(t *testing.T) string {
+	t.Helper()
+	return writeSpinifexToml(t, "[section\nkey = value without quotes\n")
+}
+
+// isolateRuntimeDir redirects the pid-file fallback (utils.pidPath, via
+// XDG_RUNTIME_DIR) to an empty temp dir, so a stop command with no explicit
+// directory override can't resolve against a real, shared, ambient pid dir.
+func isolateRuntimeDir(t *testing.T) {
+	t.Helper()
+	t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
+}
+
+// TestServiceStartCmdsReturnErrorOnMissingConfig exercises each service
+// start/stop command's RunE directly and asserts a fatal precondition
+// failure surfaces as a returned error, not a silent zero-exit return.
+// Cases with a setup func drive further into RunE toward a specific branch.
+func TestServiceStartCmdsReturnErrorOnMissingConfig(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     *cobra.Command
+		setup   func(t *testing.T)
+		wantErr string
+	}{
+		{name: "predastore start (missing host-id)", cmd: predastoreStartCmd},
+		{
+			name: "predastore start (config file load failure)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load cluster config file",
+		},
+		{
+			name: "predastore start (derive bind failure)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+			},
+			wantErr: "derive predastore bind config",
+		},
+		{
+			name: "predastore start (config path not set)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("predastore-host-id", 1)
+			},
+			wantErr: "config path is not set",
+		},
+		{
+			name: "predastore start (tls cert not set)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("predastore-host-id", 1)
+				viper.Set("predastore-config-path", filepath.Join(t.TempDir(), "predastore.toml"))
+			},
+			wantErr: "TLS cert is not set",
+		},
+		{
+			name: "predastore start (tls key not set)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("predastore-host-id", 1)
+				viper.Set("predastore-config-path", filepath.Join(t.TempDir(), "predastore.toml"))
+				viper.Set("predastore-tls-cert", filepath.Join(t.TempDir(), "cert.pem"))
+			},
+			wantErr: "TLS key is not set",
+		},
+		{
+			name: "predastore start (encryption key file not set)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("predastore-host-id", 1)
+				viper.Set("predastore-config-path", filepath.Join(t.TempDir(), "predastore.toml"))
+				viper.Set("predastore-tls-cert", filepath.Join(t.TempDir(), "cert.pem"))
+				viper.Set("predastore-tls-key", filepath.Join(t.TempDir(), "key.pem"))
+			},
+			wantErr: "encryption key file is not set",
+		},
+		{
+			name: "predastore start (start failure via unreadable predastore config)",
+			cmd:  predastoreStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("predastore-host-id", 1)
+				viper.Set("predastore-config-path", filepath.Join(t.TempDir(), "missing-predastore.toml"))
+				viper.Set("predastore-tls-cert", filepath.Join(t.TempDir(), "cert.pem"))
+				viper.Set("predastore-tls-key", filepath.Join(t.TempDir(), "key.pem"))
+				viper.Set("predastore-encryption-key-file", filepath.Join(t.TempDir(), "enc.key"))
+				viper.Set("predastore-base-path", t.TempDir())
+			},
+			wantErr: "start predastore service",
+		},
+		{name: "viperblock start (missing config)", cmd: viperblockStartCmd},
+		{
+			name: "viperblock start (config file load failure)",
+			cmd:  viperblockStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load config file",
+		},
+		{
+			name: "viperblock start (plugin-path not set)",
+			cmd:  viperblockStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+			},
+			wantErr: "plugin-path must be defined",
+		},
+		{
+			name: "viperblock start (plugin-path does not exist)",
+			cmd:  viperblockStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				viper.Set("plugin-path", filepath.Join(t.TempDir(), "missing-plugin.so"))
+			},
+			wantErr: "plugin-path does not exist",
+		},
+		{
+			name: "viperblock start (start failure via blocked base-dir)",
+			cmd:  viperblockStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				viper.Set("plugin-path", t.TempDir())
+				viper.Set("base-dir", blockedDir(t))
+			},
+			wantErr: "start viperblock service",
+		},
+		{name: "qemunbd start (missing config)", cmd: qemunbdStartCmd},
+		{
+			name: "qemunbd start (config file load failure)",
+			cmd:  qemunbdStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load config file",
+		},
+		{
+			name: "qemunbd start (start failure via blocked base-dir)",
+			cmd:  qemunbdStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				viper.Set("base-dir", blockedDir(t))
+			},
+			wantErr: "start qemunbd service",
+		},
+		{
+			name: "nats start (start failure via blocked data-dir)",
+			cmd:  natsStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("data-dir", blockedDir(t))
+			},
+			wantErr: "start nats service",
+		},
+		{name: "spinifex start (missing config)", cmd: spinifexStartCmd},
+		{
+			name: "spinifex start (config file load failure)",
+			cmd:  spinifexStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load config file",
+		},
+		{
+			name: "spinifex start (start failure via blocked base-dir)",
+			cmd:  spinifexStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				viper.Set("base-dir", blockedDir(t))
+			},
+			wantErr: "start spinifex service",
+		},
+		{name: "awsgw start (missing config)", cmd: awsgwStartCmd},
+		{
+			name: "awsgw start (config file load failure)",
+			cmd:  awsgwStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load config file",
+		},
+		{
+			name: "awsgw start (start failure via blocked base-dir)",
+			cmd:  awsgwStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				viper.Set("base-dir", blockedDir(t))
+			},
+			wantErr: "start awsgw service",
+		},
+		{
+			name: "spinifex-ui start (start failure via missing tls cert)",
+			cmd:  spinifexUIStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("spinifex-ui-base-dir", t.TempDir())
+				viper.Set("spinifex-ui-tls-cert", filepath.Join(t.TempDir(), "missing.pem"))
+			},
+			wantErr: "start spinifex-ui service",
+		},
+		{name: "vpcd start (missing config)", cmd: vpcdStartCmd},
+		{
+			name: "vpcd start (config file load failure)",
+			cmd:  vpcdStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load config file",
+		},
+		{
+			name: "vpcd start (legacy wan_bridge env rejected)",
+			cmd:  vpcdStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				t.Setenv("SPINIFEX_VPCD_WAN_BRIDGE", "br0")
+			},
+			wantErr: "deprecated 'wan_bridge'",
+		},
+		{
+			name: "vpcd start (start failure via blocked base-dir)",
+			cmd:  vpcdStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeSpinifexToml(t, minimalClusterToml))
+				viper.Set("base-dir", blockedDir(t))
+			},
+			wantErr: "start vpcd service",
+		},
+		{name: "qmp-collector start (missing config)", cmd: qmpCollectorStartCmd},
+		{
+			name: "qmp-collector start (config file load failure)",
+			cmd:  qmpCollectorStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("config", writeMalformedToml(t))
+			},
+			wantErr: "load config file",
+		},
+		{
+			name: "qmp-collector start (start failure via blocked base-dir)",
+			cmd:  qmpCollectorStartCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				toml := minimalClusterToml + fmt.Sprintf("base_dir = %q\n", blockedDir(t))
+				viper.Set("config", writeSpinifexToml(t, toml))
+			},
+			wantErr: "start qmp-collector service",
+		},
+		// The stop commands below resolve their pid directory to an isolated,
+		// always-empty temp dir, so Stop() safely fails to find a pid file
+		// rather than risking a real running process via an ambient path.
+		{
+			name: "predastore stop (no running process)",
+			cmd:  predastoreStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				viper.Set("predastore-base-path", t.TempDir())
+			},
+			wantErr: "stop predastore service",
+		},
+		{
+			name: "viperblock stop (no running process)",
+			cmd:  viperblockStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop viperblock service",
+		},
+		{
+			name: "qemunbd stop (no running process)",
+			cmd:  qemunbdStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop qemunbd service",
+		},
+		{
+			name: "nats stop (no running process)",
+			cmd:  natsStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop nats service",
+		},
+		{
+			name: "spinifex stop (no running process)",
+			cmd:  spinifexStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop spinifex service",
+		},
+		{
+			name: "awsgw stop (no running process)",
+			cmd:  awsgwStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop awsgw service",
+		},
+		{
+			name: "spinifex-ui stop (no running process)",
+			cmd:  spinifexUIStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop spinifex-ui service",
+		},
+		{
+			name: "vpcd stop (no running process)",
+			cmd:  vpcdStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop vpcd service",
+		},
+		{
+			name: "northstar stop (no running process)",
+			cmd:  northstarStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop northstar service",
+		},
+		{
+			name: "qmp-collector stop (no running process)",
+			cmd:  qmpCollectorStopCmd,
+			setup: func(t *testing.T) {
+				resetGlobalViper(t)
+				isolateRuntimeDir(t)
+			},
+			wantErr: "stop qmp-collector service",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.setup != nil {
+				tt.setup(t)
+			}
+			require.NotNil(t, tt.cmd.RunE, "command must use RunE so a fatal error propagates to a non-zero exit")
+			err := tt.cmd.RunE(tt.cmd, nil)
+			require.Error(t, err, "expected a fatal startup error to be returned, not swallowed")
+			if tt.wantErr != "" {
+				assert.ErrorContains(t, err, tt.wantErr)
+			}
+			assert.True(t, tt.cmd.SilenceErrors, "SilenceErrors must be set so cobra doesn't double-print the error root.go already reports")
+			assert.True(t, tt.cmd.SilenceUsage, "SilenceUsage must be set so a startup failure doesn't dump command usage")
+		})
+	}
+}
+
+// TestSpinifexUIStatusCmdReportsStoppedWithNoRunningProcess drives the
+// status command's success path: with no pid file at an isolated
+// XDG_RUNTIME_DIR, Status reports "stopped" and RunE must return nil.
+func TestSpinifexUIStatusCmdReportsStoppedWithNoRunningProcess(t *testing.T) {
+	isolateRuntimeDir(t)
+
+	require.NotNil(t, spinifexUIStatusCmd.RunE)
+	err := spinifexUIStatusCmd.RunE(spinifexUIStatusCmd, nil)
+	require.NoError(t, err)
+}
+
+// TestSpinifexUIStatusCmdReturnsErrorOnUnreadablePidDir drives the status
+// command's other branch: a genuine read failure (permission denied, not
+// ENOENT) must surface as an error rather than the benign "stopped" case.
+func TestSpinifexUIStatusCmdReturnsErrorOnUnreadablePidDir(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root bypasses directory permission checks")
+	}
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "spinifex-ui.pid"), []byte("123"), 0o600))
+	require.NoError(t, os.Chmod(dir, 0o000))
+	t.Cleanup(func() { _ = os.Chmod(dir, 0o755) })
+	t.Setenv("XDG_RUNTIME_DIR", dir)
+
+	require.NotNil(t, spinifexUIStatusCmd.RunE)
+	err := spinifexUIStatusCmd.RunE(spinifexUIStatusCmd, nil)
+	require.ErrorContains(t, err, "get spinifex-ui service status")
+}
+
+// TestServiceStartExitsNonZeroOnFailure proves a fatal service-start error
+// terminates the process non-zero (systemd's Restart=on-failure depends on
+// it). os.Exit can't be observed in-process, so this re-execs the test
+// binary as a subprocess and inspects its real exit code.
+func TestServiceStartExitsNonZeroOnFailure(t *testing.T) {
+	if os.Getenv("SPX_EXIT_CODE_TEST_HELPER") == "1" {
+		// Subprocess mode: run a fatal startup path for real via Execute().
+		// No flags/config supplied, so predastore start fails on host-id.
+		rootCmd.SetArgs([]string{"service", "predastore", "start"})
+		Execute()
+		return
+	}
+
+	execCmd := exec.Command(os.Args[0], "-test.run=^TestServiceStartExitsNonZeroOnFailure$")
+	execCmd.Env = append(os.Environ(), "SPX_EXIT_CODE_TEST_HELPER=1")
+	out, err := execCmd.CombinedOutput()
+
+	var exitErr *exec.ExitError
+	if !errors.As(err, &exitErr) {
+		t.Fatalf("expected the subprocess to fail with a non-zero exit, got err=%v output=%s", err, out)
+	}
+	assert.Equal(t, 1, exitErr.ExitCode(), "a fatal service-start error must exit non-zero for systemd Restart=on-failure to fire; output=%s", out)
+}