Fix the mount tests #2269

asomers · 2023-12-16T19:07:52Z

As originally written by @kamalmarhubi in #231, these tests made clever use of Linux namespaces in order to run as unprivileged users. However, a subsequent kernel bug broke this functionality, and it hasn't been fixed even 6 years later. The tests have been skipped ever since.

Get the tests to run again by removing the namespace stuff and requiring privileges instead. Also, remove the custom test harness. Now Nix will be compatible with tools like cargo-nextest.

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1659087

What does this PR do

Fixes the long-skipped mount tests so they can run again.

Checklist:

I have read CONTRIBUTING.md
I have written necessary tests and rustdoc comments
A change log has been added if this PR modifies nix's API

Cargo.toml

test/test_mount.rs

SteveLauC · 2023-12-20T00:52:26Z

It seems that the -E or --preserve-env option does not work the PATH, I can reproduce the CI behavior on my host. From this answer, we should do:

And this seems to be a ~~feature~~ of the sudo with many Linux distributions, i.e., sudo will change your PATH.

$ sudo env PATH=$PATH cargo test

But it still does not work:

$ sudo env PATH=$PATH cargo test
error: rustup could not choose a version of cargo to run, because one wasn't specified explicitly, and no default is configured.
help: run 'rustup default stable' to download the latest stable release of Rust and set it as your default toolchain.

$ sudo env PATH=$PATH rustup toolchain list
no installed toolchains

SteveLauC · 2023-12-20T01:11:21Z

I installed our MSRV toolchain and ran tests:

$ sudo env PATH=$PATH rustup default 1.69.0
$ sudo env PATH=$PATH cargo t --all-features

...

failures:

---- test_kmod::test_finit_and_delete_module stdout ----
thread 'test_kmod::test_finit_and_delete_module' panicked at 'assertion failed: status.success()', test/test_kmod/mod.rs:29:5

---- sys::test_fanotify::test_fanotify stdout ----
thread 'sys::test_fanotify::test_fanotify' panicked at 'should have read exactly one event', test/sys/test_fanotify.rs:42:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_kmod::test_finit_and_delete_module_with_params stdout ----
thread 'test_kmod::test_finit_and_delete_module_with_params' panicked at 'assertion failed: status.success()', test/test_kmod/mod.rs:29:5

---- test_kmod::test_finit_module_twice_and_delete_module stdout ----
thread 'test_kmod::test_finit_module_twice_and_delete_module' panicked at 'assertion failed: status.success()', test/test_kmod/mod.rs:29:5

---- test_kmod::test_init_and_delete_module stdout ----
thread 'test_kmod::test_init_and_delete_module' panicked at 'assertion failed: status.success()', test/test_kmod/mod.rs:29:5

---- test_kmod::test_init_and_delete_module_with_params stdout ----
thread 'test_kmod::test_init_and_delete_module_with_params' panicked at 'assertion failed: status.success()', test/test_kmod/mod.rs:29:5


failures:
    sys::test_fanotify::test_fanotify
    test_kmod::test_finit_and_delete_module
    test_kmod::test_finit_and_delete_module_with_params
    test_kmod::test_finit_module_twice_and_delete_module
    test_kmod::test_init_and_delete_module
    test_kmod::test_init_and_delete_module_with_params

test result: FAILED. 338 passed; 6 failed; 2 ignored; 0 measured; 0 filtered out; finished in 3.01s

error: test failed, to rerun pass `--test test`

Given that we have failed tests to investigate, we should probably do this in another PR, and let me give it a try since I am on Linux.

For this PR, let's revert the later changes and merge it.

asomers · 2023-12-20T01:18:39Z

I just pushed another attempt: sudo --preserve-env=HOME $(which cargo) test --all-features. But I like yours better. Assuming this fails, I'll revert and open a new PR to investigate the failures.

SteveLauC · 2023-12-20T01:32:15Z

Well, I am surprised that the tests passed in CI but failed on my host...

asomers · 2023-12-20T01:33:30Z

Well, I am surprised that the tests passed in CI but failed on my host...

Me too! It isn't often that we get a good surprise from CI.

SteveLauC · 2023-12-20T01:59:49Z

Can reproduce the CI failure:

$ sudo --preserve-env=HOME $(which cross)
Error:
   0: could not execute `rustc --print target-list`
   1: No such file or directory (os error 2)

Location:
   /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/convert/mod.rs:716

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

And I got cross to run under sudo but no tests were actually executed:

$ sudo env PATH=$PATH cross test powerpc64le-unknown-linux-gnu --all-features

info: downloading component 'rust-src'
info: installing component 'rust-src'
Trying to pull ghcr.io/cross-rs/x86_64-unknown-linux-gnu:main...
Getting image source signatures
Copying blob ceb9f63710c9 done
Copying blob 6c70d1c0e806 done
Copying blob edaedc954fb5 done
Copying blob 8b1d4c95c4a4 done
Copying blob b09634800521 done
Copying blob 3ba6a39e216b done
Copying blob c710eb4c7c4b done
Copying blob da0191c7c407 done
Copying blob 0e1376e6048d done
Copying blob fb5bccee428c done
Copying blob 913a127e772e done
Copying blob 44e47dfe46b9 done
Copying blob 2a23a22cb021 done
Copying blob f204b9544051 done
Copying config 396b402247 done
Writing manifest to image destination
Storing signatures
   Compiling libc v0.2.151
   Compiling cfg-if v1.0.0
   Compiling autocfg v1.1.0
   Compiling proc-macro2 v1.0.67
   Compiling unicode-ident v1.0.12
   Compiling bitflags v2.4.0
   Compiling thiserror v1.0.49
   Compiling parking_lot_core v0.9.8
   Compiling rustix v0.38.15
   Compiling cfg_aliases v0.1.1
   Compiling smallvec v1.11.1
   Compiling semver v1.0.19
   Compiling scopeguard v1.2.0
   Compiling linux-raw-sys v0.4.8
   Compiling ppv-lite86 v0.2.17
   Compiling pin-utils v0.1.0
   Compiling fastrand v2.0.1
   Compiling assert-impl v0.1.3
   Compiling nix v0.27.1 (/home/steve/Documents/workspace/nix-rust/asomers-nix)
   Compiling memoffset v0.9.0
   Compiling lock_api v0.4.10
   Compiling quote v1.0.33
   Compiling syn v2.0.37
   Compiling getrandom v0.2.10
   Compiling rand_core v0.6.4
   Compiling parking_lot v0.12.1
   Compiling rand_chacha v0.3.1
   Compiling rand v0.8.5
   Compiling tempfile v3.8.0
   Compiling thiserror-impl v1.0.49
   Compiling caps v0.5.5
    Finished test [unoptimized + debuginfo] target(s) in 4.97s
     Running unittests src/lib.rs (/target/x86_64-unknown-linux-gnu/debug/deps/nix-3a5b6b3636d126e1)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 19 filtered out; finished in 0.00s

     Running test/test.rs (/target/x86_64-unknown-linux-gnu/debug/deps/test-07416a684411d0f5)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 350 filtered out; finished in 0.00s

     Running test/sys/test_aio_drop.rs (/target/x86_64-unknown-linux-gnu/debug/deps/test_aio_drop-fc74ed8837120ab7)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 0.00s

     Running test/test_clearenv.rs (/target/x86_64-unknown-linux-gnu/debug/deps/test_clearenv-a2a7f9a0496a23c0)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out; finished in 0.00s

     Running test/sys/test_prctl.rs (/target/x86_64-unknown-linux-gnu/debug/deps/test_prctl-2fcdce0c6c6bc918)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 10 filtered out; finished in 0.00s

SteveLauC · 2024-01-15T03:00:23Z

.github/actions/test/action.yml

@@ -5,6 +5,11 @@ inputs:
  TARGET:
    required: true

+  SUDO:
+    description: 'Set to "sudo" to run the build with sudo, or leave undefined to run as the current user'


It seems that the description should be:

Set it to an empty string to run the tests as the current user, leave it with the default value to test with "sudo"

~~Wait, it seems that GHA will treat an empty string as the default value per the CI log?~~

Update: please ignore this, I think my mind was pretty dizzy at that tim.

SteveLauC · 2024-01-15T03:01:06Z

.github/workflows/ci.yml

@@ -84,6 +84,7 @@ jobs:
      - name: build
        uses: ./.github/actions/build
        with:
+          SUDO: ""


The build action does not take a SUDO input so we don't need to specify it here

SteveLauC · 2024-01-15T03:08:06Z

~~Have you tried running these tests with sudo on Cirrus? I am wondering if we will still get the error.~~

Update: sorry, I missed this comment:

For cirrus, I think the tests already run as root, but without a few privileges like CAP_SYS_MODULE. There's probably no easy way to get that privilege.

Well, I am surprised that the tests passed in CI but failed on my host...

And strangely, the tests are still failing on my host, but work fine in CI:(

asomers · 2024-01-15T03:25:02Z

~~Have you tried running these tests with sudo on Cirrus? I am wondering if we will still get the error.~~

Update: sorry, I missed this comment:

For cirrus, I think the tests already run as root, but without a few privileges like CAP_SYS_MODULE. There's probably no easy way to get that privilege.

Well, I am surprised that the tests passed in CI but failed on my host...

And strangely, the tests are still failing on my host, but work fine in CI:(

What errors do you get locally?

SteveLauC · 2024-01-15T03:40:20Z

What errors do you get locally?

failures:

---- test_kmod::test_finit_and_delete_module stdout ----
thread 'test_kmod::test_finit_and_delete_module' panicked at test/test_kmod/mod.rs:29:5:
assertion failed: status.success()

---- sys::test_fanotify::test_fanotify stdout ----
thread 'sys::test_fanotify::test_fanotify' panicked at test/sys/test_fanotify.rs:42:9:
should have read exactly one event
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_kmod::test_finit_and_delete_module_with_params stdout ----
thread 'test_kmod::test_finit_and_delete_module_with_params' panicked at test/test_kmod/mod.rs:29:5:
assertion failed: status.success()

---- test_kmod::test_finit_module_twice_and_delete_module stdout ----
thread 'test_kmod::test_finit_module_twice_and_delete_module' panicked at test/test_kmod/mod.rs:29:5:
assertion failed: status.success()

---- test_kmod::test_init_and_delete_module stdout ----
thread 'test_kmod::test_init_and_delete_module' panicked at test/test_kmod/mod.rs:29:5:
assertion failed: status.success()

---- test_kmod::test_init_and_delete_module_with_params stdout ----
thread 'test_kmod::test_init_and_delete_module_with_params' panicked at test/test_kmod/mod.rs:29:5:
assertion failed: status.success()


failures:
    sys::test_fanotify::test_fanotify
    test_kmod::test_finit_and_delete_module
    test_kmod::test_finit_and_delete_module_with_params
    test_kmod::test_finit_module_twice_and_delete_module
    test_kmod::test_init_and_delete_module
    test_kmod::test_init_and_delete_module_with_params

test result: FAILED. 344 passed; 6 failed; 2 ignored; 0 measured; 0 filtered out; finished in 3.00s

error: test failed, to rerun pass `--test test`

SteveLauC · 2024-01-16T00:57:49Z

The kmod tests failed because I don't have kernel-header installed, debugging that sys::test_fanotify::test_fanotify failure now.

SteveLauC · 2024-01-16T03:31:07Z

The kmod tests failed because I don't have kernel-header installed, debugging that sys::test_fanotify::test_fanotify failure now.

Ok, I think I have found the root cause, this test monitors the whole mount point of /tmp/.xxx, so it could clash with other tests, i.e., we will get notified when other tests access the files under the mount point.

Then why our CI passes, there could be several reasons:

The # of cores in our CI is limited, so we won't get that much of concurrency
Magical scheduling, it just happens to be fine

I will tweak that test a little bit so that such a possible collision won't happen.

SteveLauC · 2024-02-06T07:15:08Z

Seems that we should merge #2294 first to fix the test failure

Update: mreged, rebasing your branch should resolve that test_fanotify CI error

@kamalmarhubi

As originally written by @kamalmarhubi in nix-rust#231, these tests made clever use of Linux namespaces in order to run as unprivileged users. However, a subsequent kernel bug broke this functionality, and it hasn't been fixed even 6 years later. The tests have been skipped ever since. Get the tests to run again by removing the namespace stuff and requiring privileges instead. Also, remove the custom test harness. Now Nix will be compatible with tools like cargo-nextest. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1659087

Now that we aren't using namespaces, they don't need their own program.

Because several of the tests require root privileges. But don't use sudo with cross. It fails due to a known issue upstream: cross-rs/cross#526

asomers force-pushed the nounshare branch 2 times, most recently from 13dde59 to b903378 Compare December 16, 2023 19:41

asomers requested a review from SteveLauC December 16, 2023 19:41

SteveLauC reviewed Dec 17, 2023

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

test/test_mount.rs Outdated Show resolved Hide resolved

asomers force-pushed the nounshare branch 3 times, most recently from aef6dee to 8ed1588 Compare December 19, 2023 23:41

asomers force-pushed the nounshare branch from 5aa9911 to 1dfffca Compare December 20, 2023 01:35

asomers force-pushed the nounshare branch 3 times, most recently from 230030c to c70daf4 Compare January 6, 2024 22:39

asomers force-pushed the nounshare branch from c70daf4 to 1d5aff2 Compare January 14, 2024 16:47

asomers requested a review from SteveLauC January 14, 2024 16:48

SteveLauC requested changes Jan 15, 2024

View reviewed changes

SteveLauC mentioned this pull request Jan 16, 2024

test: tweak test_fanotify() to monitor ONLY 1 file #2294

Merged

3 tasks

asomers mentioned this pull request Jan 31, 2024

chore: changelog for 0.28.0 #2303

Merged

3 tasks

asomers force-pushed the nounshare branch from 1d5aff2 to 0e5306d Compare February 4, 2024 15:24

SteveLauC linked an issue Feb 6, 2024 that may be closed by this pull request

EACCES during test_mount on Cirrus-CI #1351

Closed

asomers added 3 commits February 7, 2024 07:53

Move the mount tests into the "test" program

b659834

Now that we aren't using namespaces, they don't need their own program.

Use "sudo" to run tests on github workflows.

d4a488a

Because several of the tests require root privileges. But don't use sudo with cross. It fails due to a known issue upstream: cross-rs/cross#526

asomers force-pushed the nounshare branch from 0e5306d to d4a488a Compare February 7, 2024 14:53

SteveLauC approved these changes Feb 9, 2024

View reviewed changes

SteveLauC added this pull request to the merge queue Feb 9, 2024

Merged via the queue into nix-rust:master with commit ca173ff Feb 9, 2024
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the mount tests #2269

Fix the mount tests #2269

asomers commented Dec 16, 2023 •

edited

Loading

SteveLauC commented Dec 20, 2023

SteveLauC commented Dec 20, 2023

asomers commented Dec 20, 2023

SteveLauC commented Dec 20, 2023

asomers commented Dec 20, 2023

SteveLauC commented Dec 20, 2023

SteveLauC Jan 15, 2024

SteveLauC Feb 6, 2024 •

edited

Loading

SteveLauC Jan 15, 2024

SteveLauC commented Jan 15, 2024 •

edited

Loading

asomers commented Jan 15, 2024

SteveLauC commented Jan 15, 2024

SteveLauC commented Jan 16, 2024

SteveLauC commented Jan 16, 2024

SteveLauC commented Feb 6, 2024 •

edited

Loading

Fix the mount tests #2269

Fix the mount tests #2269

Conversation

asomers commented Dec 16, 2023 • edited Loading

What does this PR do

Checklist:

SteveLauC commented Dec 20, 2023

SteveLauC commented Dec 20, 2023

asomers commented Dec 20, 2023

SteveLauC commented Dec 20, 2023

asomers commented Dec 20, 2023

SteveLauC commented Dec 20, 2023

SteveLauC Jan 15, 2024

Choose a reason for hiding this comment

SteveLauC Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

SteveLauC Jan 15, 2024

Choose a reason for hiding this comment

SteveLauC commented Jan 15, 2024 • edited Loading

asomers commented Jan 15, 2024

SteveLauC commented Jan 15, 2024

SteveLauC commented Jan 16, 2024

SteveLauC commented Jan 16, 2024

SteveLauC commented Feb 6, 2024 • edited Loading

asomers commented Dec 16, 2023 •

edited

Loading

SteveLauC Feb 6, 2024 •

edited

Loading

SteveLauC commented Jan 15, 2024 •

edited

Loading

SteveLauC commented Feb 6, 2024 •

edited

Loading