Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exec should wait for the command to finish before returning #372

Closed
omertuc opened this issue Jul 27, 2022 · 0 comments · Fixed by #383
Closed

Exec should wait for the command to finish before returning #372

omertuc opened this issue Jul 27, 2022 · 0 comments · Fixed by #383

Comments

@omertuc
Copy link

omertuc commented Jul 27, 2022

Describe the bug

Resource.Exec should wait for the command to finish before returning, otherwise the exit code it returns (which it gets from InspectExec) is always 0. Even for an extremely short-running command like false.

To Reproduce

Steps to reproduce the behavior:

package main

import (
	"fmt"
	"os"
	"github.com/ory/dockertest/v3"
)

func main() {
	pool, err := dockertest.NewPool("")
	if err != nil {
		fmt.Println(fmt.Errorf("new pool failed: %v", err.Error()))
		os.Exit(1)
	}

	resource, err := pool.RunWithOptions(&dockertest.RunOptions{
		Repository: "alpine",
		Tag:        "latest",
		Cmd:        []string{"sleep", "10"},
	})

	exitCode, err := resource.Exec([]string{"false"}, dockertest.ExecOptions{})
	if err != nil {
		fmt.Println(fmt.Errorf("exec failed: %v", err.Error()))
		os.Exit(1)
	}

	if exitCode == 0 {
		fmt.Println("This should never happen! yet it does...")
		os.Exit(1)
	}

	return
}

Expected behavior

I expect Exec to only return when the command is finished executing, and I expect the exit code returned by Exec to actually be the exit code of the command that finished and not always 0

Environment

go.sum:

github.com/ory/dockertest/v3 v3.9.1 h1:v4dkG+dlu76goxMiTT2j8zV7s4oPPEppKT8K8p2f1kY=
github.com/ory/dockertest/v3 v3.9.1/go.mod h1:42Ir9hmvaAPm0Mgibk6mBPi7SFvTXxEcnztDYOJ//uM=

docker version

$ docker version
Client:
 Version:           20.10.9
 API version:       1.41
 Go version:        go1.16.8
 Git commit:        c2ea9bc
 Built:             Sun Oct 10 22:41:20 2021
 OS/Arch:           linux/amd64
 Context:           default
 Experimental:      true

Server:
 Engine:
  Version:          20.10.9
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.16.8
  Git commit:       79ea9d3
  Built:            Fri Oct  8 00:00:00 2021
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.5.7
  GitCommit:
 runc:
  Version:          1.0.2
  GitCommit:        c42bf99-dirty
 docker-init:
  Version:          0.19.0
  GitCommit:

/etc/os-release

NAME="Fedora Linux"
VERSION="35 (Workstation Edition)"
ID=fedora
VERSION_ID=35
VERSION_CODENAME=""
PLATFORM_ID="platform:f35"
PRETTY_NAME="Fedora Linux 35 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:35"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f35/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=35
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=35
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Workstation Edition"
VARIANT_ID=workstation

Additional context

As a workaround users can do something like this, for now, but it's very verbose & awkward:

		exec, err := c.pool.Client.CreateExec(dc.CreateExecOptions{
			Container: c.resource.Container.ID,
			Cmd:       []string{"pg_isready"},
		})
		if err != nil {
			return false, fmt.Errorf("create exec failed: %w", err)
		}

		err = c.pool.Client.StartExec(exec.ID, dc.StartExecOptions{})
		if err != nil {
			return false, fmt.Errorf("start exec failed: %w", err)
		}

		exitCode := -1
		for {
			inspectExec, err := c.pool.Client.InspectExec(exec.ID)
			if err != nil {
				return false, fmt.Errorf("inspect exec failed: %w", err)
			}

			if !inspectExec.Running {
				exitCode = inspectExec.ExitCode
				break
			}
		}

		if exitCode != 0 {
			fmt.Printf("Waiting for docker database. pg_isready err: %v, exit code: %d\n", err, exitCode)
			return false, nil
		}

		return true, nil
flowchartsman added a commit to flowchartsman/dockertest that referenced this issue Oct 16, 2022
- This bug was due to a bug in the vendored client library where
  StartExec would return early even if opts.Detatch was true, if stderr
  and stdout were not provided. This fix always attaches them and falls
  back to io.Discard if the user does not provide their own readers.
- See: fsouza/go-dockerclient#838
- Fixes ory#372
aeneasr pushed a commit that referenced this issue Oct 18, 2022
This bug was due to a bug in the vendored client library where
StartExec would return early even if opts.Detatch was true, if stderr
and stdout were not provided. This fix always attaches them and falls
back to io.Discard if the user does not provide their own readers.

See: fsouza/go-dockerclient#838
Fixes #372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant