Various Windows fixes to support the runtime v2 shim workflow #2519
Conversation
Removes an unused and unneeded wait group. Signed-off-by: Justin Terry (VM) <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2519 +/- ##
=======================================
Coverage 45.07% 45.07%
=======================================
Files 93 93
Lines 9780 9780
=======================================
Hits 4408 4408
Misses 4654 4654
Partials 718 718
Continue to review full report at Codecov.
|
| lastError = errors.Errorf("timed out waiting for npipe %s", address) | ||
| break | ||
| } | ||
| c, lastError = winio.DialPipe(address, &remaining) |
There was a problem hiding this comment.
if it does not exist, does it return immediately or does it use the timeout?
There was a problem hiding this comment.
winio returns immediately with a file not found error because the \\.\pipe\whatever does not yet exist.
There was a problem hiding this comment.
@crosbymichael - Hang on this is the wrong commit. I dont quite know what happened but this doesnt have the right exit logic it it connects successfully.
There was a problem hiding this comment.
I mean if you are still working on it, you could maybe use a:
select {
case <-time.After(timeout):
case <-pipe:
...
}as a way to synchronize
There was a problem hiding this comment.
Ok this is now right.
I can do it that way if you prefer. Doesn't bother me either way. Since winio returns immediately for file not found it would still require the same looping logic and starting a new select query so it seems the same code in the end.
jterry75
force-pushed
the
various_win_fixes
branch
from
a42a5f6 to
6a7f4ae
Compare
| if tty { | ||
| ioCreator = cio.NewCreator(append([]cio.Opt{cio.WithStdio, cio.WithTerminal}, ioOpts...)...) | ||
| } | ||
| if nullIO { | ||
| } else if nullIO { | ||
| if tty { | ||
| return nil, errors.New("tty and null-io cannot be used together") |
There was a problem hiding this comment.
That's a great point. Missed that case will submit a fix.
|
LGTM |
Reorders the code so that it doesnt overwrite the previous allocation when creating a NewTask via ctr.exe Signed-off-by: Justin Terry (VM) <[email protected]>
Adds retry support to AnonDialer if the pipe does not exist. This will retry up to the timeout for the pipe to exist and connect. This solves the race between the containerd-shim-* start command and the reinvocation. Signed-off-by: Justin Terry (VM) <[email protected]>
jterry75
force-pushed
the
various_win_fixes
branch
from
6a7f4ae to
dcb9057
Compare
|
LGTM |
when creating a NewTask via ctr.exe
retry up to the timeout for the pipe to exist and connect. This solves
the race between the containerd-shim-* start command and the
reinvocation.
FYI - @crosbymichael @jhowardmsft