refactor: Remove redundant code in stop function

ethpandaops · Jan 21, 2025 · 0369c73 · 0369c73
1 parent bd969c0
commit 0369c73
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 44 deletions.
diff --git a/cmd/cli/commands/stop/stop.go b/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
 	}

diff --git a/cmd/cli/commands/stop/stop_test.go b/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"))
 			},

diff --git a/internal/sidecar/docker.go b/internal/sidecar/docker.go
@@ -136,9 +136,19 @@ func (s *dockerSidecar) Stop() error {
 
 // Status returns the current state of the docker container.
 func (s *dockerSidecar) Status() (string, error) {
-	cmd := exec.Command("docker", "inspect", "-f", "{{.State.Status}}", "contributoor")
+	// First check if container exists.
+	cmd := exec.Command("docker", "ps", "-a", "--filter", "name=contributoor", "--format", "{{.ID}}")
 
 	output, err := cmd.Output()
+	if err != nil || len(strings.TrimSpace(string(output))) == 0 {
+		//nolint:nilerr // We don't care about the error here.
+		return "not running", nil
+	}
+
+	// Container exists, get its status.
+	cmd = exec.Command("docker", "inspect", "-f", "{{.State.Status}}", "contributoor")
+
+	output, err = cmd.Output()
 	if err != nil {
 		return "", fmt.Errorf("failed to get container status: %w", err)
 	}

diff --git a/internal/sidecar/systemd.go b/internal/sidecar/systemd.go
@@ -311,6 +311,10 @@ func (s *systemdSidecar) checkBinaryExists() error {
 // Status returns the current state of the service.
 func (s *systemdSidecar) Status() (string, error) {
 	if runtime.GOOS == ArchDarwin {
+		if err := s.checkDaemonExists(); err != nil {
+			return "", wrapNotInstalledError(err, "launchd")
+		}
+
 		// For macOS, check launchd status.
 		cmd := exec.Command("sudo", "launchctl", "list", "io.ethpandaops.contributoor")
 
@@ -332,6 +336,10 @@ func (s *systemdSidecar) Status() (string, error) {
 		return "inactive", nil
 	}
 
+	if err := s.checkDaemonExists(); err != nil {
+		return "", wrapNotInstalledError(err, "systemd")
+	}
+
 	// For Linux/systemd, get service state.
 	cmd := exec.Command("sudo", "systemctl", "show", "-p", "ActiveState", "--value", "contributoor.service")