Skip to content

Conversation

@mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jul 18, 2016

Signed-off-by: Mrunal Patel [email protected]

test_runtime.sh Outdated

pushd $TESTDIR > /dev/null
ocitools generate --output config.json "${TEST_ARGS[@]}" --rootfs '.'
ocitools generate --tty --output config.json "${TEST_ARGS[@]}" --rootfs '.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Drive by?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/dev/console is only setup if we set terminal to true.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Mon, Jul 18, 2016 at 03:41:25PM -0700, Mrunal Patel wrote:

-ocitools generate --output config.json "${TEST_ARGS[@]}" --rootfs '.'
+ocitools generate --tty --output config.json "${TEST_ARGS[@]}" --rootfs '.'

/dev/console is only setup if we set terminal to true.

Can we stop adding “runC isn't compliant out of the box” workarounds
now that we're post 1.0.0-rc1? If there's really a reason to not
setup /dev/console when terminal is false, can we PR the spec with
that information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wking I have created a runtime-spec PR opencontainers/runtime-spec#518

Copy link
Contributor

Choose a reason for hiding this comment

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

On Mon, Jul 18, 2016 at 03:54:39PM -0700, Mrunal Patel wrote:

@@ -78,7 +78,7 @@ tar -xf rootfs.tar.gz -C ${TESTDIR}
cp runtimetest ${TESTDIR}

pushd $TESTDIR > /dev/null
-ocitools generate --output config.json "${TEST_ARGS[@]}" --rootfs '.'
+ocitools generate --tty --output config.json "${TEST_ARGS[@]}" --rootfs '.'

@wking I have created a runtime-spec PR opencontainers/runtime-spec#518

+1. In the meantime, can we land this PR without this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have taken out that change for now pending upstream PR.

@wking
Copy link
Contributor

wking commented Jul 18, 2016 via email

@mrunalp mrunalp force-pushed the fix_devices_check branch from 7fd96c1 to 6f9de0a Compare July 18, 2016 23:51
@wking
Copy link
Contributor

wking commented Jul 19, 2016

6f9de0a looks good to me.

The --tty bit should be easy to add back in if we still need it after
opencontainers/runtime-spec#518 gets sorted.

@mrunalp mrunalp merged commit e05b847 into opencontainers:master Jul 19, 2016
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.

2 participants