Skip to content

ctr/tasks: support remapped UID/GID #7864

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.

Sign up for GitHub

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

Jump to bottom
Merged
fuweid merged 1 commit into from
Mar 2, 2023
Merged

ctr/tasks: support remapped UID/GID #7864

fuweid merged 1 commit into from
Mar 2, 2023

Conversation

@Iceber
Copy link
Member

@Iceber Iceber commented Dec 23, 2022 •
edited
Loading

ctr tasks start supports resolving to uid/gid mappings from spec.

For ctr run, the NewContainer function will also set the uid/gid mapping.

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

Lines 162 to 173 in 6c8c427

if uidmap, gidmap := context.String("uidmap"), context.String("gidmap"); uidmap != "" && gidmap != "" {
uidMap, err := parseIDMapping(uidmap)
if err != nil {
return nil, err
}
gidMap, err := parseIDMapping(gidmap)
if err != nil {
return nil, err
}
opts = append(opts,
oci.WithUserNamespace([]specs.LinuxIDMapping{uidMap}, []specs.LinuxIDMapping{gidMap}))
// use snapshotter opts or the remapped snapshot support to shift the filesystem

For uid/gid mappings that already exist in the container spec, there is no need for the ctr tasks start command to ignore it

@k8s-ci-robot
Copy link

k8s-ci-robot commented Dec 23, 2022

Hi @Iceber. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Iceber
Copy link
Member Author

Iceber commented Jan 4, 2023 •
edited
Loading

@estesp @AkihiroSuda PTAL, Thanks

@Iceber
Copy link
Member Author

Iceber commented Feb 28, 2023

@fuweid PTAL, Thanks

@fuweid
Copy link
Member

fuweid commented Mar 2, 2023

@Iceber code looks good to me. But we have batch update in pass two months. Would you please rebase the main branch to run the CI again? Thanks

@Iceber
Copy link
Member Author

Iceber commented Mar 2, 2023

@fuweid rebased, and it is green

@fuweid fuweid merged commit fd1db21 into containerd:main Mar 2, 2023
kiashok added a commit to kiashok/containerd that referenced this pull request Oct 23, 2024
@kiashok
Merged PR 7712980: Update fork-external/main with upstream main
8cdc0dc 
Update fork-external/main with upstream main at commit [081d818](containerd@081d818)

Marged upstream container/main into fork-external/main

Related work items: containerd#7864, containerd#7954, containerd#8041, containerd#8044, containerd#8051, containerd#8062, containerd#8096, containerd#8103, containerd#8109, containerd#8110, containerd#8113, containerd#8114, containerd#8119, containerd#8120, containerd#8128, containerd#8130, containerd#8134, containerd#8140, containerd#8142, containerd#8143, containerd#8152, containerd#8154, containerd#8162, containerd#8164, containerd#8165, containerd#8172, containerd#8173, containerd#8177, containerd#8178, containerd#8181, containerd#8183, containerd#8187, containerd#8188, containerd#8189, containerd#8190, containerd#8191, containerd#8192, containerd#8193
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kzys kzys kzys approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

needs-ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Iceber @k8s-ci-robot @fuweid @kzys