validate: add mount type check #119

Mashimiao · 2016-06-23T11:59:07Z

Signed-off-by: Ma Shimiao [email protected]

wking · 2016-06-23T17:35:49Z

validate.go

+		}
+	}
+
+	supportedTypes["nfs"] = true


I see an nfs entry in my /proc/filesystems (and an nfs4 entry). Why are you adding it by hand here?

Ah, it seems it's really no need to add.

wking · 2016-06-24T04:30:45Z

Also related to this and the ‘bind’ type is that mount.type might
become optional (opencontainers/runtime-spec#470). In ccon, bind
mounts are handled with explict MS_BIND (and MS_REC) flags 1.

wking · 2016-06-27T05:03:26Z

man/ocitools-validate.1.md

  Check specified hooks exist and are executable on the host.

+**--host-specific**
+  Check host specified configs.


“specified” → “specific”. And I'd add more context like:

By default, validation only tests for compatibility with a hypothetical host. With this flag, validation will also run more specific tests to see whether the current host is capable of launching a container from the configuration. For example, validating a compliant Windows configuration on a Linux machine will pass without this flag ("there may be a Windows host capable of launching this container"), but will fail with it ("this host is not capable of launching this container").

wking · 2016-06-27T16:28:17Z

man/ocitools-validate.1.md

+  For example, validating a compliant Windows configuration on a Linux machine 
+  will pass without this flag ("there may be a Windows host capable of 
+  launching this container"), but will fail with it ("this host is not capable 
+  of launching this container").


nit: there's trailing whitespace here (in case we care in this repository; I don't if we do).

Fixed. Thanks.

wking · 2016-06-27T16:35:23Z

32803b5 looks good to me, excepting a possible whitespace nit 1 and
a warning for Linux configs when --host-specific is not set 2. I'm
fine punting checkPlatform changes to a follow-up PR 3.

Mashimiao · 2016-06-28T01:44:22Z

ping @liangchenye , @mrunalp

liangchenye · 2016-06-28T01:48:38Z

LGTM
the --host-specific looks great to me 👍

wking · 2016-06-28T03:21:31Z

validate.go

+	if hostCheck {
+		f, err := os.Open("/proc/filesystems")
+		if err != nil {
+			return supportedTypes, err


This should probably return nil, err. There's not much point in returning an empty map, and the caller will bail when it sees the error anyway.

On 06/28/2016 11:21 AM, W. Trevor King wrote:

This should probably return |nil, err|. There's not much point in returning an empty map, and the caller will bail when it sees the error anyway.
fixed. Thanks.

Ma Shimiao
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)

Signed-off-by: Ma Shimiao <[email protected]>

wking · 2016-06-28T03:38:19Z

Even with my pedantic hat on, I don't see anything else to polish with f1ff4d0 ;). Looks good to me.

Mashimiao · 2016-07-04T01:09:18Z

ping @mrunalp

Mashimiao · 2016-07-13T05:50:57Z

ping @mrunalp

mrunalp · 2016-07-13T22:15:34Z

LGTM

See [1,2]. [1]: opencontainers/runtime-spec#443 (comment) [2]: opencontainers/runtime-tools#119 (comment)

wking reviewed Jun 23, 2016
View reviewed changes

Mashimiao force-pushed the add-mount-type-check branch 2 times, most recently from 8550459 to 26cfdc4 Compare June 24, 2016 06:53

wking reviewed Jun 27, 2016
View reviewed changes

Mashimiao force-pushed the add-mount-type-check branch from 26cfdc4 to 32803b5 Compare June 27, 2016 07:00

wking reviewed Jun 27, 2016
View reviewed changes

Mashimiao force-pushed the add-mount-type-check branch from 32803b5 to 262cdbf Compare June 28, 2016 01:42

wking reviewed Jun 28, 2016
View reviewed changes

validate: add mount type check

f1ff4d0

Signed-off-by: Ma Shimiao <[email protected]>

Mashimiao force-pushed the add-mount-type-check branch from 262cdbf to f1ff4d0 Compare June 28, 2016 03:31

wking mentioned this pull request Jul 9, 2016

validate: only default rlimits and capabilities are recognized #132

Open

mrunalp merged commit 3c402c7 into opencontainers:master Jul 13, 2016

wking added a commit to wking/ccon that referenced this pull request Jul 23, 2016

WIP: ccon-oci: (r)bind handling

e867fc3

See [1,2]. [1]: opencontainers/runtime-spec#443 (comment) [2]: opencontainers/runtime-tools#119 (comment)

wking mentioned this pull request Aug 13, 2016

config: Document 'recursive' and 'bind' mount options extensions opencontainers/runtime-spec#530

Closed

Mashimiao deleted the add-mount-type-check branch November 14, 2016 09:43

wking added a commit to wking/ccon that referenced this pull request Feb 26, 2018

WIP: ccon-oci: (r)bind handling

a50a751

See [1,2]. [1]: opencontainers/runtime-spec#443 (comment) [2]: opencontainers/runtime-tools#119 (comment)

validate: add mount type check #119

validate: add mount type check #119

Conversation

Mashimiao commented Jun 23, 2016

Uh oh!

wking Jun 23, 2016

Choose a reason for hiding this comment

Uh oh!

Mashimiao Jun 24, 2016

Choose a reason for hiding this comment

Uh oh!

wking commented Jun 24, 2016

Uh oh!

wking Jun 27, 2016

Choose a reason for hiding this comment

Uh oh!

wking Jun 27, 2016

Choose a reason for hiding this comment

Uh oh!

Mashimiao Jun 28, 2016

Choose a reason for hiding this comment

Uh oh!

wking commented Jun 27, 2016

Uh oh!

Mashimiao commented Jun 28, 2016

Uh oh!

liangchenye commented Jun 28, 2016

Uh oh!

wking Jun 28, 2016

Choose a reason for hiding this comment

Uh oh!

Mashimiao Jun 28, 2016

Choose a reason for hiding this comment

Uh oh!

wking commented Jun 28, 2016 via email

Uh oh!

Mashimiao commented Jul 4, 2016

Uh oh!

Mashimiao commented Jul 13, 2016

Uh oh!

mrunalp commented Jul 13, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants