Detect running containers to remove unnecessary error messages (#81)

* Detect running containers to prevent unneeded kills

* Use reusable command executor to get failure message

* Separate kill and cleanup

* Fix linter error and address review comment

* Address review comments

* Address review comments

cli_plugin.go

@@ -1,6 +1,7 @@
 package podman
 
 import (
+	"fmt"
 	"io"
 
 	log "go.arcalot.io/log/v2"
@@ -26,19 +27,39 @@ func (p *CliPlugin) Read(b []byte) (n int, err error) {
 }
 
 func (p *CliPlugin) Close() error {
-	if err := p.wrapper.KillAndClean(p.containerName); err != nil {
-		return err
+	containerRunning, err := p.wrapper.ContainerRunning(p.containerImage)
+	if err != nil {
+		p.logger.Warningf("error while checking if container exists (%s);"+
+			" killing container in case it still exists", err.Error())
+	} else if containerRunning {
+		p.logger.Infof("container %s still exists; killing container", p.containerName)
 	}
+	var killErr error
+	if err != nil || containerRunning {
+		killErr = p.wrapper.Kill(p.containerName)
+	}
+
+	// Still clean up even if the kill fails. Clean() uses the --force parameter, so that
+	// will be another attempt at killing the container.
+	cleanErr := p.wrapper.Clean(p.containerName)
 
 	if err := p.stdin.Close(); err != nil {
-		p.logger.Errorf("failed to close stdin pipe")
+		p.logger.Warningf("failed to close stdin pipe")
 	} else {
-		p.logger.Infof("stdin pipe successfully closed")
+		p.logger.Debugf("stdin pipe successfully closed")
 	}
 	if err := p.stdout.Close(); err != nil {
-		p.logger.Infof("failed to close stdout pipe")
+		p.logger.Warningf("failed to close stdout pipe")
 	} else {
-		p.logger.Infof("stdout pipe successfully closed")
+		p.logger.Debugf("stdout pipe successfully closed")
+	}
+	switch {
+	case killErr != nil && cleanErr != nil:
+		return fmt.Errorf("error while killing container (%s) and cleaning up container (%s)", killErr.Error(), cleanErr.Error())
+	case killErr != nil:
+		return killErr
+	case cleanErr != nil:
+		return cleanErr
 	}
 	return nil
 }

internal/cliwrapper/cliwrapper.go

@@ -53,6 +53,19 @@ func (p *cliWrapper) ImageExists(image string) (*bool, error) {
 	return &exists, nil
 }
 
+func (p *cliWrapper) ContainerRunning(containerID string) (bool, error) {
+	outStr, err := p.runPodmanCmd(
+		"checking whether container is running",
+		"container", "ls", "--format", "{{.Names}}",
+	)
+	if err != nil {
+		return true, err
+	}
+	outSlice := strings.Split(outStr, "\n")
+	exists := util.SliceContains(outSlice, containerID)
+	return exists, nil
+}
+
 func (p *cliWrapper) PullImage(image string, platform *string) error {
 	commandArgs := []string{"pull"}
 	if platform != nil {
@@ -82,21 +95,23 @@ func (p *cliWrapper) Deploy(image string, podmanArgs []string, containerArgs []s
 	return stdin, stdout, nil
 }
 
-func (p *cliWrapper) KillAndClean(containerName string) error {
-	cmdKill := p.getPodmanCmd("kill", containerName)
-	p.logger.Debugf("Killing with command %v", cmdKill.Args)
-	if err := cmdKill.Run(); err != nil {
-		p.logger.Warningf("failed to kill pod %s, probably the execution terminated earlier", containerName)
+func (p *cliWrapper) Kill(containerName string) error {
+	_, err := p.runPodmanCmd("killing container "+containerName, "kill", containerName)
+	if err != nil {
+		p.logger.Warningf("failed to kill pod %s (%s); it may have exited earlier", containerName, err.Error())
 	} else {
-		p.logger.Warningf("successfully killed container %s", containerName)
+		p.logger.Debugf("successfully killed container %s", containerName)
 	}
+	return nil
+}
 
+func (p *cliWrapper) Clean(containerName string) error {
 	msg := "removing container " + containerName
 	_, err := p.runPodmanCmd(msg, "rm", "--force", containerName)
 	if err != nil {
 		p.logger.Errorf(err.Error())
 	} else {
-		p.logger.Infof("successfully removed container %s", containerName)
+		p.logger.Debugf("successfully removed container %s", containerName)
 	}
 	return nil
 }

internal/cliwrapper/cliwrapper_interface.go

@@ -6,11 +6,13 @@ import (
 
 type CliWrapper interface {
 	ImageExists(image string) (*bool, error)
+	ContainerRunning(image string) (bool, error)
 	PullImage(image string, platform *string) error
 	Deploy(
 		image string,
 		podmanArgs []string,
 		containerArgs []string,
 	) (io.WriteCloser, io.ReadCloser, error)
-	KillAndClean(containerName string) error
+	Kill(containerName string) error
+	Clean(containerName string) error
 }