-
Notifications
You must be signed in to change notification settings - Fork 143
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
validate: add mount type check #119
Conversation
} | ||
} | ||
|
||
supportedTypes["nfs"] = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see an nfs
entry in my /proc/filesystems
(and an nfs4
entry). Why are you adding it by hand here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it seems it's really no need to add.
Also related to this and the ‘bind’ type is that mount.type might |
8550459
to
26cfdc4
Compare
@@ -21,6 +21,9 @@ Validate an OCI bundle | |||
**--hooks** | |||
Check specified hooks exist and are executable on the host. | |||
|
|||
**--host-specific** | |||
Check host specified configs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“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").
26cfdc4
to
32803b5
Compare
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"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: there's trailing whitespace here (in case we care in this repository; I don't if we do).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Thanks.
32803b5
to
262cdbf
Compare
ping @liangchenye , @mrunalp |
LGTM |
if hostCheck { | ||
f, err := os.Open("/proc/filesystems") | ||
if err != nil { | ||
return supportedTypes, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <mashimiao.fnst@cn.fujitsu.com>
262cdbf
to
f1ff4d0
Compare
Even with my pedantic hat on, I don't see anything else to polish with
f1ff4d0 ;). Looks good to me.
|
ping @mrunalp |
ping @mrunalp |
LGTM |
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com