Uh oh!
There was an error while loading. Please reload this page.
fix issue #228 replace start to run - #232
Conversation
Signed-off-by: heartlock <21521209@zju.edu.cn>
Mashimiao
commented
Oct 7, 2016
This has been talked in #184. I think you'd better separate it into create and start. |
heartlock
commented
Oct 7, 2016
@Mashimiao good idea, I will update it. but I think when the work of issuse #66 is complete, test_runtime.sh will be removed. I don't know if it is necessary to update it here. |
Mashimiao
commented
Oct 7, 2016
I think it is, as test_runtime.sh is the only runtime test tool by now. |
Signed-off-by: heartlock <21521209@zju.edu.cn>
| info "Runtime is ${RUNTIME}, no need to create before start" | ||
| else | ||
| info "Runtime is ${RUNTIME}, need to create before start" | ||
| fi |
There was a problem hiding this comment.
If created succeed, we don't need to print. But if failed, we should judge to stop or continue depend on which kind of error.
There was a problem hiding this comment.
when exitcode of command "${RUNTIME} create ${conID}" is 1, runtime has no command "create" continue to run command "start". If exitcode of command "${RUNTIME} create ${conID}" is -1, runtime create container fail and print error. but I don't know how to get exitcode of command "${RUNTIME} create ${conID}".
There was a problem hiding this comment.
when executed "${RUNTIME} create ${conID}", you can use $? to get the exit code.
just as
sleep 1
echo $?
0
There was a problem hiding this comment.
but if runtime has no command "create", execute "${RUNTIME} create ${conID}" will cause a crash.https://github.com/opencontainers/runtime-tools/blob/master/test_runtime.sh#L2
There was a problem hiding this comment.
I think it is unwise to remove it.
There was a problem hiding this comment.
In my opinion, just now it's not necessary to remove. You can just error out if failed and continue when created succeed.
There was a problem hiding this comment.
On Sat, Oct 08, 2016 at 07:05:08PM -0700, heartlock wrote:
but if runtime has no command "create"…
Then it's not compliant with opencontainers/runtime-spec#513 or the current create requirements. You can use:
${RUNTIME} create ${conID} || die "failed to create ${conID}"
${RUNTIME} start ${conID} || die "failed to start ${conID}"
with a die command that prints the message to stderr and exits 1.
But the real problem with converting test_runtime.sh to the spec's current create/start API is that the start exit code is not the container process's exit code. You have to do subreaper magic or similar to see how runtimetest exited.
Mashimiao
commented
Oct 8, 2016
And there is no need two commits, just one commit will be fine. |
Mashimiao
commented
Nov 30, 2016
carried in #285 |
Signed-off-by: heartlock 21521209@zju.edu.cn