Codecov Report

Attention: Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 21.82%. Comparing base (f46b631) to head (e22e77d).
Report is 1 commits behind head on main.

@@             Coverage Diff             @@
##             main     #604       +/-   ##
===========================================
- Coverage   34.31%   21.82%   -12.49%     
===========================================
  Files          65      122       +57     
  Lines        6598    10730     +4132     
===========================================
+ Hits         2264     2342       +78     
- Misses       4020     8070     +4050     
- Partials      314      318        +4     

Rebased.
CI is green except for codecov - although it is very confusing why would it be unhappy - I could not find out why in the detailed report.

x-ref; #611 (comment)

@imeoer rebased, and I included some of the fixes from @apostasie (from #604) in this patch, adding them as co-author.

My goal with these sets of PRs was to see if we could have a release with the new modules in place (removing the ones deprecated in containerd) before this module moves to v2, so that I can update the version used in BuildKit in preparation of dependencies starting to move to containerd v2 (these module moves in containerd are intended to help with that transition to make sure a common module is used between v1 and v2 codebases).

⚠️ Given that this PR removes some functionality, perhaps it warrants a v0.14.0 ❓ maybe tagging current main / #610 as v0.13.15 ?

Rebased.

CI is green.

Thanks @imeoer !

And thanks @thaJeztah for all the prep work!

@thaJeztah do you know if/when moby is going to make the switch to containerdv2? (specifically interested in pkg/sysinfo, which AFAIK does pull in just containerd/pkg/userns and containerd/pkg/seccomp)

You're welcome! There's so many projects that have a dependency on each other - it's sometimes difficult to wrap your head around; in all honesty some of these packages may not have had an immediate impact, but some were definitely good to get out of the way to (hopefully) smoothen the transition.

I actually just Yesterday noticed the github.com/containerd/cgroups -> github.com/containerd/cgroups/v3 change in this PR (which probably could've been included already to get rid of the old version), but probably not critical.

do you know if/when moby is going to make the switch to containerdv2?

We don't have a timeline yet; I know there's some work ongoing w.r.t. containerd/errdefs to unify various errdefs implementations (containerd, moby, buildkit, others?), and there was a draft PR to test v2, but no timeline yet.

specifically interested in pkg/sysinfo, which AFAIK does pull in just containerd/pkg/userns and containerd/pkg/seccomp)

For pkg/userns, I should add a warning; I opened PRs to get rid of that dependency (moving it to a separate module), but the current approach must not end up in a release, as (after discussion) it was not in the right module, and will be move to a separate module (github.com/moby/sys/userns instead of being part of github.com/moby/sys/user); see the PR below;

• user: move userns package to separate module, and retract v0.2.0 moby/sys#145

For containerd/pkg/seccomp, I think we should consider making that a separate module; that may involve unifying the containerd and moby implementation (the containerd profile was originally forked from the moby repository, but took a slightly different approach).

I did some refactoring some time ago to isolate the seccomp and apparmor profiles in the moby repository with the intent to eventually move them separate, but that's been some time ago, and we likely would have to spend some time looking at the current differences between containerd and moby, and to see if we can unify the implementations so that those profiles can live in separate modules (to be shared between projects).

Thanks a lot for all the info @thaJeztah

I think we might just copy / fork moby/pkg/sysinfo short term in nerdctl (as done in containerd/nerdctl#3173) so that we can fix the containerd dependency - and then we will move back to moby upstream when it is migrated over.