tests,ci: Move "test-basic" (bare mode) to installed test #1217
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
Our CI uses default Docker, which has SELinux labeling but is rather evil in returning `EOPNOTSUPP` to any attempts to set `security.selinux`, even if to the same value. The previous fire 🔥 for this was: ostreedev#759 The `bare` repo mode really only makes sense as uid 0, so our installed test framework is a good match for this. However, the unit tests *do* work in a privileged container even as non-root, and *also* should work on SELinux-disabled systems. So let's teach the test framework how to skip in those situations. I tested this both in a priv container (my default builder) and an unpriv container (like our CI). At the same time, start executing the `test-basic.sh` from an installed test, so we get better coverage than before. This is just the start - all of the sysroot tests really need the same treatment.
|
Migrated from #1199 |
|
OK, this should be ready for review. |
jlebon
reviewed
Sep 26, 2017
tests/libtest.sh
Outdated
| # Skip unless SELinux is disabled, or we can relabel. | ||
| # Default Docker has security.selinux xattrs, but returns | ||
| # EOPNOTSUPP when trying to set them, even to the existing value. | ||
| # https://github.com/ostreedev/ostree/pull/1182 |
There was a problem hiding this comment.
Is this the right link? I don't see anything about that there.
| label=$(grep -E -e "^${selinux_xattr}=" < label.txt |sed -e "s,${selinux_xattr}=,,") | ||
| if setfattr -n ${selinux_xattr} -v ${label} testlabel.txt 2>err.txt; then | ||
| echo "SELinux enabled in $(pwd), and have privileges to relabel" | ||
| return 0 |
There was a problem hiding this comment.
We should cache the result of this here too just like the others, though we can do that later!
There was a problem hiding this comment.
Possibly, though right now it's a file-global skip unlike the other skip_ones.
|
☀️ Test successful - status-atomicjenkins |
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.
Our CI uses default Docker, which has SELinux labeling but is rather
evil in returning
EOPNOTSUPPto any attempts to setsecurity.selinux,even if to the same value.
The previous fire 🔥 for this was: #759
The
barerepo mode really only makes sense as uid 0, so our installedtest framework is a good match for this. However, the unit tests do
work in a privileged container even as non-root, and also should
work on SELinux-disabled systems. So let's teach the test framework
how to skip in those situations.
I tested this both in a priv container (my default builder) and an unpriv
container (like our CI).
At the same time, start executing the
test-basic.shfrom an installed test,so we get better coverage than before.
This is just the start - all of the sysroot tests really need the
same treatment.