1795592 - wrench jobs do not run on new pixel 5 phones

Status: RESOLVED FIXED

(Testing :: General, defect, P2)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

as seen on try, the wrench jobs are failing for pixel5 phones.

[task 2022-10-15T00:12:56.636Z] 00:12:56     INFO - Installing APK...
[task 2022-10-15T00:13:08.799Z] 00:12:56     INFO - >> Install app: Attempt #1 of 5
[task 2022-10-15T00:13:08.799Z] 00:13:01     INFO - Setting up SD card...
[task 2022-10-15T00:13:08.799Z] Traceback (most recent call last):
[task 2022-10-15T00:13:08.799Z]   File "/builds/task_166579275453607/fetches/mozharness/scripts/android_wrench.py", line 286, in <module>
[task 2022-10-15T00:13:08.799Z]     test.do_test()
[task 2022-10-15T00:13:08.799Z]   File "/builds/task_166579275453607/fetches/mozharness/scripts/android_wrench.py", line 245, in do_test
[task 2022-10-15T00:13:08.799Z]     self.setup_sdcard(TestMode.OPTIMIZED_SHADER_COMPILATION)
[task 2022-10-15T00:13:08.799Z]   File "/builds/task_166579275453607/fetches/mozharness/scripts/android_wrench.py", line 187, in setup_sdcard
[task 2022-10-15T00:13:08.799Z]     self.device.push(args_file, self.wrench_dir + "/args")
[task 2022-10-15T00:13:08.799Z]   File "/builds/task_166579275453607/fetches/mozdevice/mozdevice/adb.py", line 3002, in push
[task 2022-10-15T00:13:08.799Z]     self.command_output(["push", local, remote], timeout=timeout)
[task 2022-10-15T00:13:08.799Z]   File "/builds/task_166579275453607/fetches/mozdevice/mozdevice/adb.py", line 1753, in command_output
[task 2022-10-15T00:13:08.799Z]     return ADBCommand.command_output(
[task 2022-10-15T00:13:08.799Z]   File "/builds/task_166579275453607/fetches/mozdevice/mozdevice/adb.py", line 388, in command_output
[task 2022-10-15T00:13:08.799Z]     raise ADBProcessError(adb_process)
[task 2022-10-15T00:13:08.799Z] mozdevice.adb.ADBProcessError: args: adb wait-for-device push /builds/task_166579275453607/build/wrench_args /storage/emulated/0/Android/data/org.mozilla.wrench/files/wrench/args, exitcode: 1, stdout: /builds/task_166579275453607/build/wrench_args: 1 file pushed, 0 skipped. 0.4 MB/s (59 bytes in 0.000s)
[task 2022-10-15T00:13:08.799Z] adb: error: failed to copy '/builds/task_166579275453607/build/wrench_args' to '/storage/emulated/0/Android/data/org.mozilla.wrench/files/wrench/args': remote couldn't create file: Permission denied
[task 2022-10-15T00:13:08.799Z] script.py: command finished

I assume we need to add org.mozilla.wrench to a permissions list.

:aerickson, do you have any magic thoughts here?

Comment 1

[Andrew Erickson [:aerickson]]

Hmm, we're running as root... shouldn't be any permissions errors.

Does the directory exist? mkdir -p? Strange to encounter this just now on the p5's though.

Comment 2

[Geoff Brown [:gbrown]]

android_wrench.py tries to mkdir -p just before the push fails:
https://searchfox.org/mozilla-central/rev/73ff5f1ac45202c4621a1bcee4b35d20027e71ad/testing/mozharness/scripts/android_wrench.py#167

The directory is "/storage/emulated/0/Android/data/org.mozilla.wrench/files/wrench", hard-coded in android_wrench.py. I verified that /storage/emulated/0/Android/data exists on the p5.

I don't know what the problem is, but I recall that mozdevice goes to some trouble to find a writable location, for the test root; that might be a better approach. On the other hand, it looks like that issue has already been considered:
https://searchfox.org/mozilla-central/rev/73ff5f1ac45202c4621a1bcee4b35d20027e71ad/testing/mozharness/scripts/android_wrench.py#44

Comment 3

[Andrew Erickson [:aerickson]]

(In reply to Joel Maher ( :jmaher ) (UTC -8) from comment #0)

I assume we need to add org.mozilla.wrench to a permissions list.

It's an adb shell command that's failing (the test doesn't seem to be executing our app yet), so app manifest files shouldn't be an issue yet.

I couldn't find anything in the release notes for Android 12 or 13 (p5s are on 13) that might cause an issue, but Android 11 has a section on scoped storage (https://developer.android.com/about/versions/11/privacy/storage#scoped-storage).

It's interesting that android_wrench.py can create the directory, but can't push a file there.

Do we have tests working on the Pixel5s that require root?

Does anyone have a rooted p5? I'd like to know if they can create that directory and push a file to it. Wondering if there are issues with our Bitbar devices or if there is a GUI prompt (seems unlikely).

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I don't know if any of the other unittests require root- I think they do for crash dump stuff, but the rest might fail quietly?

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

hmm, this seemed to work fairly easy on samsung a51 (Android 11) in bug 1765742.

Comment 6

[Brenden Hyde [:bxbrenden] (he/him)]

:aerickson :gbrown I was wondering if you could offer some insight. This wrench test on the Pixel 5 is trying to copy a file to the /storage/emulated/0/Android/data/org.mozilla.wrench/files/wrench directory and is failing with a permission denied error. I have confirmed the following:

• the source file exists and is owned by root with 0644 permissions
• the destination directory exists and is owned by root with 0755 permissions
• the process of copying the file is being run as root

Even with all of the above, we're still get a permission denied error as you can see in this live.log file in Taskcluster or this treeherder view of my try-push. Do you have any other thoughts of what might be preventing this?

Comment 7

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/jobs?repo=try&revision=3021773c5ed9b22b32c1ff87337aa726c3c4ba03
has mozdevice verbose logging. Nothing jumps out at me, but it does give more information.

On mozilla-central, I think I see other "Android 13.0 Pixel5 AArch64 Shippable" tests running on the same device - is that right? Those seem to be using a test profile directory (which would require pushes to set up) on /data/local/... instead of /storage/emulated/... or /sdcard/... I wonder if using a directory under /data/local would be a quick way forward.

Comment 8

[Geoff Brown [:gbrown]]

I think the permission error is related to scoped storage, but I haven't actually found a definitive statement of how adb should work with respect to scoped storage.

Some forums say you can disable scoped storage with "sm set-isolated-storage off", but that command doesn't seem to work: https://treeherder.mozilla.org/jobs?repo=try&revision=7e0121932952b8430d74ee6ebdcf0bd1599631ed

I recommend switching to /data/local/... (hopefully device.test_root, hopefully configurable at the task level), although I recognize that may not be simple.

Comment 9

[Andrew Erickson [:aerickson]]

I agree with Geoff... seems like something regarding the scoped storage in Android 13. It's very strange that being root doesn't override those protections.

Comment 10

[Jamie Nicol [:jnicol]]

I stumbled across this problem yesterday when trying to run wrench tests locally on my Pixel 7. I wrote a patch to use the internal data dir instead of external, which works on my local device. But it fails on both the Pixel 2 and Samsung A51 on CI:

mozdevice.adb.ADBProcessError: args: adb wait-for-device push /builds/task_167638354604947/build/wrench_args /data/data/org.mozilla.wrench/files/wrench/args, exitcode: 1, stdout: adb: error: stat failed when trying to push to /data/data/org.mozilla.wrench/files/wrench/args: Permission denied

The difference appears to be because the ones on CI are rooted whereas my local device is not. If I set use_root = False then it succeeds (well the tests fail because of other changes in my tree, but they get past this error).

Does anyone have any insight why using root prevents the adb push to /data/data/org.mozilla.wrench/ from working? I can see in the code that it makes us not use run-as. However I wouldn't have thought that was necessary when rooted.

Comment 12

[Geoff Brown [:gbrown]]

Sorry, but I'm very fuzzy on android permissions these days.

In very high-level terms, I think your (comment 10) results make sense, in that, as I fuzzily recall, scoped storage is all about ensuring that only the app has access to its data: eg, only when run-as org.mozilla.wrench can you access /data/data/org.mozilla.wrench. Counter-intuitively (from a traditional Linux perspective), I believe that scoped storage considerations are stronger than root, in modern android.

Your WIP patch looks like a sensible approach to this thorny issue.

Comment 13

[Jamie Nicol [:jnicol]]

Thanks Geoff, makes sense.

Do you know what the best way to avoid using root for these tests would be? (for the try push I linked to above I just unconditionally set use_root = False in ADBCommand.__init__(), which is of course a big hack. Is there a way to set that variable from android_wrench.py? Or would it actually be better to change ADBCommand to prefer using run-as over root for all types of test (falling back to root if the package is not debuggable)?

Comment 14

[Geoff Brown [:gbrown]]

How about this for a quick fix: https://treeherder.mozilla.org/jobs?repo=try&revision=e9fa03363175725a3bc9dfad0b8d0b32cc5eec97

Or, for a more proper fix, update AndroidMixin to accept a use_root parameter (or environment variable, or ...?) and pass that on to mozdevice at https://searchfox.org/mozilla-central/rev/3067ba33c2202459aae52b7a0f7218a71fe5f7c3/testing/mozharness/mozharness/mozilla/testing/android.py#71.

Comment 15

[Jamie Nicol [:jnicol]]

Attachment: Bug 1795592 - Allow wrench to run on recent Android versions. r?gbrown,#gfx-reviewers

To run webrender's wrench tests the app needs to read a file to parse
the command line args from, and write a file with the test
output. These files also need to be written and read manually over adb
by the engineer running the tests (or more likely by the
android_wrench.py script).

Currently we use the external data dir for these files. However, on
recent android versions these are no longer accessible to the app
without jumping through some hoops. This patch makes us instead use
the internal data dir. It also makes the app "debuggable" so that
these files can be written to via adb using run-as.

Additionally, we must ensure that the android_wrench.py script uses
run-as instead of root to push and pull the files, as root does not
have permission to do so on recent android versions.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b0de6542378
Allow wrench to run on recent Android versions. r=gbrown

Comment 17

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/6b0de6542378

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

this is looking great when migrated to pixel5:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=cZt_mG0HTaWgRac1yBcMAQ.0&tier=1%2C2%2C3&revision=38ab3c6acfd51443ccd6e62b8efef91406a81d3b

thanks for the work on this, I will get this migrated!