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

Decide which runc to use for validating the rust oci test validation #2616

Closed
YJDoc2 opened this issue Jan 4, 2024 · 8 comments · Fixed by #2647
Closed

Decide which runc to use for validating the rust oci test validation #2616

YJDoc2 opened this issue Jan 4, 2024 · 8 comments · Fixed by #2647

Comments

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Jan 4, 2024

Even though PRs for features such as domain name and sched_entity are merged in rnuc's main branch, no release till now contains the support for them. That is causing causing block for merging #1544 and #2495 , as the rust oci tests fail on runc's released version. When tested on runc compiled from main branch's latest commit, the tests are passing, as the supporting code is present.

We can take two approaches for solving this :

  1. In the CI, clone runc and build from main instead of using the released versions. That way we will get all latest features supported by runc, but then we would be validating our tests on latest runc instead of the released versions, also issues with potential regressions that might get in runc itself. We can pin by runc commit.
  2. Do a more complex scheme of tests, where we either ignore the un-supported tests or validate that the tests are failing on runc, and consider that failure as success for runc. That way we can track what features we support not yet supported by runc etc. This would require a bit complex cfg based or feature based building and some code/macros to appropriately validate tests. Also would make our tests more tailored to youki and runc instead of generic oci adhering runtime.

I would like to take one approach or the other soon, rather than waiting for runc to release the features, as both PR have been pending for long time only due to the runc validation failing.

cc @utam0k

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Jan 9, 2024

@jprendes , if possible can you take a look at this and let me know your thoughts?

@jprendes
Copy link
Contributor

jprendes commented Jan 9, 2024

Hi!

I'm no expert in runc development, but IIUC there was a runc release last week
And there was another one ~2 months ago

None of those have the features from main?

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Jan 9, 2024

None of those have the features from main?

Hey it would appear so. For #1544 the changes were merged >5 months back, and the changes are not in the latest release. I tested manual build of master, which did pass the tests.

@utam0k
Copy link
Member

utam0k commented Jan 9, 2024

runc v1.2.0 seems to plan to include it.
opencontainers/runc#3963

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Jan 12, 2024

runc v1.2.0 seems to plan to include it.
opencontainers/runc#3963

Hey @utam0k , I didn't know that. However, seeing that issue ti seems that the release would still take quite some time, and I would prefer not to keep the PRs blocked till then. If you don't want to use main branch of runc for testing we can take an approach similar to sudo-rs testing.

@utam0k
Copy link
Member

utam0k commented Jan 12, 2024

@YJDoc2 Make sense a lot 👍

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Jan 12, 2024

@YJDoc2 Make sense a lot 👍

I'll go ahead and see if I can do a conditional ignore of the tests similar to sudo-rs testing, otherwise we can switch over to the main branch of runc. 👍

@utam0k
Copy link
Member

utam0k commented Jan 13, 2024

FYI: contained ignores some unit tests when the runtime, except for runc, is used. It is judged by ENV.
https://github.com/containers/youki/pull/2558/files#diff-a2de7550554c0198ac7f959c78a19ee0996921c98034411de47a7dd49b0c209bR79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants