Skip to content

runtime: add linux default fs validation - #121

Merged
mrunalp merged 2 commits into
opencontainers:masterfrom
Mashimiao:runtime-test-test-linux-default-fs
Jun 30, 2016
Merged

runtime: add linux default fs validation#121
mrunalp merged 2 commits into
opencontainers:masterfrom
Mashimiao:runtime-test-test-linux-default-fs

Conversation

@Mashimiao

@MashimiaoMashimiao commented Jun 23, 2016

Copy link
Copy Markdown

Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com

wking
wking reviewed Jun 23, 2016
View reviewed changes
Comment threadcmd/runtimetest/main.go Outdated

for fs, fstype := range defaultFS {
if !(mountsMap[fs] == fstype) {
return fmt.Errorf("%v must exists and expected type is %v", fs, fstype)

@wkingwkingJun 23, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: “exists” → “exist”.

@MashimiaoMashimiaoJun 24, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed, thanks.

@wking

wking commented Jun 23, 2016

Copy link
Copy Markdown
Contributor

On Thu, Jun 23, 2016 at 09:11:21AM -0700, Ma Shimiao wrote:

  • runtime: add linux default fs validation

This looks ok to me, although there are fuzzy corner cases if
config.json specifies a different type be mounted over one of these
locations. See the last point in 1 about when the runtime is
supposed to supply these mounts/devices, and we still have no wording
about that in the spec itself.

@Mashimiao
Mashimiaoforce-pushed the runtime-test-test-linux-default-fs branch from e2b3180 to 258adc8CompareJune 24, 2016 01:28
@Mashimiao

Mashimiao commented Jun 25, 2016

Copy link
Copy Markdown
Author

As smoe validations related to linux os, splited linux-specific valdiations

@wking

wking commented Jun 25, 2016

Copy link
Copy Markdown
Contributor

On Fri, Jun 24, 2016 at 07:16:22PM -0700, Ma Shimiao wrote:

As smoe validations related to linux os, splited linux-specific
valdiations

258adc8 itself looks good for me (except for “valdations” →
“validations” in the commit message).

I'm still not convinced that the current spec wording is enforcable
1, but I'm ok with punting on that for now (until someone asks us to
test the case where a config.json asks for a bind mount over /tmp? ;).

@Mashimiao
Mashimiaoforce-pushed the runtime-test-test-linux-default-fs branch from 258adc8 to 798d84fCompareJune 25, 2016 08:12
@Mashimiao

Mashimiao commented Jun 25, 2016

Copy link
Copy Markdown
Author

fixed valdations->validations

@wking

wking commented Jun 25, 2016

Copy link
Copy Markdown
Contributor

On Sat, Jun 25, 2016 at 01:12:52AM -0700, Ma Shimiao wrote:

fixed valdations->validations

798d84f looks good to me if we're punting on the corner cases 1.

liangchenye
liangchenye reviewed Jun 26, 2016
View reviewed changes
Comment threadcmd/runtimetest/main.go Outdated
defaultFS = map[string]string{
"/proc": "proc",
"/sys": "sysfs",
"dev/pts": "devpts",

@liangchenyeliangchenyeJun 26, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change "dev/pts" to /dev/pts ?

same with "dev/shm"

@MashimiaoMashimiaoJun 26, 2016

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed

Ma Shimiao added 2 commits June 26, 2016 22:15
Ma Shimiao
runtime: add linux default fs validation
4cfd018
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Ma Shimiao
split linux-specific validations
e4b2183
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
@Mashimiao
Mashimiaoforce-pushed the runtime-test-test-linux-default-fs branch from 798d84f to e4b2183CompareJune 26, 2016 14:18
@Mashimiao

Mashimiao commented Jun 28, 2016

Copy link
Copy Markdown
Author

ping @liangchenye@mrunalp

@liangchenye

liangchenye commented Jun 28, 2016

Copy link
Copy Markdown
Member

e4b2183 LGTM

@mrunalp

mrunalp commented Jun 30, 2016

Copy link
Copy Markdown
Contributor

LGTM

@mrunalp
mrunalp merged commit 239e8e9 into opencontainers:masterJun 30, 2016
@Mashimiao
Mashimiao deleted the runtime-test-test-linux-default-fs branch November 14, 2016 09:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Mashimiao@wking@liangchenye@mrunalp