A bit out of topic of this PR.

The problem is that if we don't have something like #1615. For CI in cri-containerd, we still need to play all kinds of vendor tricks to clone containerd code, and automatically update containerd code to vendor new CRI plugin. :(

It is fine, but a bit ugly. (see containerd/cri#561 and containerd/cri#523 and containerd/cri#527.)

Another option is to completely merge containerd and cri-containerd into one repo.

@Random-Liu is there a cri-containerd issue open for this problem with a full description? I have a proposal, I'd like to make sure I'm commending in the right place and that I have all the context.

@Random-Liu I understand but we are clearly going down a rabbit hole of complexity that is untenable. It drastically affects the contribution experience. If the problem is that zfs isn't built in, we don't build a massively complex system of vendoring and export main packages, we just include zfs. Same thing with CRI.

If we incorporate everything and want to refactor the result, then we can do so at that time.

we still need to play all kinds of vendor tricks to clone containerd code, and automatically update containerd code to vendor new CRI plugin.

I mentioned in one of the issues you linked, but plugins aren't really a special case, they are just like any other dependency in vendor. dep makes updating dependencies a lot easier by specifying both a constraint version and a pinned version. This is probably the best way to solve any vendoring pain.

I understand but we are clearly going down a rabbit hole of complexity that is untenable. It drastically affects the contribution experience.

@stevvooe If we have #1615, we don't need to do those crazy script stuff in cri-containerd. I also don't like those scripts, that's why I'd like #1615 to land if possible. :)

@Random-Liu The problem with #1615 is that it creates other dependencies that we might have to support in the future. It also creates a split that might not be ideal in the future. I'd like to see a few options before we jump down that particular path.

@stevvooe Actually, a better option is to use golang plugin mode. Let's think about whether we can make it work.

Codecov Report

Merging #2096 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2096   +/-   ##
=======================================
  Coverage   41.19%   41.19%           
=======================================
  Files          66       66           
  Lines        7725     7725           
=======================================
  Hits         3182     3182           
  Misses       4041     4041           
  Partials      502      502

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b307df2...f4113a9. Read the comment docs.

@containerd/containerd-maintainers Are we a go or no go on this one? I get that a plugin system would be nice but it seems like there is some demand for zfs support.

How big is the demand? It's it big enough how suppose we can go ahead, but I'd prefer it being in a cmd/containerd/builtins_zfs_linux.go so it can be easily disabled at compile time.

I'd prefer it being in a cmd/containerd/builtins_zfs_linux.go so it can be easily disabled at compile time.

I think we can take those PRs as they come, but I'd like for 1.1 to have zfs and aufs support, as these are the biggest reasons to request plugins support.

LGTM

I would like to see a follow up though to separate this by a build tag

LGTM