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

TestExecInTTY: execin_test.go:351: unexpected carriage-return in output #2425

Open
kolyshkin opened this issue May 20, 2020 · 11 comments · Fixed by #2723
Open

TestExecInTTY: execin_test.go:351: unexpected carriage-return in output #2425

kolyshkin opened this issue May 20, 2020 · 11 comments · Fixed by #2723
Labels

Comments

@kolyshkin
Copy link
Contributor

During the course of CI for #2411 I saw a single failure from Fedora32 on Vagrant (logs here):

=== RUN   TestExecInTTY
    TestExecInTTY: execin_test.go:351: unexpected carriage-return in output
--- FAIL: TestExecInTTY (0.14s)

It only happened once, and so I suspect some race in the code or in the test. The test code comes from #1146.

@cyphar maybe you could take a look?

@XiaodongLoong
Copy link
Contributor

@kolyshkin
Copy link
Contributor Author

@kolyshkin
Copy link
Contributor Author

Another occurrence: #2682 (comment)

@kolyshkin
Copy link
Contributor Author

Seems that github actions is faster so we'll see this one more and more now since #2690 is in

kolyshkin added a commit to kolyshkin/runc that referenced this issue Dec 4, 2020
This helps to debug opencontainers#2425

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Dec 4, 2020
This helps to debug opencontainers#2425

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Dec 18, 2020
Do help to debug opencontainers#2425.

Previous commit 1909051 modified the code in the wrong place.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
ctalledo pushed a commit to ctalledo/sysbox-runc that referenced this issue Jan 6, 2021
This helps to debug opencontainers/runc#2425

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
ctalledo pushed a commit to ctalledo/sysbox-runc that referenced this issue Jan 6, 2021
Do help to debug opencontainers/runc#2425.

Previous commit 1909051b9cf modified the code in the wrong place.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

OK, finally got the output (from https://github.com/opencontainers/runc/runs/1659399228?check_suite_focus=true):

=== RUN   TestExecInTTY
832
    execin_test.go:349: unexpected carriage-return in output "PID   USER     TIME  COMMAND\r\n    1 root      0:00 cat\n    6 root      0:00 ps\n"
833
--- FAIL: TestExecInTTY (0.14s)

Note the first line ends with \r\n, while the subsequent ones only have \n. This looks like a race between running ps and setting terminal.

@kolyshkin
Copy link
Contributor Author

Easy to repro locally:

$ cd libcontainer/integration
$ go test -c .
$ $ sudo ./integration.test -test.run TestExecInT -test.count 1000
--- FAIL: TestExecInTTY (0.02s)
    execin_test.go:332: unexpected carriage-return in output "PID   USER     TIME  COMMAND\r\n    1 root      0:00 cat\r\n    8 root      0:00 ps\r\n"
--- FAIL: TestExecInTTY (0.03s)
    execin_test.go:332: unexpected carriage-return in output "PID   USER     TIME  COMMAND\r\n    1 root      0:00 cat\r\n    7 root      0:00 ps\r\n"
FAIL

@kolyshkin
Copy link
Contributor Author

kolyshkin commented Jan 7, 2021

Definitely a race:

sudo ./integration.test -test.run TestExecInT -test.count 1000
...
execin_test.go:332: unexpected carriage-return in output "PID USER TIME COMMAND\r\n 1 root 0:00 cat\r\n 7 root 0:00 ps\r\n"
execin_test.go:332: unexpected carriage-return in output "PID USER TIME COMMAND\r\n 1 root 0:00 cat\n 7 root 0:00 ps\n"
execin_test.go:332: unexpected carriage-return in output "PID USER TIME COMMAND\r\n 1 root 0:00 cat\r\n 8 root 0:00 ps\r\n"

Most of the times it passes, sometimes it's \r\n in the first line only, sometimes it's \r\n in all lines.

@kolyshkin
Copy link
Contributor Author

Reopened by #2749

dqminh pushed a commit to dqminh/runc that referenced this issue Feb 3, 2021
This helps to debug opencontainers#2425

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
dqminh pushed a commit to dqminh/runc that referenced this issue Feb 3, 2021
Do help to debug opencontainers#2425.

Previous commit 1909051 modified the code in the wrong place.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

This is not fixed, thus reopening.

@kolyshkin kolyshkin reopened this Oct 26, 2023
@kolyshkin
Copy link
Contributor Author

One more manifestation of the same race is #4029 in which we use a very short-lived container which produces a single line of output. Sometimes it's with CR, sometimes without 😕

@kolyshkin
Copy link
Contributor Author

Yet another manifestation of the same race is #4074.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants