virtcontainers: Initial import

[sameo]

This is a virtcontainers 1.0.7 import into Kata Containers runtime.

virtcontainers is a Go library designed to manage hardware virtualized
pods and containers. It is the core Clear Containers framework and will
become the core Kata Containers framework, as discussed at
#33

Some additional pointers:

virtcontainers README, including some design and architecure notes:
https://github.com/containers/virtcontainers/blob/master/README.md

virtcontainers 1.0 API:
https://github.com/containers/virtcontainers/blob/master/documentation/api/1.0/api.md

Fixes #40

Signed-off-by: Samuel Ortiz sameo@linux.intel.com

[grahamwhaley]

I think there will be obvious things that will need cleanup and adaptation here, including but not limited to:

• docs
• CI scripts

but, as a first direct copy import that we can then base PRs on top of to fix up those items, and build the new API upon:

lgtm

[jodh-intel]

Thanks @sameo. There are a few bits I think we can remove:

• Drop the .ci/ as we already have that at the top-level
  (we can make changes to those top-level scripts specifically for vc if required potentially later).
• I'd consider removing NEWS entirely as it's something else (to forget) to maintain and can be handled via a normal release PR + tag.
• Drop .pullapprove.yml + OWNERS as we already have the former at the top-level. We could move OWNERS to the top-level, but again it will need updating with who is in the current top-level .pullapprove.yml so I'd vote we just zap it.
• Drop CONTRIBUTING.md and CODE_OF_CONDUCT.md as we have those at the top-level too.

[laijs]

The name "vircontainers" is confusing here since there will be a single unified runtime.
And how to update vircontainers/ when the repo vircontainers is updated?

[laijs]

IMO, I hope there will be two repos.
kata-containers/libkata: a repo like "vircontainers" or runv/hypervisor.
kata-containers/runkata: the cli repo like cc-runtime or runv/cli.

sandbox, agent, network, hypervisors and persistence as #32 described should be in different packages under kata-containers/libkata.

both vircontainers and runv can be refactored as it.

[sameo]

The name "vircontainers" is confusing here since there will be a single unified runtime.

If you're thinking about something like libkata, I'd disagree. We really want this library to not be kata specific. It's always been a design requirement as a driving function for building properly architectured software.

And how to update vircontainers/ when the repo vircontainers is updated?

See #40 for more details, but the idea is that the virtcontainers repo would be frozen, and kata-containers/runtime/virtcontainers/ becomes the single point of development for virtcontainers. There will be a transition period where we'd have to manually update, but we want to make it as short as possible.

[sameo]

IMO, I hope there will be two repos.
kata-containers/libkata: a repo like "vircontainers" or runv/hypervisor.

A few objections here:

• I'd prefer not to call it libkata at that point.
• We already have a lot of repos, I was hoping we could refrain ourselves from creating more as virtcontainers is going to be the biggest runtime dependency.
• virtcontainers and runc/hypervisor are not comparable.
• runc/hypervisor is not a repo.

kata-containers/runkata: the cli repo like cc-runtime or runv/cli.

The only reason why we have not put virtcontainers under clearcontainers/cc-runtime is to make it clear it was not designed to be a Clear Containers/Intel dependent component. Kata Containers is a more neutral organization and it's ok to move it there, but I believe both virtcontainers and the runtime cli should live under the runtime repos:

• gitub.com/kata-containers/runtime/virtcontainers (or some more relevant name if there's a consensus about name changing)
• github.com/kata-containers/runtime/cli for the actual cli implementation on top of virtcontainers.

We should not forget that a big part of what we build is an OCI compatible runtime and that is mostly defined by the CLI level compatibility.

[WeiZhang555]

gitub.com/kata-containers/runtime/virtcontainers (or some more relevant name if there's a consensus about name changing)
github.com/kata-containers/runtime/cli for the actual cli implementation on top of virtcontainers.

This is so good to me. As a core part of kata-runtime, it makes sense to put it directly under runtime/ .
So many repos is starting to make me headache 😛

This is a good start, though there are still many further modifications need to do. LGTM as a first step

[sboeuf]

I agree with @jodh-intel comments but I am fine if we do that as a follow up PR.
LGTM

[sameo]

@sboeuf @jodh-intel comments addressed

[sboeuf]

Thanks @sameo
About the CI, we are removing it from virtcontainers since we already have it in the repo. But we need to port what was done in virtcontainers into the kata runtime CI. Particularly, the testing of a PR affecting virtcontainers with Clear Containers. Any PR related to virtcontainers should be tested both for Clear Containers and Kata Containers.

[gnawux]

As my comments in #40.

I think we should agree on APIs before merging this PR though I don't want to put a -1 here. I am not saying I don't like the code. I don't like to merge code before the APIs are seriously discussed.

[jessfraz]

Dope! :) looks good to me

[amshinde]

lgtm

[devimc]

LGTM

[jodh-intel]

lgtm

[tallclair]

So it sounds like there are 2 somewhat conflicting goals:

1. virtcontainers is a generic library that can be reused outside of the kata runtime
2. Reduce the maintenance burden of managing 2 repos, in particular the need to keep the vendored dependency in runtime up to date.

Is that an accurate summary? One strategy is to move forward with the approach you have here, but in my experience we'll need to put some strict import restrictions in place (through presubmit tests) that prevent virtcontainers from gradually fusing with the runtime. The alternative approach would be to make it a separate repo, which is a much stronger separation, and automate the task of keeping the runtime repo up-to-date. Either way, I think some automation is required.

[sboeuf]

@tallclair that's a very clear description of the problem, and that's also the reason why virtcontainers is currently living in its own repo, decoupled from Intel Clear Containers. Unfortunately, the decision of moving this library to the kata-runtime repo has been taken in order to prevent contributors from confusion...

[sboeuf]

Adding do-not-merge until everybody agreed.

[sameo]

@tallclair That's an accurate summary indeed. Today we have this automation in place as virtcontainers does not live under the Clear Containers organization.

TL;DR I feel moving virtcontainers under the runtime repo is a safe and an easier approach for new contributors. But I'm not strongly against creating a dedicated repo for it if the community thinks it's a cleaner solution.

I'm happy to create a new virtcontainers repo if folks believe that this is the best solution. From our experience, this makes the developer's experience less straightforward as testing virtcontainers changes means playing with some dep tricks. This can be scripted but makes things less easy than just applying changes directly into the runtime repo.

Honestly I did not take into account the risk of reducing virtcontainers agnosticism by making it a runtime package, because this has been a feature we've been really careful about conserving from the very beginning. But your concerns might be valid as we're adding potentially much more contributors to it now, by moving it here.

@laijs already expressed his opinion about it. @gnawux @bergwolf @WeiZhang555 @jessfraz @sboeuf @jodh-intel @egernst would you prefer having a separate repo for virtcontainers over moving it under the runtime repo directly?

[WeiZhang555]

would you prefer having a separate repo for virtcontainers over moving it under the runtime repo directly?

Both are ok for me. But I prefer putting virtcontainers under runtime directly.

[sboeuf]

@sameo I would prefer a separate repo.

[sameo]

So to summarize the current state (please correct me if I misunderstood) we have:

@jessfraz is ok with this
@WeiZhang555 is ok with this
@sameo is ok with this (obviously)
@tallclair gives us a fair warning but does not have a strong preference for where it should live.
@gnawux would prefer to see the API discussion going further but does not want to NACK this PR at that point.
@sboeuf is ok with the PR but would prefer to see the virtcontainers code live in a different repo.
@devimc, @amshinde, @jodh-intel, @egernst and @grahamwhaley (All of them are virtcontainers contributors, whatever that means...) have ACKed this PR.

If that's a fair depiction of the current status, I'd like to consider removing the do-not-merge tag and proceed with that PR. Please speak up if you disagree.

[gnawux]

@sameo we are going to have the meeting in hours, let's discuss in the meeting. I don't think it is good to merge code before we have an agreement on APIs.

[laijs]

For ALL APIs(related functions, structures, interfaces ...), please add the document // unstable.
Because the APIs are going to be shifted to smooth the gap and meet the requirement.
We don't need to keep ALL APIs always as they were when the time they were imported.
We don't want the initial APIs block our future progresses.

On the other hand, // unstable should be also added when the new APIs are added or current ones are updated. // unstable will be kept until some milestones.

[devimc]

LGTM

[sameo]

Updated to import virtcontainers 1.0.8.
@laijs I will add a specific document explicitly stating that all APIs are alpha (i.e. not stable).

[WeiZhang555]

LGTM.

Codes are so many and it's hard to review, but let's trust virtcontainers and use it as our fundamental code base, and keep improving it in following iterations.
We all know these APIs are alpha, and we shouldn't release a 1.0 tag until all requirements we agreed are satisfied . Even without document we still have Github records, that's one nature advantage of a github project :-)

[laijs]

Ok, let the wheel run.