-
Notifications
You must be signed in to change notification settings - Fork 201
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
ci: split storage ci jobs for root and non-root testing #276
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
from
July 5, 2022 15:32
f1e9391
to
85cf193
Compare
nirs
requested changes
Jul 5, 2022
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
from
July 6, 2022 08:43
85cf193
to
a0cda98
Compare
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
2 times, most recently
from
July 6, 2022 16:18
e43c03a
to
4f6e74a
Compare
nirs
requested changes
Jul 6, 2022
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
from
July 7, 2022 07:18
4f6e74a
to
3ea9edf
Compare
I still do not follow why do you need any differentiation for CI. Do we need to run twice, once as vdsm and second time as root? For local you should use the same way as CI, I do not see any value in non-containerized runs. Anyone can run a container today |
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
3 times, most recently
from
July 8, 2022 08:46
6a47a06
to
4bee7a1
Compare
Mark all storage tests that require root with the root label so that they get correctly picked or excluded up by the CI jobs. Signed-off-by: Albert Esteve <[email protected]>
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
from
July 14, 2022 07:11
4bee7a1
to
1b937a6
Compare
nirs
requested changes
Jul 14, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Add tox environments for storage user and root tests (i.e., storage-user and storage-root, respectively). These environments are a subset of the storage environment, that is split considering the root mark. This way, we can easily run tests marked as as a root user, and tests marked as non-root, separately. The new environments have their own different target coverage. The 'storage-root' environment sets a different coverage file and folder to avoid overwriting other environment's coverage reports with root ownership. Signed-off-by: Albert Esteve <[email protected]>
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
2 times, most recently
from
July 15, 2022 09:33
82cef1a
to
f4ffdd1
Compare
nirs
requested changes
Jul 15, 2022
Add targets for running tox in the storage-user and storage-root environments. Signed-off-by: Albert Esteve <[email protected]>
nirs
reviewed
Jul 15, 2022
Most of vdsm code run as vdsm:kvm, but vdsm tests run as root by default in the CI, causing several problems, e.g.: - Different behaviour, when run locally and the CI - Some tests are skipped as root We can separate the tests in two different jobs: - storage-tests: run test marked as "not root". It still requires the container to be privileged for setting up the storage. - storage-tests-root: run tests marked as "root". Fixes: oVirt#128 Signed-off-by: Albert Esteve <[email protected]>
Remove tox storage environment as it is not used in the CIs anymore. Consequently, remove also the associated make target 'tests-storage'. Signed-off-by: Albert Esteve <[email protected]>
aesteve-rh
force-pushed
the
update-ci-non-root-testing
branch
from
July 15, 2022 12:44
f4ffdd1
to
9656966
Compare
nirs
approved these changes
Jul 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split storage CI jobs between unprivileged
vdsm
user tests (mark non-root) and root tests.Comparison
With the numbers below we end up executing 50 more tests that were skipped before and safe more than 2 minutes in execution time.
Numbers are not averaged, only the last run is considered.
Before
All storage tests
collected 2702 items / 101 deselected / 2601 selected
Required test coverage of 68% reached. Total coverage: 68.87%
= 2472 passed, 123 skipped, 101 deselected, 5 xfailed, 1 xpassed, 1 warning in 371.31s (0:06:11) =
After
Non-root tests
collected 2700 items / 235 deselected / 2465 selected
Required test coverage of 61% reached. Total coverage: 61.87%
= 2402 passed, 59 skipped, 235 deselected, 4 xfailed, 1 warning in 164.30s (0:02:44) =
Root tests
collected 2700 items / 2566 deselected / 134 selected
Required test coverage of 47% reached. Total coverage: 47.76%
= 120 passed, 12 skipped, 2566 deselected, 1 xfailed, 1 xpassed, 1 warning in 211.60s (0:03:31) =
Total
= 2522 passed, 71 skipped, 5 xfailed, 1 xpassed, in max time 211.60s (0:03:31) =
Fixes: #128
Signed-off-by: Albert Esteve [email protected]