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

rootless: exec join the user+mount namespace #2569

Merged

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Mar 7, 2019

it is not enough to join the user namespace where the container is
running. We also need to join the mount namespace so that we can
correctly look-up inside of the container rootfs. This is necessary
to lookup the mounted /etc/passwd file when --user is specified.

Closes: #2566

Signed-off-by: Giuseppe Scrivano [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2019
@giuseppe
Copy link
Member Author

giuseppe commented Mar 7, 2019

/retest

@giuseppe giuseppe force-pushed the rootless-fix-exec-with-user branch 2 times, most recently from 70ef3f1 to b28bc9b Compare March 7, 2019 08:51
@rhatdan
Copy link
Member

rhatdan commented Mar 7, 2019

LGTM
bot, retest this please

@TomSweeneyRedHat
Copy link
Member

LGTM
Test unhappiness doesn't appear to be related to these changes.

it is not enough to join the user namespace where the container is
running.  We also need to join the mount namespace so that we can
correctly look-up inside of the container rootfs.  This is necessary
to lookup the mounted /etc/passwd file when --user is specified.

Closes: containers#2566

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the rootless-fix-exec-with-user branch from 626bff1 to 6017641 Compare March 7, 2019 14:34
continue
}
return false, -1, errors.Errorf("dependency container %s is not running", ctr.ID())
conmonPid, err := strconv.Atoi(string(data))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we drop the pod == 0 check here, or is it handled in JoinDirectUserAndMountNS()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should fail as it is not running, but I am going to add it again as it is clearer to understand.

Thanks to catch it

@mheon
Copy link
Member

mheon commented Mar 7, 2019

/retest

when we are creating a container that depends on another one, be sure
we also join its mount namespace in addition to the user namespace.

Closes: containers#2556

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the rootless-fix-exec-with-user branch from 6017641 to 081291c Compare March 7, 2019 14:51
@mheon
Copy link
Member

mheon commented Mar 8, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 8, 2019
@openshift-merge-robot openshift-merge-robot merged commit c4815e8 into containers:master Mar 8, 2019
@maflcko
Copy link

maflcko commented Mar 13, 2019

Sorry if this is the wrong place to ask, but is there a release schedule?

I presume when everything in https://github.com/containers/libpod/pulls?q=is%3Apr+is%3Aopen+label%3A%22Release+Notes+1.2.0%22 is merged, this will make it into a release?

@mheon
Copy link
Member

mheon commented Mar 13, 2019

@MarcoFalke no explicit schedule so far, aside from a general goal of making at least one release a month.

I think we're looking to cut a 1.2 late next week or early the week after, once we land some more work from @baude related to healthchecks

@rhatdan
Copy link
Member

rhatdan commented Mar 14, 2019

@MarcoFalke We usually release when major next features get to complete. Or when there is enough bug fixes to warrant it. Usually no less then every other month.

But we don't have a schedule.

@debarshiray
Copy link
Member

This might have caused #2673

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants