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

libcontainer/specconv/spec_linux: Add support for (no)lazytime #1460

Merged
merged 1 commit into from
Jun 29, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented May 26, 2017

Part of catching runC up with the spec, which punts valid options to mount(8). Part of opencontainers/runtime-spec#771.

@wking
Copy link
Contributor Author

wking commented May 26, 2017

Travis failure should be resolved by #1464. I'll rebase this PR on top of master if/when #1464 lands.

@wking
Copy link
Contributor Author

wking commented May 26, 2017

Updated with 9707d1dcdfb37e as requested by @cyphar. I'm still not sure if he wants me to pull in #1464 as well.

@wking
Copy link
Contributor Author

wking commented May 30, 2017

Rebased onto master with cdfb37ea57e41f to pick up #1464.

@wking
Copy link
Contributor Author

wking commented May 30, 2017

The current Travis failure is a checkpoint issue. I saw a similar checkpoint issue on a different Go version in #1463, but I'm pretty sure this is unrelated to my PR because master recently failed with the same issue.

@hqhq
Copy link
Contributor

hqhq commented Jun 2, 2017

Can you squash? It's a whole caching up for mount options and don't need to be a separate commit for each entry.

And also silent, loud, (no)iversion, and (no)acl.  This is part of
catching runC up with the spec, which punts valid options to mount(8)
[1,2].

(no)acl is a filesystem-specific entry in mount(8), but it's
represented by a MS_* flag in mount(2) so we need an entry in the
translation table.

[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0-rc5/config.md#L68
[2]: opencontainers/runtime-spec#771

Signed-off-by: W. Trevor King <[email protected]>
@wking wking force-pushed the mount-option-lazytime branch from a57e41f to 4f81337 Compare June 2, 2017 03:44
@wking
Copy link
Contributor Author

wking commented Jun 2, 2017 via email

@cyphar
Copy link
Member

cyphar commented Jun 2, 2017

Actually, let's drop noacl and rbind but keep the rest. 😉

In all seriousness, I'm worried by how many test failures we are having these days. @avagin do you think this is caused by the dropping of the wait_for_container code in our testing?

@crosbymichael
Copy link
Member

@cyphar why rbind? that is extremely common

@crosbymichael
Copy link
Member

@cyphar
Copy link
Member

cyphar commented Jun 2, 2017

@crosbymichael It was a joke. 😉

@wking
Copy link
Contributor Author

wking commented Jun 20, 2017

Triggering a Travis re-build now that #1484 has landed.

@wking wking reopened this Jun 20, 2017
@wking
Copy link
Contributor Author

wking commented Jun 20, 2017

Still get a Travis error, this time:

not ok 4 checkpoint --pre-dump and restore

apparently from trying to signal a finished process:

# runc list (status=0):
# ID             PID         STATUS      BUNDLE             CREATED                         OWNER
# test_busybox   0           stopped     /tmp/busyboxtest   2017-06-20T19:58:04.67322906Z   root
# runc kill test_busybox KILL (status=1):
# time="2017-06-20T19:58:05Z" level=error msg="container_linux.go:308: signaling init process caused \"os: process already finished\"\n" 
# container_linux.go:308: signaling init process caused "os: process already finished"

I think #1489 may be part of fixing that, possibly with a guard in the bats tests to avoid calling runc kill in the first place on stopped containers. It seems orthogonal to this PR in any case.

@avagin
Copy link
Contributor

avagin commented Jun 20, 2017

In all seriousness, I'm worried by how many test failures we are having these days. @avagin do you think this is caused by the dropping of the wait_for_container code in our testing?

If we remove a useless thing and start catching new bugs, it doesn't mean that the thing was not useless;).

@wking wking closed this Jun 21, 2017
@wking
Copy link
Contributor Author

wking commented Jun 21, 2017

Triggering a Travis re-build now that #1489 has landed.

@wking wking reopened this Jun 21, 2017
@wking
Copy link
Contributor Author

wking commented Jun 21, 2017

And Travis is happy :).

@cyphar
Copy link
Member

cyphar commented Jun 29, 2017

@wking By the way, we can retrigger travis build without closing and reopening PRs. 😉

@cyphar
Copy link
Member

cyphar commented Jun 29, 2017

LGTM.

Approved with PullApprove

@crosbymichael
Copy link
Member

crosbymichael commented Jun 29, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit fef3ace into opencontainers:master Jun 29, 2017
@wking wking deleted the mount-option-lazytime branch August 10, 2017 18:19
@@ -639,6 +639,7 @@ func parseMountOptions(options []string) (int, []int, string, int) {
clear bool
flag int
}{
"acl": {false, unix.MS_POSIXACL},
Copy link
Member

Choose a reason for hiding this comment

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

This implementation might not be correct:

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.

6 participants