Support vsock connection to task api #9738
Conversation
|
Hi @abel-von. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
abel-von
force-pushed
the
add-vsock
branch
2 times, most recently
from
91eef4e
to
ebf9144
Compare
core/runtime/v2/shim/util_unix.go
Outdated
| conn.Close() | ||
| // when it is EOF, maybe the server side is not ready. | ||
| if err == io.EOF { | ||
| time.Sleep(10 * time.Millisecond) |
There was a problem hiding this comment.
10 a bit too much IMO.
Should we reduce this or at least add a warning message.
There was a problem hiding this comment.
Changed this value to timeout/10 to do 10 retries before timeout. and warning log is added.
| if port > math.MaxUint32 { | ||
| return nil, fmt.Errorf("vsock port %d is invalid", port) | ||
| } | ||
| return vsock.Dial(uint32(contextID), uint32(port), &vsock.Config{}) |
There was a problem hiding this comment.
Recent Linux kernels got vsock with loopback support. Possibly we could have a test for this just for sanity check? (I'm fine to have this in follow up)
There was a problem hiding this comment.
It seems we have to listen the loopback first and then we can check the connection to it. Shall we do it when init or use a sync.Once to make the check done only once.
There was a problem hiding this comment.
I'd do everything within a single unit test.
There was a problem hiding this comment.
oh, I got what you mean now.
abel-von
force-pushed
the
add-vsock
branch
2 times, most recently
from
a94a51c
to
967dc6a
Compare
abel-von
force-pushed
the
add-vsock
branch
2 times, most recently
from
e8d4933
to
d891a8c
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
|
@abel-von could you pls rebase the PR ? |
Signed-off-by: Abel Feng <fshb1988@gmail.com>
|
done @mxpv |
dmcgowan
changed the title
In VM based sandbox scenario, we can start Task api Server inside VM. and containerd could connect it directly, without a shim process to do the api proxy or conversion.