runtime-rs: fix interoperability issues between runtime-rs and cri-o #8986
Conversation
|
@littlejawa @gkurz @fidencio please take a look |
|
nit: in the commit message of your second commit: |
pmores
force-pushed
the
drop-shim-v2-address-value-validation
branch
from
cd5f960
to
d03c916
Compare
It appears that under the shim v2 protocol, a shim has no use of its own for the -address value, it just passes it back to container runtime's (mostly containerd or cri-o) event-publishing binary. Since the -address value only flows through the shim, being passed to the shim by a container runtime and then essentially passed back by shim to the container runtime, it seems inappropriate for a shim to validate the value that is fully owned and only used by the container runtime. This commit removes such validation from runtime-rs. Doing so, it solves (part of) an interoperability problem between runtime-rs and cri-o. cri-o seems to intentionally choose not to implement the event-publishing part of the shim v2 protocol and thus it has no value it could pass to runtime-rs for -address. As a result, it sends an empty string which has been failing the excessive validation performed by runtime-rs so far. Signed-off-by: Pavel Mores <pmores@redhat.com>
pmores
force-pushed
the
drop-shim-v2-address-value-validation
branch
from
d03c916
to
e0afbee
Compare
|
Pushed a new version to address flaws pointed out above, including the tests to stand a chance of CI passing. |
|
/test |
|
@studychao @amshinde @justxuewei this PR already has 2 approvals and CI passed but I'd still like to hear from you and make sure this won't break the runtime-rs experience with containerd. My current goal is to merge this tomorrow. Please provide feeback ASAP. |
For what it's worth, I launch runtime-rs both from cri-o and containerd and haven't run into any problems as far as I can tell. |
Since cri-o doesn't seem to use address for event publishing as mentioned in the previous commit it will not send it. However, the exact way of not sending it is unfortunately different from what is assumed by runtime-rs. Due to an implementation detail of cri-o which uses containerd libraries for some low-level tasks, TTRPC_ADDRESS will not be missing from environment as assumed, instead it will be present with an empty value. This commit contains a small adjustment to account for that and use LogForwarder even if TTRPC_ADDRESS is present, but with an empty value. Fixes kata-containers#8985 Signed-off-by: Pavel Mores <pmores@redhat.com>
pmores
force-pushed
the
drop-shim-v2-address-value-validation
branch
from
e0afbee
to
6346e04
Compare
katacontainersbot
added
size/medium
Thanks @pmores. The motivations behind my question are:
|
|
/test |
|
/retest-arm-unit |
runtime-rs cannot currently be launched by cri-o since cri-o passes an empty value for -address which runtime-rs incorrectly rejects due to an excessive -address value validation. This PR fixes this along with a different but related problem of incorrect handling of the shim v2 protocol
TTRPC_ADDRESSenvironment variable.This also makes runtime-rs consistent with the go runtime which correctly tolerates an empty -address value passed by cri-o.
Thanks to @littlejawa for insights in cri-o implementation!