hyperhq / runv

Hypervisor-based Runtime for OCI
Apache License 2.0
828 stars 129 forks source link

Merge containerd-nslistener and runv-shim #523

Closed WeiZhang555 closed 7 years ago

WeiZhang555 commented 7 years ago

There's a conflict for current architecture, currently for each container, we have four processes: runv-shim, runv-containerd, ns-listener and qemu-system-x86. We show pid of runv-shim as runv container's pid so that the runv-shim can proxy the exit code, signal and winsize change, but we also need to show pid of nslistener as runv container's pid so that it can listen on network change so that the CNI network plugin can work.

Also the future POD support also need one single shim/ns-listener process pid to identify the container, two processes makes everything a chaos.

All these things lead to one result: runv-shim and nslistener must belong to a single process. That's exactly what this commit is doing.

Enjoy it!

Signed-off-by: Zhang Wei zhangwei555@huawei.com

WeiZhang555 commented 7 years ago

According to my test, it works well. This is ready for review. ping @laijs

gnawux commented 7 years ago

looks the protobuf data are generated with a different generator. Is this an expected change?

WeiZhang555 commented 7 years ago

@gnawux I was using the latest protoc from github and got this result, the protobuf generating command is

$ protoc -I containerd/api/grpc/types/ containerd/api/grpc/types/api.proto --go_out=plugins=grpc:containerd/api/grpc/types
$ protoc --version
libprotoc 3.2.0

I'm not sure if I made something wrong, any suggestion?

WeiZhang555 commented 7 years ago

It seems the original "github.com/gogo/protobuf/proto" is a fork of "github.com/golang/protobuf/proto"(which is used by me) with some extra code generation features, but it seems that the official version also works fine. I don't know why you choose a forked version, do I need to switch to github.com/gogo/protobuf/proto ?

gnawux commented 7 years ago

@WeiZhang555 wait @laijs to clarify what the prefer generator is.

WeiZhang555 commented 7 years ago

Close this as this will introduce problem for POD support.