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

Refactor Directory structure #694

Merged
merged 15 commits into from Mar 9, 2022

Conversation

YJDoc2
Copy link
Collaborator

@YJDoc2 YJDoc2 commented Feb 7, 2022

Apologies for the big PR, but major of the changes are moved files.
This PR does some needed refactoring of directory structure, which will make it easier to understand and work on youki.

Major Changelog :

  • created a dir integration-tests which will hold all integration tests, including oci-tests and its rust port, and later containerd etc.
  • moved rust-integration-tests, runtimetest and test_framework from /crates to integration_tests/rust-integration-tests, as those do not belong to youki crates, and are better fit in the integration tests
  • added scripts dir, where all the scripts including build script and script to run the integration tests will reside, this will make it easier to modify and run tests and build from a single point
  • moved the oci-tests submodule from /integration_test to /integration_tests/oci-runtime-tests

This PR will form the first step for #601 , and does not complete it. I made this PR, as even by itself it is quite big, and I did not want to make the review more hard. After this is merged, the next steps in separate PRs will be :

  • Improve scripts and makefiles . This PR adds scripts and makefiles just to make it work with CI. They are still a bit cumbersome to use manually. I will make another PR which should make it easier to manually use.
  • Update the docs. Even though no code is changed, the docs need to be updated for new directory structure and scripts. This will also need its own separate PR.

When testing on my own fork, I noticed that the oci-integration-tests were failing, but not because of changes I made, but because one of the go tests was failing. Adding its link here, in case the same happens in this PR : https://github.com/YJDoc2/youki/runs/5091082926?check_suite_focus=true

@YJDoc2 YJDoc2 requested a review from utam0k February 7, 2022 10:19
@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Feb 7, 2022

@utam0k Apologies for such a big PR 😓 😓 , but most are moved files. Take a look whenever possible for you, and let me know if any changes are required.

Thanks :)

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Feb 7, 2022

It appears that the same issue is also failing this CI , with same 3 capabilites :

...
not ok 53 - unexpected bounding capability CAP_PERFMON not set
not ok 54 - unexpected bounding capability CAP_BPF not set
not ok 55 - unexpected bounding capability CAP_CHECKPOINT_RESTORE not set
ok 56 - expected effective capability CAP_CHOWN set
...
ok 93 - unexpected effective capability CAP_AUDIT_READ not set
not ok 94 - unexpected effective capability CAP_PERFMON not set
not ok 95 - unexpected effective capability CAP_BPF not set
not ok 96 - unexpected effective capability CAP_CHECKPOINT_RESTORE not set
ok 97 - expected inheritable capability CAP_CHOWN set
...
ok 175 - unexpected permitted capability CAP_AUDIT_READ not set
not ok 176 - unexpected permitted capability CAP_PERFMON not set
not ok 177 - unexpected permitted capability CAP_BPF not set
not ok 178 - unexpected permitted capability CAP_CHECKPOINT_RESTORE not set
ok 179 - expected ambient capability CAP_CHOWN set
...

@utam0k
Copy link
Member

utam0k commented Feb 9, 2022

I'll check this PR next my holiday! But it seems this PR has conflicts. Can I ask you to solve it?

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Feb 9, 2022

Hey, no issues, take your time :)
I am working on why the integration tests are failing, after that I'll also solve the conflicts, so if there are any other commits by then and other conflicts, those will get solved as well.
Thanks :)

@utam0k
Copy link
Member

utam0k commented Feb 12, 2022

It would be nice if the make command could work from the top of the directory, is that possible?

@YJDoc2 YJDoc2 force-pushed the improvement/refactor-dir-structure branch from c0946b1 to 8a8cc29 Compare February 23, 2022 12:35
@YJDoc2 YJDoc2 force-pushed the improvement/refactor-dir-structure branch from 063d1c6 to 40e7b74 Compare February 23, 2022 13:02
@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Feb 23, 2022

Hey @utam0k finally the CI is done 😅
Please take a look :)

@utam0k
Copy link
Member

utam0k commented Feb 25, 2022

@YJDoc2 Can I ask you to update the README and resolve conflicts since the path to build.sh has changed?

@YJDoc2 YJDoc2 force-pushed the improvement/refactor-dir-structure branch from de67a26 to 9f7ceb5 Compare February 25, 2022 15:35
@YJDoc2 YJDoc2 mentioned this pull request Feb 25, 2022
@YJDoc2 YJDoc2 force-pushed the improvement/refactor-dir-structure branch from 9f7ceb5 to b2ec0d9 Compare February 25, 2022 15:52
@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Feb 26, 2022

@utam0k please take a look :)
I feel even though this does complete directory refactor, the commands to build and run tests and youki can use a lot of improvements, as in this PR, they are pretty hard to use. After this I'll be working on that.

@utam0k
Copy link
Member

utam0k commented Feb 26, 2022

@utam0k please take a look :)
I feel even though this does complete directory refactor, the commands to build and run tests and youki can use a lot of improvements, as in this PR, they are pretty hard to use. After this I'll be working on that.

@YJDoc2 Can I ask you to update this?
https://github.com/containers/youki#build

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Mar 5, 2022

Hey @utam0k I have updated the readme and docs tutorial, please check
Also Currently there are conflicts, but what I feel is :
If we merge this one first, then there will be a lot of issues for PRs which are open like #758 etc.
I do not mind waiting till they are merged, and then resolving conflicts on my side.
Let me know 👍

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Mar 9, 2022

Hey, the conflicts are resolved, the CI is passing, and currently there are no other PRs open!
Please take a look 👍 😄

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

💯

@utam0k utam0k merged commit 01ba43b into containers:main Mar 9, 2022
@YJDoc2 YJDoc2 deleted the improvement/refactor-dir-structure branch October 7, 2022 05:00
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.

None yet

2 participants