Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

containerd runtime selection #1348

Closed
wants to merge 4 commits into from

Conversation

TBBle
Copy link
Collaborator

@TBBle TBBle commented Jan 25, 2020

WIP: Trying to validate on Travis, but temporarily contains a change this will be part of a different PR so I can compile-test locally on Windows too.

Depends on #1387

@TBBle TBBle mentioned this pull request Jan 25, 2020
@AkihiroSuda
Copy link
Member

ping

@TBBle
Copy link
Collaborator Author

TBBle commented Feb 28, 2020

Sorry, haven't had a chance to get back to this since #1314 was merged. The first thing I need to do is mark expected-fail all the tests that fail with containerd-1.2, so I can ensure I don't break the rest with this work.

windows-layer mounts (the only mount type supported by Windows) do not
currently use the `target` parameter to `mount.All`.

So instead, we treat them like bind-mounts and access the activated
layer in-place. Unlike the bind-mounts, we still need to Mount and
Unmount the layer.

Since we don't own the directory, we must not delete it after we unmount
it.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
This is the shim used by the containder Runtime V2 on Linux, per the
default setting of `io.containerd.runc.v2`.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
This reverts commit 9290c15.

This was pinned during the upgrade to containerd 1.3 series, which
changed the default runtime on Linux to io.containerd.runc.v2.

No specific rationale was listed for this pinning, and clearly it's the
wrong thing to do in the presence of Windows, which does not have this
runtime.

Instead, we rely on the containerd-internal defaults, which distinguish
the runtimes for Linux and Windows.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
This reverts commit c4f0305.

containerd 1.2 is still supported, per discussion at
moby#1314 (comment)

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
@TBBle
Copy link
Collaborator Author

TBBle commented Mar 1, 2020

Blocking this behind #1387, as it's not as simple as I'd hoped to make containerd 1.2 pass CI, so breaking that out separately.

@TBBle TBBle force-pushed the containerd_runtime_selection branch from 841dccd to bf96afd Compare March 1, 2020 13:24
@AkihiroSuda
Copy link
Member

Why do you need 1.2 for this?

@TBBle
Copy link
Collaborator Author

TBBle commented Mar 3, 2020

@AkihiroSuda See #1314 (comment) and #1387.

If it turns out we don't need containerd 1.2 support, then I'd for-now go with the initial pair of commits (cad9e12 and 0843d06) for now, and file this PR's ideas as "perhaps on a rainy Sunday afternoon, if I have nothing else particularly to do".

@TBBle
Copy link
Collaborator Author

TBBle commented Jul 16, 2020

BuildKit 0.7.0 formally dropped Containerd 1.2 support for the containerd worker, so I'm going to drop this never-really-started work, in favour of removing the hard-coded containerd 1.2 runner as in my initial work. (1, 2). See #1570.

@TBBle TBBle closed this Jul 16, 2020
@TBBle TBBle deleted the containerd_runtime_selection branch July 16, 2020 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants