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

ci: don't run tests on overlayfs/tmpfs #1199

Closed
wants to merge 6 commits into from
Closed

ci: don't run tests on overlayfs/tmpfs #1199

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eb5d690
tests: allow specifying tmpdir
jlebon Sep 20, 2017
f5a389a
tests/libtest: check that we have setfattr
jlebon Sep 20, 2017
87f018d
ci: don't run tests on overlayfs/tmpfs
jlebon Sep 20, 2017
79295f6
fixup! ci: don't run tests on overlayfs/tmpfs
jlebon Sep 21, 2017
e05d699
fixup! ci: don't run tests on overlayfs/tmpfs
jlebon Sep 21, 2017
25769d4
fixup! ci: don't run tests on overlayfs/tmpfs
jlebon Sep 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions .papr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ branches:
required: true
context: f26-primary

container:
image: registry.fedoraproject.org/fedora:26
host:
distro: fedora/26/atomic

env:
# Enable all the sanitizers for this primary build.
Expand All @@ -17,11 +17,15 @@ env:
GI_SCANNERFLAGS: '--warn-error'
ASAN_OPTIONS: 'detect_leaks=0' # Right now we're not fully clean, but this gets us use-after-free etc
# TODO when we're doing leak checks: G_SLICE: "always-malloc"
TEST_TMPDIR: /srv

tests:
- ci/ci-commitmessage-submodules.sh
- ci/build-check.sh
- ci/ci-release-build.sh
- docker run -e TEST_TMPDIR -e CFLAGS -e GI_SCANNERFLAGS -e ASAN_OPTIONS
--privileged -v $PWD:$PWD --workdir $PWD -v /srv:/srv
Copy link
Member

Choose a reason for hiding this comment

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

There's nothing for this change that requires --privileged right? I suspect it's just SELinux labeling that we'd hit; relabeling with chcon -R -h -t container_file_t $PWD and doing the same with TEST_TMPDIR would let us avoid being privileged, and that in turn would help guarantee these tests can be run in Kube for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I added a fixup to try that out. ⬇️

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, hmm, we're still hitting

error: lsetxattr: Permission denied

when trying to commit files (with a relabelfrom SELinux violation).

I suspect it's just SELinux labeling that we'd hit; relabeling with chcon -R -h -t container_file_t $PWD and doing the same with TEST_TMPDIR would let us avoid being privileged,

But we'll still be trying to re-apply all the labels when committing to a bare repo, right? Unless we teach libglnx to only update security.selinux if the label isn't already set to the wanted value, though that's quite a performance hit. We could hide the behaviour behind an env var, I guess? We could also put back --privileged (or at least --security-opt label:disable) and add another testsuite to make sure the in-container path doesn't break?

registry.fedoraproject.org/fedora:26 sh -c
'ci/ci-commitmessage-submodules.sh &&
ci/build-check.sh &&
ci/ci-release-build.sh'

timeout: 30m

Expand All @@ -38,13 +42,13 @@ host:
distro: centos/7/atomic

env:
CFLAGS: ''
TEST_TMPDIR: /srv

tests:
# FIXME revert setting to 7/3/1611 when repos sync
# https://github.com/projectatomic/rpm-ostree/pull/985
- docker run --privileged -v $PWD:$PWD --workdir $PWD
registry.centos.org/centos/centos:7.3.1611 sh -c
- docker run -e TEST_TMPDIR --privileged -v $PWD:$PWD --workdir $PWD
-v /srv:/srv registry.centos.org/centos/centos:7.3.1611 sh -c
'yum install -y git && ci/build-check.sh'

---
Expand Down
3 changes: 2 additions & 1 deletion buildutil/tap-test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

srcd=$(cd $(dirname $1) && pwd)
bn=$(basename $1)
tempdir=$(mktemp -d /var/tmp/tap-test.XXXXXX)
TEST_TMPDIR=${TEST_TMPDIR:-/var/tmp}
tempdir=$(mktemp -d $TEST_TMPDIR/tap-test.XXXXXX)
touch ${tempdir}/.testtmp
function cleanup () {
if test -f ${tempdir}/.testtmp; then
Expand Down
6 changes: 4 additions & 2 deletions tests/libostreetest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ ot_test_setup_sysroot (GCancellable *cancellable,
if (!ot_test_run_libtest ("setup_os_repository \"archive\" \"syslinux\"", error))
return FALSE;

/* Keep this in sync with the overlayfs bits in libtest.sh */
struct statfs stbuf;
{ g_autoptr(GString) buf = g_string_new ("mutable-deployments");
if (statfs ("/", &stbuf) < 0)
const char *pwd = g_getenv ("PWD");
g_assert (pwd);
if (statfs (pwd, &stbuf) < 0)
return glnx_null_throw_errno (error);
/* Keep this in sync with the overlayfs bits in libtest.sh */
#ifndef OVERLAYFS_SUPER_MAGIC
#define OVERLAYFS_SUPER_MAGIC 0x794c7630
#endif
Expand Down
5 changes: 4 additions & 1 deletion tests/libtest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export OSTREE_GPG_HOME=${test_tmpdir}/gpghome/trusted
# See comment in ot-builtin-commit.c and https://github.com/ostreedev/ostree/issues/758
# Also keep this in sync with the bits in libostreetest.c
echo evaluating for overlayfs...
case $(stat -f --printf '%T' /) in
case $(stat -f --printf '%T' $PWD) in
overlayfs)
echo "overlayfs found; enabling OSTREE_NO_XATTRS"
export OSTREE_SYSROOT_DEBUG="${OSTREE_SYSROOT_DEBUG},no-xattrs"
Expand Down Expand Up @@ -517,6 +517,9 @@ os_repository_new_commit ()
# Usage: if ! skip_one_without_user_xattrs; then ... more tests ...; fi
_have_user_xattrs=''
have_user_xattrs() {
if ! which setfattr 2>/dev/null; then
fatal "no setfattr available to determine xattr support"
fi
if test "${_have_user_xattrs}" = ''; then
touch test-xattrs
if setfattr -n user.testvalue -v somevalue test-xattrs 2>/dev/null; then
Expand Down