Remove final v2remotefail failures

Most have been fixed, others I replaced with SkipIfRemote

Fix ContainerStart on tunnel, it needs to wait for the exit status
before returning.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

8 files changed, 93 insertions, 30 deletions

diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go
index d0f90d900..7ea3b16a6 100644
--- a/pkg/domain/infra/tunnel/containers.go
+++ b/pkg/domain/infra/tunnel/containers.go
@@ -481,27 +481,67 @@ func startAndAttach(ic *ContainerEngine, name string, detachKeys *string, input,
 
 func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []string, options entities.ContainerStartOptions) ([]*entities.ContainerStartReport, error) {
 	reports := []*entities.ContainerStartReport{}
-	for _, name := range namesOrIds {
+	var exitCode = define.ExecErrorCodeGeneric
+	ctrs, err := getContainersByContext(ic.ClientCxt, false, false, namesOrIds)
+	if err != nil {
+		return nil, err
+	}
+	// There can only be one container if attach was used
+	for i, ctr := range ctrs {
+		name := ctr.ID
 		report := entities.ContainerStartReport{
 			Id:       name,
-			RawInput: name,
-			ExitCode: 125,
+			RawInput: namesOrIds[i],
+			ExitCode: exitCode,
 		}
+		ctrRunning := ctr.State == define.ContainerStateRunning.String()
 		if options.Attach {
-			report.Err = startAndAttach(ic, name, &options.DetachKeys, options.Stdin, options.Stdout, options.Stderr)
-			if report.Err == nil {
-				exitCode, err := containers.Wait(ic.ClientCxt, name, nil)
-				if err == nil {
-					report.ExitCode = int(exitCode)
+			err = startAndAttach(ic, name, &options.DetachKeys, options.Stdin, options.Stdout, options.Stderr)
+			if err == define.ErrDetach {
+				// User manually detached
+				// Exit cleanly immediately
+				report.Err = err
+				reports = append(reports, &report)
+				return reports, nil
+			}
+			if ctrRunning {
+				reports = append(reports, &report)
+				return reports, nil
+			}
+
+			if err != nil {
+				report.ExitCode = define.ExitCode(report.Err)
+				report.Err = err
+				reports = append(reports, &report)
+				return reports, errors.Wrapf(report.Err, "unable to start container %s", name)
+			}
+			exitCode, err := containers.Wait(ic.ClientCxt, name, nil)
+			if err == define.ErrNoSuchCtr {
+				// Check events
+				event, err := ic.GetLastContainerEvent(ctx, name, events.Exited)
+				if err != nil {
+					logrus.Errorf("Cannot get exit code: %v", err)
+					report.ExitCode = define.ExecErrorCodeNotFound
+				} else {
+					report.ExitCode = event.ContainerExitCode
 				}
 			} else {
-				report.ExitCode = define.ExitCode(report.Err)
+				report.ExitCode = int(exitCode)
 			}
 			reports = append(reports, &report)
 			return reports, nil
 		}
-		report.Err = containers.Start(ic.ClientCxt, name, &options.DetachKeys)
-		report.ExitCode = define.ExitCode(report.Err)
+		// Start the container if it's not running already.
+		if !ctrRunning {
+			err = containers.Start(ic.ClientCxt, name, &options.DetachKeys)
+			if err != nil {
+				report.Err = errors.Wrapf(err, "unable to start container %q", name)
+				report.ExitCode = define.ExitCode(err)
+				reports = append(reports, &report)
+				continue
+			}
+		}
+		report.ExitCode = 0
 		reports = append(reports, &report)
 	}
 	return reports, nil
diff --git a/pkg/domain/infra/tunnel/events.go b/pkg/domain/infra/tunnel/events.go
index e6f4834b9..53bae6cef 100644
--- a/pkg/domain/infra/tunnel/events.go
+++ b/pkg/domain/infra/tunnel/events.go
@@ -2,8 +2,10 @@ package tunnel
 
 import (
 	"context"
+	//	"fmt"
 	"strings"
 
+	"github.com/containers/podman/v2/libpod/events"
 	"github.com/containers/podman/v2/pkg/bindings/system"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/pkg/errors"
@@ -29,3 +31,33 @@ func (ic *ContainerEngine) Events(ctx context.Context, opts entities.EventsOptio
 	}()
 	return system.Events(ic.ClientCxt, binChan, nil, &opts.Since, &opts.Until, filters, &opts.Stream)
 }
+
+// GetLastContainerEvent takes a container name or ID and an event status and returns
+// the last occurrence of the container event
+func (ic *ContainerEngine) GetLastContainerEvent(ctx context.Context, nameOrID string, containerEvent events.Status) (*events.Event, error) {
+	// check to make sure the event.Status is valid
+	if _, err := events.StringToStatus(containerEvent.String()); err != nil {
+		return nil, err
+	}
+	var event events.Event
+	return &event, nil
+
+	/*
+		        FIXME: We need new bindings for this section
+			filters := []string{
+				fmt.Sprintf("container=%s", nameOrID),
+				fmt.Sprintf("event=%s", containerEvent),
+				"type=container",
+			}
+
+			containerEvents, err := system.GetEvents(ctx, entities.EventsOptions{Filter: filters})
+			if err != nil {
+				return nil, err
+			}
+			if len(containerEvents) < 1 {
+				return nil, errors.Wrapf(events.ErrEventNotFound, "%s not found", containerEvent.String())
+			}
+			// return the last element in the slice
+			return containerEvents[len(containerEvents)-1], nil
+	*/
+}
diff --git a/test/e2e/config.go b/test/e2e/config.go
index 71c4dee31..0e1850614 100644
--- a/test/e2e/config.go
+++ b/test/e2e/config.go
@@ -27,8 +27,4 @@ var (
 	// v2fail is a temporary variable to help us track
 	// tests that fail in v2
 	v2fail = "does not pass integration tests with v2 podman"
-
-	// v2remotefail is a temporary variable to help us track
-	// tests that fail in v2 remote
-	v2remotefail = "does not pass integration tests with v2 podman remote"
 )
diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go
index 6845f1199..3fce536e2 100644
--- a/test/e2e/create_test.go
+++ b/test/e2e/create_test.go
@@ -108,13 +108,12 @@ var _ = Describe("Podman create", func() {
 	})
 
 	It("podman create --entrypoint \"\"", func() {
-		Skip(v2remotefail)
 		session := podmanTest.Podman([]string{"create", "--entrypoint", "", ALPINE, "ls"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(podmanTest.NumberOfContainers()).To(Equal(1))
 
-		result := podmanTest.Podman([]string{"inspect", "-l", "--format", "{{.Config.Entrypoint}}"})
+		result := podmanTest.Podman([]string{"inspect", session.OutputToString(), "--format", "{{.Config.Entrypoint}}"})
 		result.WaitWithDefaultTimeout()
 		Expect(result.ExitCode()).To(Equal(0))
 		Expect(result.OutputToString()).To(Equal(""))
@@ -134,7 +133,6 @@ var _ = Describe("Podman create", func() {
 	})
 
 	It("podman create --mount flag with multiple mounts", func() {
-		Skip(v2remotefail)
 		vol1 := filepath.Join(podmanTest.TempDir, "vol-test1")
 		err := os.MkdirAll(vol1, 0755)
 		Expect(err).To(BeNil())
@@ -160,7 +158,6 @@ var _ = Describe("Podman create", func() {
 		if podmanTest.Host.Arch == "ppc64le" {
 			Skip("skip failing test on ppc64le")
 		}
-		Skip(v2remotefail)
 		mountPath := filepath.Join(podmanTest.TempDir, "secrets")
 		os.Mkdir(mountPath, 0755)
 		session := podmanTest.Podman([]string{"create", "--name", "test", "--mount", fmt.Sprintf("type=bind,src=%s,target=/create/test", mountPath), ALPINE, "grep", "/create/test", "/proc/self/mountinfo"})
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index 745f7564e..7d50c02b2 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -123,13 +123,12 @@ var _ = Describe("Podman exec", func() {
 	})
 
 	It("podman exec terminal doesn't hang", func() {
-		Skip(v2remotefail)
-		setup := podmanTest.Podman([]string{"run", "-dti", fedoraMinimal, "sleep", "+Inf"})
+		setup := podmanTest.Podman([]string{"run", "-dti", "--name", "test1", fedoraMinimal, "sleep", "+Inf"})
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
 		for i := 0; i < 5; i++ {
-			session := podmanTest.Podman([]string{"exec", "-lti", "true"})
+			session := podmanTest.Podman([]string{"exec", "-ti", "test1", "true"})
 			session.WaitWithDefaultTimeout()
 			Expect(session.ExitCode()).To(Equal(0))
 		}
diff --git a/test/e2e/healthcheck_run_test.go b/test/e2e/healthcheck_run_test.go
index c020860ea..71e73af9c 100644
--- a/test/e2e/healthcheck_run_test.go
+++ b/test/e2e/healthcheck_run_test.go
@@ -174,7 +174,6 @@ var _ = Describe("Podman healthcheck run", func() {
 	})
 
 	It("podman healthcheck single healthy result changes failed to healthy", func() {
-		Skip(v2remotefail)
 		session := podmanTest.Podman([]string{"run", "-dt", "--name", "hc", "--health-retries", "2", "--health-cmd", "ls /foo || exit 1", ALPINE, "top"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
diff --git a/test/e2e/images_test.go b/test/e2e/images_test.go
index ddf2e20b8..d9ad10fe9 100644
--- a/test/e2e/images_test.go
+++ b/test/e2e/images_test.go
@@ -176,7 +176,7 @@ var _ = Describe("Podman images", func() {
 	})
 
 	It("podman images filter before image", func() {
-		Skip(v2remotefail)
+		SkipIfRemote("FIXME This should work on podman-remote")
 		dockerfile := `FROM docker.io/library/alpine:latest
 RUN apk update && apk add strace
 `
@@ -189,7 +189,6 @@ RUN apk update && apk add strace
 	})
 
 	It("podman images workingdir from  image", func() {
-		Skip(v2remotefail)
 		dockerfile := `FROM docker.io/library/alpine:latest
 WORKDIR /test
 `
@@ -341,7 +340,7 @@ WORKDIR /test
 	})
 
 	It("podman images --all flag", func() {
-		Skip(v2remotefail)
+		SkipIfRemote("FIXME This should work on podman-remote")
 		podmanTest.RestoreAllArtifacts()
 		dockerfile := `FROM docker.io/library/alpine:latest
 RUN mkdir hello
@@ -361,7 +360,6 @@ ENV foo=bar
 	})
 
 	It("podman images filter by label", func() {
-		Skip(v2remotefail)
 		dockerfile := `FROM docker.io/library/alpine:latest
 LABEL version="1.0"
 LABEL "com.example.vendor"="Example Vendor"
@@ -374,7 +372,7 @@ LABEL "com.example.vendor"="Example Vendor"
 	})
 
 	It("podman with images with no layers", func() {
-		Skip(v2remotefail)
+		SkipIfRemote("FIXME This should work on podman-remote")
 		dockerfile := strings.Join([]string{
 			`FROM scratch`,
 			`LABEL org.opencontainers.image.authors="<somefolks@example.org>"`,
diff --git a/test/e2e/start_test.go b/test/e2e/start_test.go
index 45c26fe43..35b5cab6e 100644
--- a/test/e2e/start_test.go
+++ b/test/e2e/start_test.go
@@ -131,14 +131,16 @@ var _ = Describe("Podman start", func() {
 	})
 
 	It("podman failed to start with --rm should delete the container", func() {
-		Skip(v2remotefail)
 		session := podmanTest.Podman([]string{"create", "--name", "test1", "-it", "--rm", ALPINE, "foo"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
 		start := podmanTest.Podman([]string{"start", "test1"})
 		start.WaitWithDefaultTimeout()
-		Expect(start).To(ExitWithError())
+
+		wait := podmanTest.Podman([]string{"wait", "test1"})
+		wait.WaitWithDefaultTimeout()
+		Expect(wait).To(ExitWithError())
 
 		Eventually(podmanTest.NumberOfContainers(), defaultWaitTimeout, 3.0).Should(BeZero())
 	})