-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
TestLinksEtcHostsContentMatch: use container.Exec() #36569
Conversation
I am not quite sure why but this test is sometimes failing like this: > 15:21:41 --- FAIL: TestLinksEtcHostsContentMatch (0.53s) > 15:21:41 assertions.go:226: > > Error Trace: links_linux_test.go:46 > 15:21:41 > Error: Not equal: > 15:21:41 > expected: "127.0.0.1\tlocalhost\n::1\tlocalhost > ip6-localhost > ip6-loopback\nfe00::0\tip6-localnet\nff00::0\tip6-mcastprefix\nff02::1\tip6-allnodes\nff02::2\tip6-allrouters\n172.17.0.2\tf53feb6df161\n" > 15:21:41 > received: "" To eliminate some possible failures (like ignoring stderr from `cat` or its exit code), let's use container.Exec() to read a file from a container. Fixes: e6bd20e ("Migrate some integration-cli test to api tests") Signed-off-by: Kir Kolyshkin <[email protected]>
@yongtang PTAL |
|
||
var b bytes.Buffer | ||
_, err = stdcopy.StdCopy(&b, ioutil.Discard, body) | ||
cID := container.Run(t, ctx, client, container.WithNetworkMode("host")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could add
poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond))
so that to make sure the container is running?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like such a check is redundant.
As far as I can see from the source code, container.Run() is synchronous operation, i.e. a successful return from container.Run() means the container is running (and its status is set to running)
Surely, a container can exit in between calls to container.Run() and container.Exec(), but there's no need to check for that either, as this is done in Exec().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🌵
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! just wanted to open an issue for this test 😄
Windows is failing on another flaky test (being worked on in #36571), but otherwise green;
|
I am not quite sure why but this test is sometimes failing like this:
To eliminate some possible failures (like ignoring stderr from
cat
or its exit code), let's use container.Exec() to read a file from a container.