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

runtimetest:add platform validation #15

Closed
wants to merge 1 commit into from

Conversation

wangkirin
Copy link
Contributor

add platform validation function in runtimetest
compare user defined in config.json with runtime parameters to determine if the test pass

Signed-off-by: Wang Qilin qilin.wang@huawei.com

Signed-off-by: Wang Qilin <qilin.wang@huawei.com>
@wking
Copy link
Contributor

wking commented Jan 25, 2016

On Mon, Jan 25, 2016 at 03:25:46AM -0800, Wang Qilin wrote:

add platform validation function in runtimetest

It looks like this test (as of 7be5036) is raising an error if the
configured platform doesn't match the current platform. However,
‘runtimetest’ isn't about validating bundles, it's about validating
runtimes (as I understand it anyway). So the test should actually be
ensuring:

When passed a config specifying a different platform (e.g. an amd64
config on an x86 system) that the runtime exits with an error.

Although neither 1 nor the inflight opencontainers/runtime-spec#225 2 has
wording specified behavior in this situation. Maybe we need a specs
PR first to clarify that?

@mrunalp
Copy link
Contributor

mrunalp commented Jan 26, 2016

@wangkirin Yep, agree with @wking

@zenlint
Copy link
Contributor

zenlint commented Jan 30, 2016

@wking +1.

@mrunalp
Copy link
Contributor

mrunalp commented Mar 8, 2016

@wangkirin Could you please update the PR per the feedback?

@wangkirin
Copy link
Contributor Author

@wking @mrunalp @zenlinTechnofreak Sorry for being late !
Inspired by discussions above , I think we can expand this issue to two aspects.

Firtst, When passed a config specifying a platform different from the runtime(in other words container engine like docker,rkt) is running on ,whether the runtime should exit with an error . If the answer is yes ,may be it's out of the scope of runtimetest because it didn't actually began to run. And we should find a way to capture the error info because it should come from runtime itself ,not the runtimetest program.

Second, If we allow the container begin to run even the platform specified in the config.json is different from the runtime( like current runc) . I think the runtime should give a hint and should be captured by related runtime test program.

And I also agree with @wking , we should have further definition in specs, may be not only in the platform-specific-configuration section , but also in the lifecycle section.

So I suggest pending this PR

@wking
Copy link
Contributor

wking commented Mar 8, 2016

On Tue, Mar 08, 2016 at 05:13:36AM -0800, Wang Qilin wrote:

When passed a config specifying a platform different from the
runtime``(in other wordscontainer enginelike docker,rkt) is running on ,whether the runtime should exit with an error . If the answer is yes ,may be it's out of the scope ofruntimetestbecause it didn't actually began to run. And we should find a way to capture the error info because it should come fromruntime itself ,not the
runtimetest program.

+1 to this. There are a number of places where the runtime can die
before the user-specified process is executed (e.g. pre-start hooks
failing). Using a shell-based test harness (runC may be using Bats
[1,2]) would make testing that sort of thing easy.

However, the spec doesn't currently get into how errors are exposed at
all 3, and until we address that there's no grounds for compliance
testing this behavior.

 Subject: Re: runc integration testcases
 Date: Thu, 21 Jan 2016 17:00:20 -0800
 Message-ID: <CAEu77a3rY_SyyNk1_rA0tngA0=E0L1FF_-iG=TEkNwsMYJ0EHw@mail.gmail.com>

wking pushed a commit to wking/ocitools-v2 that referenced this pull request Nov 17, 2016
…releases-docs

Add governance and releases docs
@Mashimiao
Copy link

The SPEC says "The runtime MUST generate an error if it does not support the specified os."
If os or arch doesn't match we expect in configuration and platform validation is executed, then we can confirm the runtime did not generate an error as expected.
Though error message is not suitable, I think this validation is generally correct. we can make it move on.
How do you think about it? @wangkirin @mrunalp @wking @liangchenye @hqhq

@wking
Copy link
Contributor

wking commented Mar 1, 2017 via email

@Mashimiao
Copy link

@wking Agree with you, putting platform validation into the validation suite will be better.

@Mashimiao Mashimiao closed this Mar 6, 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.

5 participants