From 8863e0f00503c1930e74376658abc7632555d1e2 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 23 Sep 2020 09:11:16 -0400 Subject: 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 --- pkg/domain/infra/tunnel/containers.go | 62 ++++++++++++++++++++++++++++------- pkg/domain/infra/tunnel/events.go | 32 ++++++++++++++++++ test/e2e/config.go | 4 --- test/e2e/create_test.go | 5 +-- test/e2e/exec_test.go | 5 ++- test/e2e/healthcheck_run_test.go | 1 - test/e2e/images_test.go | 8 ++--- test/e2e/start_test.go | 6 ++-- 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=""`, 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()) }) -- cgit v1.2.3-54-g00ecf