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

Fix ensurecontainerstarted logic and better debug #3573

Merged
merged 1 commit into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/testing/tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ You already saw two (`test.Expects` and `test.Contains`):
First, `test.Expects(exitCode int, errors []error, outputCompare Comparator)`, which is
convenient to quickly describe what you expect overall.

`exitCode` is obvious (note that passing -1 as an exit code will just verify the commands does fail without comparing the code).
`exitCode` is obvious (note that passing -1 as an exit code will just
verify the commands does fail without comparing the code, and -2 will not verify the exit
code at all).

`errors` is a slice of go `error`, that allows you to compare what is seen on stderr
with existing errors (for example: `errdefs.ErrNotFound`), or more generally
Expand Down
58 changes: 31 additions & 27 deletions pkg/testutil/nerdtest/registry/cesanta.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,34 +82,38 @@ func (cc *CesantaConfig) Save(path string) error {
return err
}

// FIXME: this is a copy of the utility method EnsureContainerStarted
// We cannot reference it (circular dep), so the copy.
// To be fixed later when we will be done migrating test helpers to the new framework and we can split them
// in meaningful subpackages.

func ensureContainerStarted(helpers test.Helpers, con string) {
const maxRetry = 5
const sleep = time.Second
success := false
for i := 0; i < maxRetry && !success; i++ {
time.Sleep(sleep)
count := i
cmd := helpers.Command("container", "inspect", con)
cmd.Run(&test.Expected{
Output: func(stdout string, info string, t *testing.T) {
var dc []dockercompat.Container
err := json.Unmarshal([]byte(stdout), &dc)
assert.NilError(t, err, "Unable to unmarshal output\n"+info)
assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
if dc[0].State.Running {
success = true
return
}
if count == maxRetry-1 {
// FIXME: there is currently no simple way to capture stderr
// Sometimes, it is convenient for debugging, like here
// Here we cheat with unbuffer which will bundle stderr and stdout together
// This is just bad
t.Error(helpers.Err("logs", con))
t.Fatalf("container %s still not running after %d retries", con, count)
}
},
})
started := false
for i := 0; i < 5 && !started; i++ {
helpers.Command("container", "inspect", con).
Run(&test.Expected{
ExitCode: test.ExitCodeNoCheck,
Output: func(stdout string, info string, t *testing.T) {
var dc []dockercompat.Container
err := json.Unmarshal([]byte(stdout), &dc)
if err != nil || len(dc) == 0 {
return
}
assert.Equal(t, len(dc), 1, "Unexpectedly got multiple results\n"+info)
started = dc[0].State.Running
},
})
time.Sleep(time.Second)
}

if !started {
ins := helpers.Capture("container", "inspect", con)
lgs := helpers.Capture("logs", con)
ps := helpers.Capture("ps", "-a")
helpers.T().Log(ins)
helpers.T().Log(lgs)
helpers.T().Log(ps)
helpers.T().Fatalf("container %s still not running after %d retries", con, 5)
}
}

Expand Down
46 changes: 27 additions & 19 deletions pkg/testutil/nerdtest/utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,28 +87,36 @@ func InspectImage(helpers test.Helpers, name string) dockercompat.Image {
}

const (
maxRetry = 5
maxRetry = 10
sleep = time.Second
)

func EnsureContainerStarted(helpers test.Helpers, con string) {
for i := 0; i < maxRetry; i++ {
count := i
cmd := helpers.Command("container", "inspect", con)
cmd.Run(&test.Expected{
Output: func(stdout string, info string, t *testing.T) {
var dc []dockercompat.Container
err := json.Unmarshal([]byte(stdout), &dc)
assert.NilError(t, err, "Unable to unmarshal output\n"+info)
assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info)
if dc[0].State.Running {
return
}
if count == maxRetry-1 {
t.Fatalf("container %s still not running after %d retries", con, count)
}
time.Sleep(sleep)
},
})
started := false
for i := 0; i < maxRetry && !started; i++ {
helpers.Command("container", "inspect", con).
Run(&test.Expected{
ExitCode: test.ExitCodeNoCheck,
Output: func(stdout string, info string, t *testing.T) {
var dc []dockercompat.Container
err := json.Unmarshal([]byte(stdout), &dc)
if err != nil || len(dc) == 0 {
return
}
assert.Equal(t, len(dc), 1, "Unexpectedly got multiple results\n"+info)
started = dc[0].State.Running
},
})
time.Sleep(sleep)
}

if !started {
ins := helpers.Capture("container", "inspect", con)
lgs := helpers.Capture("logs", con)
ps := helpers.Capture("ps", "-a")
helpers.T().Log(ins)
helpers.T().Log(lgs)
helpers.T().Log(ps)
helpers.T().Fatalf("container %s still not running after %d retries", con, maxRetry)
}
}
8 changes: 7 additions & 1 deletion pkg/testutil/test/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import (
"gotest.tools/v3/icmd"
)

const ExitCodeGenericFail = -1
const ExitCodeNoCheck = -2

// GenericCommand is a concrete Command implementation
type GenericCommand struct {
Config Config
Expand Down Expand Up @@ -117,7 +120,10 @@ func (gc *GenericCommand) Run(expect *Expected) {
// Build the debug string - additionally attach the env (which iCmd does not do)
debug := result.String() + "Env:\n" + strings.Join(env, "\n")
// ExitCode goes first
if expect.ExitCode == -1 {
if expect.ExitCode == ExitCodeNoCheck { //nolint:revive
// -2 means we do not care at all about exit code
} else if expect.ExitCode == ExitCodeGenericFail {
// -1 means any error
apostasie marked this conversation as resolved.
Show resolved Hide resolved
assert.Assert(gc.t, result.ExitCode != 0,
"Expected exit code to be different than 0\n"+debug)
} else {
Expand Down
2 changes: 1 addition & 1 deletion pkg/testutil/test/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (help *helpersInternal) Anyhow(args ...string) {
// Fail will run a command and make sure it does fail
func (help *helpersInternal) Fail(args ...string) {
help.Command(args...).Run(&Expected{
ExitCode: -1,
ExitCode: ExitCodeGenericFail,
})
}

Expand Down