Skip to content

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

Merged
estesp merged 1 commit intocontainerd:release/1.7from
henry118:proxy
Oct 7, 2024
Merged

[release/1.7] Allow proxy plugins to have capabilities#10731
estesp merged 1 commit intocontainerd:release/1.7from
henry118:proxy

Conversation

@henry118
Copy link
Copy Markdown
Member

@henry118 henry118 commented Sep 26, 2024 •
edited
Loading

Backport: allow proxy plugins to have capabilities

Original PR: #10337

Fixes: #10732

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

@henry118
Allow proxy plugins to have capabilities
9507403 
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
Copy Markdown
Member Author

henry118 commented Sep 26, 2024

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

@samuelkarp
Copy link
Copy Markdown
Member

samuelkarp commented Sep 26, 2024

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

@henry118
Copy link
Copy Markdown
Member Author

henry118 commented Sep 26, 2024

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.

containerd/snapshotter_opts_unix.go

Line 49 in 6fbbe70

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

@henry118
Copy link
Copy Markdown
Member Author

henry118 commented Sep 26, 2024 •
edited
Loading

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.

containerd/cmd/ctr/commands/run/run_unix.go

Lines 176 to 181 in 6fbbe70

// 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
Copy Markdown
Member Author

henry118 commented Sep 27, 2024

I reported a bug #10732 for this case.

@henry118
Copy link
Copy Markdown
Member Author

henry118 commented Sep 28, 2024

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

@henry118
Copy link
Copy Markdown
Member Author

henry118 commented Oct 4, 2024

@samuelkarp Do you think this patch can be merged?

@henry118
Copy link
Copy Markdown
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
@dmcgowan dmcgowan added impact/changelog and removed area/runtime Runtime labels Oct 8, 2024
@dmcgowan dmcgowan mentioned this pull request Oct 8, 2024
Merged
@henry118 henry118 deleted the proxy branch January 14, 2025 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@estesp estesp estesp approved these changes

@kiashok kiashok kiashok approved these changes

Assignees

No one assigned

Labels

impact/changelog size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@henry118 @samuelkarp @dmcgowan @estesp @kiashok @k8s-ci-robot