Skip to content

fix issue #228 replace start to run #232

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.

Sign up for GitHub

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

Closed
heartlock wants to merge 2 commits into from
Closed

fix issue #228 replace start to run #232

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88eb7de
replace start to run
heartlock Oct 7, 2016
0f2fdf6
replace start to create-start
heartlock Oct 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion test_runtime.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,17 @@ cp runtimetest ${TESTDIR}

oci-runtime-tool generate --output "${TESTDIR}/config.json" "${TEST_ARGS[@]}" --rootfs '.'

TESTCMD="${RUNTIME} start $(uuidgen)"
conID=$(uuidgen)

CREATECMD="${RUNTIME} create ${conID}"
TESTCMD="${RUNTIME} start ${conID}"
pushd $TESTDIR > /dev/null
if ! ${CREATECMD}; then
info "Runtime is ${RUNTIME}, no need to create before start"
else
info "Runtime is ${RUNTIME}, need to create before start"
fi

@Mashimiao Mashimiao Oct 8, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@heartlock heartlock Oct 8, 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.

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}".

@Mashimiao Mashimiao Oct 8, 2016 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

when executed "${RUNTIME} create ${conID}", you can use $? to get the exit code.
just as
sleep 1
echo $?
0

@heartlock heartlock Oct 9, 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.

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

@Mashimiao Mashimiao Oct 9, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If necessary, I think we can remove it.

@heartlock heartlock Oct 9, 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.

I think it is unwise to remove it.

@Mashimiao Mashimiao Oct 9, 2016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In my opinion, just now it's not necessary to remove. You can just error out if failed and continue when created succeed.

@wking wking Oct 10, 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.

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.


if ! ${TESTCMD}; then
error "Runtime ${RUNTIME} failed validation"
else
Expand Down