ci: fix a race in TestExecIn and TestExecInTTY #4445
Conversation
lifubang
force-pushed
the
fix-ci-race-execin
branch
from
a6c839a
to
ee42499
Compare
Please add this into the commit message. |
Or do a retry. |
I think because these are integration tests, so maybe we need to keep this to check we didn't exec into an unrelated container? |
Fixes: opencontainers#4437 We can use a chan to wait the output from init process. After we received the content, it means that the init process has started. Then we can exec into this container to use ps command to check the init process and the exec process are both exist. Signed-off-by: lifubang <[email protected]>
lifubang
force-pushed
the
fix-ci-race-execin
branch
from
ee42499
to
f08116b
Compare
| _ = stdoutR.Close() | ||
| _ = stdoutW.Close() |
There was a problem hiding this comment.
can't we just defer the close, instead of a function? We are ignoring the error anyways.
I mean it for all the cases, not just this.
| _ = stdinR.Close() | ||
| defer stdinW.Close() //nolint: errcheck | ||
| defer func() { | ||
| _, _ = stdinW.Write([]byte("hello")) |
There was a problem hiding this comment.
why do we write this? To stop the go routine? Can you add a comment or the message written can be self-explanatory instead of this hello?
If it's that, is it needed? Won't the close of the pipe already free the goroutine?
Fixes: #4437
We can use a chan to wait the output from init process. After we received the content,
it means that the init process has started. Then we can exec into this container to use
ps command to check the init process and the exec process are both exist.
Signed-off-by: lifubang [email protected]