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

runtime-linux: Condition /proc/self/fd symlinks on source existence #736

Merged
merged 1 commit into from
May 10, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented Mar 16, 2017

Since #666 it's no longer guaranteed that /proc will exist. And there doesn't seem to be much point in requiring symlinks which will be known broken.

This commit also tightens the timing. Before it was just “after the container has /proc mounted”, which could have happened during the delete operation (if the container authors wanted to be especially ornery). With this commit, I've put the creation in step 2 of the lifecycle. And within step 2, it happens after mounts has been processed.

Since 279c3c0 (linux: relax filesystem requirements for container,
2017-01-23, opencontainers#666) it's no longer guaranteed that /proc will exist.
And there doesn't seem to be much point in requiring symlinks which
will be known broken.

This commit also tightens the timing.  Before it was just "after the
container has `/proc` mounted", which could have happened during the
'delete' operation (if the container authors wanted to be especially
ornery).  With this commit, I've put the creation in step 2 of the
lifecycle.  And within step 2, it happens after 'mounts' has been
processed.

Signed-off-by: W. Trevor King <wking@tremily.us>
@tianon
Copy link
Member

tianon commented May 10, 2017

LGTM

Approved with PullApprove

1 similar comment
@crosbymichael
Copy link
Member

crosbymichael commented May 10, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 844f392 into opencontainers:master May 10, 2017
@wking wking deleted the dev-symlink-conditional branch May 10, 2017 23:50
@vbatts vbatts mentioned this pull request Jul 5, 2017
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.

3 participants