Merge pull request #73 from ethpandaops/fix/systemd-logs

fix(logs): use journalctl for systemd runMethod logs

cmd/cli/commands/logs/logs.go

@@ -83,19 +83,5 @@ func showLogs(
 		return fmt.Errorf("invalid sidecar run method: %s", cfg.RunMethod)
 	}
 
-	// Check if service is running.
-	running, err := runner.IsRunning()
-	if err != nil {
-		log.Errorf("could not check sidecar status: %v", err)
-
-		return err
-	}
-
-	if !running {
-		fmt.Printf("%sContributoor is not running%s\n", tui.TerminalColorYellow, tui.TerminalColorReset)
-
-		return nil
-	}
-
 	return runner.Logs(c.Int("tail"), c.Bool("follow"))
 }

cmd/cli/commands/logs/logs_test.go

@@ -36,22 +36,9 @@ func TestShowLogs(t *testing.T) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
 				}).Times(1)
-				d.EXPECT().IsRunning().Return(true, nil)
 				d.EXPECT().Logs(100, false).Return(nil)
 			},
 		},
-		{
-			name:      "docker - service not running",
-			runMethod: config.RunMethod_RUN_METHOD_DOCKER,
-			tailLines: 100,
-			follow:    false,
-			setupMocks: func(cfg *sidecarmock.MockConfigManager, d *sidecarmock.MockDockerSidecar, b *sidecarmock.MockBinarySidecar, s *sidecarmock.MockSystemdSidecar) {
-				cfg.EXPECT().Get().Return(&config.Config{
-					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
-				}).Times(1)
-				d.EXPECT().IsRunning().Return(false, nil)
-			},
-		},
 		{
 			name:      "docker - logs fail",
 			runMethod: config.RunMethod_RUN_METHOD_DOCKER,
@@ -61,7 +48,6 @@ func TestShowLogs(t *testing.T) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
 				}).Times(1)
-				d.EXPECT().IsRunning().Return(true, nil)
 				d.EXPECT().Logs(100, false).Return(errors.New("logs failed"))
 			},
 			expectedError: "logs failed",
@@ -75,7 +61,6 @@ func TestShowLogs(t *testing.T) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_BINARY,
 				}).Times(1)
-				b.EXPECT().IsRunning().Return(true, nil)
 				b.EXPECT().Logs(50, true).Return(nil)
 			},
 		},
@@ -88,7 +73,6 @@ func TestShowLogs(t *testing.T) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_SYSTEMD,
 				}).Times(1)
-				s.EXPECT().IsRunning().Return(true, nil)
 				s.EXPECT().Logs(200, false).Return(nil)
 			},
 		},

cmd/cli/commands/status/status.go

@@ -10,6 +10,8 @@ import (
 	"github.com/ethpandaops/contributoor/pkg/config/v1"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 )
 
 func RegisterCommands(app *cli.App, opts *options.CommandOpts) {
@@ -91,6 +93,12 @@ func showStatus(
 		return fmt.Errorf("failed to check status: %w", err)
 	}
 
+	// Get the underlying status from the sidecar.
+	status, err := runner.Status()
+	if err != nil {
+		return fmt.Errorf("failed to get status: %w", err)
+	}
+
 	// Print status information.
 	fmt.Printf("%sContributoor Status%s\n", tui.TerminalColorLightBlue, tui.TerminalColorReset)
 	fmt.Printf("%-20s: %s\n", "Version", cfg.Version)
@@ -103,13 +111,12 @@ func showStatus(
 		fmt.Printf("%-20s: %s\n", "Output Server", cfg.OutputServer.Address)
 	}
 
-	// Print running status with color
+	// Print running status with color.
 	statusColor := tui.TerminalColorRed
-	statusText := "Stopped"
+	statusText := cases.Title(language.English).String(status)
 
 	if running {
 		statusColor = tui.TerminalColorGreen
-		statusText = "Running"
 	}
 
 	fmt.Printf("%-20s: %s%s%s\n", "Status", statusColor, statusText, tui.TerminalColorReset)

cmd/cli/commands/status/status_test.go

@@ -34,6 +34,7 @@ func TestShowStatus(t *testing.T) {
 		// Create mock docker sidecar that's running
 		mockDocker := mock.NewMockDockerSidecar(ctrl)
 		mockDocker.EXPECT().IsRunning().Return(true, nil)
+		mockDocker.EXPECT().Status().Return("running", nil)
 
 		// Create mock binary sidecar (shouldn't be used)
 		mockBinary := mock.NewMockBinarySidecar(ctrl)
@@ -78,6 +79,7 @@ func TestShowStatus(t *testing.T) {
 		// Create mock binary sidecar that's stopped
 		mockBinary := mock.NewMockBinarySidecar(ctrl)
 		mockBinary.EXPECT().IsRunning().Return(false, nil)
+		mockBinary.EXPECT().Status().Return("stopped", nil)
 
 		// Create mock systemd sidecar (shouldn't be used)
 		mockSystemd := mock.NewMockSystemdSidecar(ctrl)
@@ -99,6 +101,43 @@ func TestShowStatus(t *testing.T) {
 		assert.NoError(t, err)
 	})
 
+	t.Run("shows status for systemd service", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		mockConfig := mock.NewMockConfigManager(ctrl)
+		mockConfig.EXPECT().Get().Return(&config.Config{
+			Version:           "1.0.0",
+			RunMethod:         config.RunMethod_RUN_METHOD_SYSTEMD,
+			NetworkName:       config.NetworkName_NETWORK_NAME_MAINNET,
+			BeaconNodeAddress: "http://localhost:5052",
+		}).AnyTimes()
+		mockConfig.EXPECT().GetConfigPath().Return("/path/to/config.yaml")
+
+		mockDocker := mock.NewMockDockerSidecar(ctrl)
+		mockBinary := mock.NewMockBinarySidecar(ctrl)
+
+		// Create mock systemd sidecar that's active
+		mockSystemd := mock.NewMockSystemdSidecar(ctrl)
+		mockSystemd.EXPECT().IsRunning().Return(true, nil)
+		mockSystemd.EXPECT().Status().Return("active", nil)
+
+		mockGithub := servicemock.NewMockGitHubService(ctrl)
+		mockGithub.EXPECT().GetLatestVersion().Return("1.0.0", nil)
+
+		err := showStatus(
+			cli.NewContext(nil, nil, nil),
+			logrus.New(),
+			mockConfig,
+			mockDocker,
+			mockSystemd,
+			mockBinary,
+			mockGithub,
+		)
+
+		assert.NoError(t, err)
+	})
+
 	t.Run("handles github service error gracefully", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
@@ -113,6 +152,7 @@ func TestShowStatus(t *testing.T) {
 
 		mockDocker := mock.NewMockDockerSidecar(ctrl)
 		mockDocker.EXPECT().IsRunning().Return(true, nil)
+		mockDocker.EXPECT().Status().Return("running", nil)
 		mockSystemd := mock.NewMockSystemdSidecar(ctrl)
 
 		// Create mock GitHub service that returns an error

cmd/cli/commands/stop/stop.go

@@ -87,21 +87,6 @@ func stopContributoor(
 		return fmt.Errorf("invalid sidecar run method: %s", cfg.RunMethod)
 	}
 
-	// Check if running before attempting to stop.
-	running, err := runner.IsRunning()
-	if err != nil {
-		log.Errorf("could not check sidecar status: %v", err)
-
-		return err
-	}
-
-	// If the service is not running, we can just return.
-	if !running {
-		fmt.Printf("%sContributoor is not running. Use 'contributoor start' to start it%s\n", tui.TerminalColorYellow, tui.TerminalColorReset)
-
-		return nil
-	}
-
 	if err := runner.Stop(); err != nil {
 		return err
 	}

cmd/cli/commands/stop/stop_test.go

@@ -27,65 +27,40 @@ func TestStopContributoor(t *testing.T) {
 		expectedError string
 	}{
 		{
-			name:      "docker - stops running service successfully",
+			name:      "docker - stops service successfully",
 			runMethod: config.RunMethod_RUN_METHOD_DOCKER,
 			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerSidecar, b *mock.MockBinarySidecar, g *servicemock.MockGitHubService) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
 					Version:   "latest",
 				}).Times(1)
-				d.EXPECT().IsRunning().Return(true, nil)
 				d.EXPECT().Stop().Return(nil)
 				g.EXPECT().GetLatestVersion().Return("v1.0.0", nil)
 			},
 		},
-		{
-			name:      "docker - service not running",
-			runMethod: config.RunMethod_RUN_METHOD_DOCKER,
-			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerSidecar, b *mock.MockBinarySidecar, g *servicemock.MockGitHubService) {
-				cfg.EXPECT().Get().Return(&config.Config{
-					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
-				}).Times(1)
-				d.EXPECT().IsRunning().Return(false, nil)
-				g.EXPECT().GetLatestVersion().Return("v1.0.0", nil)
-			},
-		},
 		{
 			name:      "docker - stop fails",
 			runMethod: config.RunMethod_RUN_METHOD_DOCKER,
 			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerSidecar, b *mock.MockBinarySidecar, g *servicemock.MockGitHubService) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
 				}).Times(1)
-				d.EXPECT().IsRunning().Return(true, nil)
 				d.EXPECT().Stop().Return(errors.New("stop failed"))
 				g.EXPECT().GetLatestVersion().Return("v1.0.0", nil)
 			},
 			expectedError: "stop failed",
 		},
 		{
-			name:      "binary - stops running service successfully",
+			name:      "binary - stops service successfully",
 			runMethod: config.RunMethod_RUN_METHOD_BINARY,
 			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerSidecar, b *mock.MockBinarySidecar, g *servicemock.MockGitHubService) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_BINARY,
 				}).Times(1)
-				b.EXPECT().IsRunning().Return(true, nil)
 				b.EXPECT().Stop().Return(nil)
 				g.EXPECT().GetLatestVersion().Return("v1.0.0", nil)
 			},
 		},
-		{
-			name:      "binary - service not running",
-			runMethod: config.RunMethod_RUN_METHOD_BINARY,
-			setupMocks: func(cfg *mock.MockConfigManager, d *mock.MockDockerSidecar, b *mock.MockBinarySidecar, g *servicemock.MockGitHubService) {
-				cfg.EXPECT().Get().Return(&config.Config{
-					RunMethod: config.RunMethod_RUN_METHOD_BINARY,
-				}).Times(1)
-				b.EXPECT().IsRunning().Return(false, nil)
-				g.EXPECT().GetLatestVersion().Return("v1.0.0", nil)
-			},
-		},
 		{
 			name:      "invalid sidecar run method",
 			runMethod: config.RunMethod_RUN_METHOD_UNSPECIFIED,
@@ -104,7 +79,6 @@ func TestStopContributoor(t *testing.T) {
 				cfg.EXPECT().Get().Return(&config.Config{
 					RunMethod: config.RunMethod_RUN_METHOD_DOCKER,
 				}).Times(1)
-				d.EXPECT().IsRunning().Return(true, nil)
 				d.EXPECT().Stop().Return(nil)
 				g.EXPECT().GetLatestVersion().Return("", errors.New("github error"))
 			},

internal/sidecar/binary.go

@@ -164,6 +164,39 @@ func (s *binarySidecar) Stop() error {
 	return nil
 }
 
+// Status returns the current state of the binary process.
+func (s *binarySidecar) Status() (string, error) {
+	cfg := s.sidecarCfg.Get()
+	pidFile := filepath.Join(cfg.ContributoorDirectory, "contributoor.pid")
+
+	// If no PID file, process is not running.
+	pidBytes, err := os.ReadFile(pidFile)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return "stopped", nil
+		}
+
+		return "", fmt.Errorf("failed to read pid file: %w", err)
+	}
+
+	pidStr := string(pidBytes)
+	if !regexp.MustCompile(`^\d+$`).MatchString(pidStr) {
+		return "unknown", fmt.Errorf("invalid PID format")
+	}
+
+	// kill -0 just checks if process exists. It doesn't actually send a
+	// signal that affects the process.
+	cmd := exec.Command("kill", "-0", pidStr)
+	if err := cmd.Run(); err != nil {
+		os.Remove(pidFile)
+
+		//nolint:nilerr // We don't care about the error here.
+		return "stopped", nil
+	}
+
+	return "running", nil
+}
+
 // IsRunning checks if the binary service is running.
 func (s *binarySidecar) IsRunning() (bool, error) {
 	cfg := s.sidecarCfg.Get()
@@ -230,9 +263,11 @@ func (s *binarySidecar) Logs(tailLines int, follow bool) error {
 		return fmt.Errorf("failed to expand config path: %w", err)
 	}
 
-	logFile := filepath.Join(expandedDir, "logs", "debug.log")
-
-	args := []string{}
+	var (
+		debugLog   = filepath.Join(expandedDir, "logs", "debug.log")
+		serviceLog = filepath.Join(expandedDir, "logs", "service.log")
+		args       = make([]string, 0)
+	)
 
 	if follow {
 		args = append(args, "-f")
@@ -242,7 +277,7 @@ func (s *binarySidecar) Logs(tailLines int, follow bool) error {
 		args = append(args, "-n", fmt.Sprintf("%d", tailLines))
 	}
 
-	args = append(args, logFile)
+	args = append(args, debugLog, serviceLog)
 
 	cmd := exec.Command("tail", args...)
 	cmd.Stdout = os.Stdout