Better error message for podman auto service (#1030)

* Better error message for podman auto svc

Signed-off-by: Matej Vasek <mvasek@redhat.com>

* change per review request

Co-authored-by: Lance Ball <lball@redhat.com>

Co-authored-by: Lance Ball <lball@redhat.com>

docker/docker_client.go

@@ -1,6 +1,7 @@
 package docker
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -117,34 +118,62 @@ func newClientWithPodmanService() (dockerClient client.CommonAPIClient, dockerHo
 	dockerHost = fmt.Sprintf("unix://%s", podmanSocket)
 
 	cmd := exec.Command("podman", "system", "service", dockerHost, "--time=0")
+
+	outBuff := bytes.Buffer{}
+	cmd.Stdout = &outBuff
+	cmd.Stderr = &outBuff
+
 	err = cmd.Start()
 	if err != nil {
 		return
 	}
 
+	waitErrCh := make(chan error)
+	go func() { waitErrCh <- cmd.Wait() }()
+
 	dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithHost(dockerHost), client.WithAPIVersionNegotiation())
 	stopPodmanService := func() {
 		_ = cmd.Process.Signal(syscall.SIGTERM)
 		_ = os.RemoveAll(tmpDir)
+
+		select {
+		case <-waitErrCh:
+			// the podman service has been shutdown, we don't care about error
+			return
+		case <-time.After(time.Second * 1):
+			// failed to gracefully shutdown the podman service, sending SIGKILL
+			_ = cmd.Process.Signal(syscall.SIGKILL)
+		}
 	}
 	dockerClient = clientWithAdditionalCleanup{
 		CommonAPIClient: dockerClient,
 		cleanUp:         stopPodmanService,
 	}
 
-	podmanServiceRunning := false
-	// give a time to podman to start
-	for i := 0; i < 40; i++ {
-		if _, e := dockerClient.Ping(context.Background()); e == nil {
-			podmanServiceRunning = true
-			break
+	svcUpCh := make(chan struct{})
+	go func() {
+		// give a time to podman to start
+		for i := 0; i < 40; i++ {
+			if _, e := dockerClient.Ping(context.Background()); e == nil {
+				svcUpCh <- struct{}{}
+			}
+			time.Sleep(time.Millisecond * 250)
 		}
-		time.Sleep(time.Millisecond * 250)
-	}
+	}()
 
-	if !podmanServiceRunning {
+	select {
+	case <-svcUpCh:
+		return
+	case <-time.After(time.Second * 10):
 		stopPodmanService()
-		err = errors.New("failed to start podman service")
+		err = errors.New("the podman service has not come up in time")
+	case err = <-waitErrCh:
+		// If this `case` is not selected then the waitErrCh is eventually read by calling stopPodmanService
+		if err != nil {
+			err = fmt.Errorf("failed to start the podman service (cmd out: %q): %w", outBuff.String(), err)
+		} else {
+			err = fmt.Errorf("the podman process exited before the service come up (cmd out: %q)", outBuff.String())
+		}
 	}
 
 	return

docker/docker_client_linux_test.go

@@ -2,6 +2,7 @@ package docker_test
 
 import (
 	"context"
+	"strings"
 	"testing"
 	"time"
 
@@ -11,7 +12,7 @@ import (
 
 // Test that we are starting podman service on behalf of user
 // if docker daemon is not present.
-func TestNewDockerClientWithAutomaticPodman(t *testing.T) {
+func TestNewDockerClientWithAutomaticPodmanSuccess(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute*1)
 	defer cancel()
@@ -21,7 +22,7 @@ func TestNewDockerClientWithAutomaticPodman(t *testing.T) {
 
 	dockerClient, _, err := docker.NewClient("unix:///var/run/nonexistent.sock")
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
 	}
 	defer dockerClient.Close()
 
@@ -32,6 +33,22 @@ func TestNewDockerClientWithAutomaticPodman(t *testing.T) {
 
 }
 
+func TestNewDockerClientWithAutomaticPodmanFail(t *testing.T) {
+	src := `package main;import ("os";"fmt");func main(){fmt.Println("something went wrong");os.Exit(1);}`
+
+	defer WithExecutable(t, "podman", src)()
+	defer WithEnvVar(t, "DOCKER_HOST", "")()
+
+	_, _, err := docker.NewClient("unix:///var/run/nonexistent.sock")
+	if err == nil {
+		t.Error("expected error but got nil")
+		return
+	}
+	if !strings.Contains(err.Error(), "something went wrong") {
+		t.Error("error doesn't contain stdout of the podman command")
+	}
+}
+
 // Go source code of mock podman implementation.
 // It just emulates docker /_ping endpoint for all URIs.
 const mockPodmanSrc = `package main