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

[release/1.7] Allow proxy plugins to have capabilities #10731

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

henry118
Copy link
Member

@henry118 henry118 commented Sep 26, 2024

Backport: allow proxy plugins to have capabilities

Original PR: #10337

Fixes: #10732

Signed-off-by: Henry Wang [email protected]
(cherry picked from commit 5b8dfbd)

Signed-off-by: Kern Walster <[email protected]>
(cherry picked from commit 5b8dfbd)
@dosubot dosubot bot added the area/runtime Runtime label Sep 26, 2024
@henry118
Copy link
Member Author

/test pull-containerd-node-e2e-1-7

@samuelkarp
Copy link
Member

We don't generally backport new features. Is there a compelling reason to do so here?

@henry118
Copy link
Member Author

This will allow remote snapshotters (i.e. fuse-overlayfs) to report their capabilities like "id-map", so that the id mapped mount optimal path can be executed to speed up the container startup times in userns use cases.

// Snapshotter supports ID remapping, we don't need to do anything.

@henry118
Copy link
Member Author

henry118 commented Sep 26, 2024

Since the remap-labels option is already part of 1.7, and fuse-overlayfs is also the only one that claims to support it. I am not sure how it could possibly work without this patch. Therefore this is a bug that should be fixed IMHO.

// use snapshotter opts or the remapped snapshot support to shift the filesystem
// currently the only snapshotter known to support the labels is fuse-overlayfs:
// https://github.com/AkihiroSuda/containerd-fuse-overlayfs
if context.Bool("remap-labels") {
cOpts = append(cOpts, containerd.WithNewSnapshot(id, image,
containerd.WithRemapperLabels(0, uidMap.HostID, 0, gidMap.HostID, uidMap.Size)))

@henry118
Copy link
Member Author

I reported a bug #10732 for this case.

@henry118
Copy link
Member Author

/test pull-containerd-node-e2e-1-7

@henry118
Copy link
Member Author

henry118 commented Oct 4, 2024

@samuelkarp Do you think this patch can be merged?

@henry118
Copy link
Member Author

henry118 commented Oct 4, 2024

AFAIK backports like this isn't really unprecedented. For instance, #10096.

@estesp estesp merged commit ac00c7b into containerd:release/1.7 Oct 7, 2024
57 checks passed
@dmcgowan dmcgowan added impact/changelog and removed area/runtime Runtime labels Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants