Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

stage0: Create an mtab symlink if not present #3265

Merged
merged 2 commits into from
Oct 11, 2016
Merged

stage0: Create an mtab symlink if not present #3265

merged 2 commits into from
Oct 11, 2016

Conversation

@euank
Copy link
Member

@euank euank commented Oct 6, 2016

Several Linux tools expect this to exist, but several images do not
include it. This creates it when it does not exist.

Fixes #3264

@euank euank added this to the v1.17.0 milestone Oct 6, 2016
@euank euank force-pushed the create-mtab-link branch from 5ab6236 to 0a7ced1 Compare October 6, 2016 21:07
@euank
stage0: Create an mtab symlink if not present
db333d0 
Several Linux tools expect this to exist, but several images do not
include it. This creates it when it does not exist.

Fixes rkt#3264
@euank euank force-pushed the create-mtab-link branch from 0a7ced1 to 08a45be Compare October 7, 2016 01:06
@euank euank force-pushed the create-mtab-link branch from 08a45be to 0840388 Compare October 7, 2016 02:01
s-urbaniak
s-urbaniak approved these changes Oct 7, 2016
View reviewed changes
Copy link
Contributor

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

LGTM but I'd like to have @lucab evaluate potential impacts.

@lucab lucab self-assigned this Oct 7, 2016
@lucab
Copy link
Member

lucab commented Oct 7, 2016

/etc/mtab has been deprecated some time ago, but you are right that some older images still rely on it so I'm ok in adding it.

I've just some doubts with the approach here which modifies the image rootfs before execution, while for all other cases we have been doing this in prepare-app. Is this case different somehow or are there reasons to do it separately instead?

@euank
Copy link
Member Author

euank commented Oct 7, 2016

I thought about prepare-app vs stage0 and ended up with it in stage0 because I saw this behaviour as a "generic across stage1s" thing. It also isn't too different from setting up resolv.conf, which is done in stage0.
I don't think I have a strong reason beyond the desire to keep prepare-app small and be generic across stage1s.
Happy to move it if you think it should be.

@s-urbaniak
Copy link
Contributor

s-urbaniak commented Oct 11, 2016

@lucab ping, is @euank's explanation ok?

@lucab
Copy link
Member

lucab commented Oct 11, 2016

I acknowledge that we have an hidden problem here: an image running under stage1-fly is currently not getting the same environment setup as other stage1 pods, as we have duplicate logic for this in prepare-app and in other places.
In the long term we may need to rethink this part of image setup, possibly moving some of this to the rendering phase (similarly to here).

However, this PR alone is fine so LGTM.

@lucab lucab merged commit a58ed03 into rkt:master Oct 11, 2016
@euank euank deleted the create-mtab-link branch October 11, 2016 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@lucab lucab lucab approved these changes

@s-urbaniak s-urbaniak s-urbaniak approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/usability kind/enhancement reviewed/lgtm

Projects

None yet

Milestone

v1.17.0

Development

Successfully merging this pull request may close these issues.

3 participants

@euank @lucab @s-urbaniak