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

define the default value for the cwd #286

Closed
wants to merge 1 commit into from

Conversation

laijs
Copy link
Contributor

@laijs laijs commented Dec 29, 2015

Signed-off-by: Lai Jiangshan jiangshanlai@gmail.com

Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
@wking
Copy link
Contributor

wking commented Dec 29, 2015

On Mon, Dec 28, 2015 at 07:50:21PM -0800, Lai Jiangshan wrote:

  • define the default value for the cwd

I think the root directory is a good default on Linux when the
container creates/joins a different mount namespace (and that seems to
be the default used by nsenter(1)). However, when you don't
create/join a different mount namespace, I think it makes more sense
to preserve the current working directory and not call chdir(2) at
all. That will allow your host-mount-namespace config.json process to
access data and such from your bundle directory by looking in its
working directory.

@laijs
Copy link
Contributor Author

laijs commented Dec 29, 2015

I think the root directory is a good default on Linux when the

the user may not be the root. "home directory" seems to be good one,
but it is a little hard for the runtime resolves the "home directory".
it requires parsing the /etc/passwd like #191. If #191 is accepted,
it is really good to define the default value of the cwd as "home directory".

@laijs
Copy link
Contributor Author

laijs commented Dec 29, 2015

currently, both runc&runv use "/" as the default value.

@philips
Copy link
Contributor

philips commented Dec 29, 2015

On Tue, Dec 29, 2015 at 10:03 AM W. Trevor King notifications@github.com
wrote:

On Mon, Dec 28, 2015 at 07:50:21PM -0800, Lai Jiangshan wrote:

  • define the default value for the cwd

I think the root directory is a good default on Linux when the
container creates/joins a different mount namespace (and that seems to
be the default used by nsenter(1)). However, when you don't
create/join a different mount namespace,

I really really want to avoid codifying special behavior. We either do one,
the other, or just require someone to be explicit. My preference would be
require the CWD be set, then "/".

I think it makes more sense
to preserve the current working directory and not call chdir(2) at
all. That will allow your host-mount-namespace config.json process to
access data and such from your bundle directory by looking in its
working directory.

I can see the use case but this is a little to magical IMHO.

@wking
Copy link
Contributor

wking commented Dec 29, 2015

On Tue, Dec 29, 2015 at 03:36:24AM -0800, Brandon Philips wrote:

I really really want to avoid codifying special behavior.

I think this falls under my preferred “don't do anything if this isn't
set” 1, which is how I've handled this in ccon with a pivot-root
caveat 2. I just checked with ccon, and using setns(2) to join a
mount namespace lands you in / (regardless of whether you pivoted root
in that namespace). So I don't think “we only attempt to apply this
setting if it's set” is codifying special behavior.

@mrunalp
Copy link
Contributor

mrunalp commented Jan 13, 2016

I think we don't need to add this specifically for Linux.

@crosbymichael
Copy link
Member

After reviewing in your OCI f2f we think this should be a required field so that runtimes don't default to something different if it is optional or different platforms have different defaults. By allowing the user to specify this then there is no question what the cwd will be for their container.

@vbatts vbatts mentioned this pull request Jan 13, 2016
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.

5 participants