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.

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

Jump to bottom

tests: fix some failures reported by oci-runtime-validation #16

Merged
merged 17 commits into from
Mar 8, 2019
Merged

tests: fix some failures reported by oci-runtime-validation #16

merged 17 commits into from
Mar 8, 2019

Conversation

giuseppe
Copy link
Member

No description provided.

mrunalp
mrunalp approved these changes Feb 28, 2019
View reviewed changes
Copy link

@mrunalp mrunalp left a comment

Choose a reason for hiding this comment

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

lgtm

giuseppe added 5 commits March 1, 2019 09:18
@giuseppe
container: reintroduce realpath when path is '.'
874a8ba 
it seems oci-runtime-validation is creating config files where rootfs
is set to '.'.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
linux: change ptmx target to pts/ptmx
9db4ce0 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
cgroup: ignore errors on initializing memory subsystem
58809a6 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
container: fix error message
45a3fcb 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
mount: find bind in the options
d1cb1ab 
the OCI specifications say that "bind" or "rbind" are specified in the
options.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the fix-oci-validation branch from 24989bf to cc263e1 Compare March 1, 2019 13:14
@giuseppe giuseppe force-pushed the fix-oci-validation branch from cc263e1 to 0df41fc Compare March 1, 2019 14:17
@giuseppe
Copy link
Member Author

giuseppe commented Mar 1, 2019

@mrunalp @alban with these changes I get down to 20 failures, which are the same ones I see with runc on Fedora 29.

@alban are the oci-runtime-validation tests supposed to work with runc and get no failures? If so, what environment should I use?

@alban
Copy link
Contributor

alban commented Mar 1, 2019

@giuseppe I haven't tried oci-runtime-validation against runc recently, but last time I tried, there was still some tests that don't pass.

/cc @dongsupark

@dongsupark
Copy link

@alban @giuseppe Exactly.
When I ran oci-runtime-validation against runc (probably 1.0-rc6), I got at least 10 failed tests. I have never been able to get the tests passed.
One of the reasons is that a PR to fix a compliance issue in runc hooks was never merged. AFAIK there could be other compliance issues, which block the final 1.0 release of runc.

@giuseppe
Copy link
Member Author

giuseppe commented Mar 2, 2019

When I ran oci-runtime-validation against runc (probably 1.0-rc6), I got at least 10 failed tests. I have never been able to get the tests passed.

on what system are you running the tests? I've tried several times both on Ubuntu and Fedora and I never manage to get less than 20 failures with runc

@giuseppe
oci: do not assume the process block is present
b636d3e 
This is a quite weird requirement in the OCI specs, but let's make
sure we honor it.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@dongsupark
Copy link

Fedora 29.
Anyway the number of failed tests can vary, so it can be >= 20 on some systems.
So I believe you. :-)

giuseppe added 6 commits March 2, 2019 22:55
@giuseppe
seccomp: skip redundant rules
08229f3 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
container: drop test for process existence
20fec71 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
oci: include container status in hooks stdin
b54d77a 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
oci: include annotations in hooks stdin
53260fc 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
cgroup: use the pids controller for hybrid mode cleanup
fd1928e 
Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe
oci: raise an error if deleting a 'created' container
fbefa09 
this is a weird requirement from the OCI specs, but it makes the
oci-runtime-tools tests pass.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the fix-oci-validation branch from 16a0592 to fbefa09 Compare March 3, 2019 10:15
@giuseppe giuseppe mentioned this pull request Mar 3, 2019
Merged
@giuseppe
Copy link
Member Author

giuseppe commented Mar 3, 2019

I've done some more changes and now crun gets to 0 failures 😃 🎉 🎉 🎉

I needed to fix some tests in runtime-tools. They were probably never really used as they fail with runc. I've opened a PR here: opencontainers/runtime-tools#687

Some of the issues I was seeing were on my end. I've had to change the CPU type for the VM I was using so that 1gb hugetlb can be used and I've needed to add a IDE disk so the major:minor 8:0 is a valid device.

The PR for runtime-tools doesn't affect which is still at 11 failures.

@giuseppe
Copy link
Member Author

giuseppe commented Mar 4, 2019

/cc @vbatts

@vbatts
Copy link
Contributor

vbatts commented Mar 4, 2019

Oh nice

@giuseppe giuseppe force-pushed the fix-oci-validation branch from 2b49e20 to d9e3a43 Compare March 6, 2019 21:37
@giuseppe giuseppe force-pushed the fix-oci-validation branch 8 times, most recently from 4421727 to 268b435 Compare March 7, 2019 08:15
@giuseppe giuseppe force-pushed the fix-oci-validation branch from 268b435 to 30def0a Compare March 7, 2019 09:32
vbatts
vbatts approved these changes Mar 7, 2019
View reviewed changes
Copy link
Contributor

@vbatts vbatts left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe
Copy link
Member Author

giuseppe commented Mar 7, 2019

yey tests are happy! Now I've enabled the oci-validation as part of the PR merge tests. Except some tests that fail or are flaky on Travis (but I promise they work on a F29 VM!)

@giuseppe giuseppe merged commit 30def0a into master Mar 8, 2019
@giuseppe giuseppe deleted the fix-oci-validation branch March 8, 2019 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrunalp mrunalp mrunalp approved these changes

@vbatts vbatts vbatts approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@giuseppe @alban @dongsupark @vbatts @mrunalp