Closed Bug 1245347 Opened 4 years ago Closed 3 years ago

ADBRootError: Can not run command setenforce Permissive as root!

Categories

(Testing :: mozregression, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: kbrosnan, Assigned: bc)

References

Details

Attachments

(3 files, 1 obsolete file)

Executing mozregression -n=fennec --good=2016-1-30 --bad=2016-2-01 on two linux machines I get the following output. Target device is a Nexus 5x running Android 6.0

 0:06.86 LOG: MainThread adb INFO Setting SELinux Permissive Mode
 0:06.87 LOG: MainThread Test Runner ERROR Unable to install '/tmp/tmpbEFNe6/2016-01-13--mozilla-central--fennec-46.0a1.multi.android-arm.apk'
Traceback (most recent call last):

  File "/usr/lib/python2.7/site-packages/mozregression/launchers.py", line 51, in __init__
    self._install(dest)

  File "/usr/lib/python2.7/site-packages/mozregression/launchers.py", line 306, in _install
    self.adb = ADBAndroid()

  File "/usr/lib/python2.7/site-packages/mozdevice/adb_android.py", line 92, in __init__
    self.shell_output("setenforce Permissive", timeout=timeout, root=True)

  File "/usr/lib/python2.7/site-packages/mozdevice/adb.py", line 1120, in shell_output
    timeout=timeout, root=root)

  File "/usr/lib/python2.7/site-packages/mozdevice/adb.py", line 1016, in shell
    raise ADBRootError('Can not run command %s as root!' % cmd)

ADBRootError: Can not run command setenforce Permissive as root!

 0:06.87 LOG: MainThread main ERROR Unable to install '/tmp/tmpbEFNe6/2016-01-13--mozilla-central--fennec-46.0a1.multi.android-arm.apk'

may be the same as bug 1233756 but not sure.
Bob, maybe you have an  idea of what that could be ? Is it the same as bug 1233756 ?
Flags: needinfo?(bob)
Just noticed that the dates are wrong. I used "mozregression -n=fennec --good=2016-1-13 --bad=2016-1-27". I had used a previous release of mozregression/mozdevice on the 5x with success.
parkouss: No, bug 1233756 is about setting SELinux Permissive in devicemanagerADB.

This is from https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py#1006

The properties regarding root are set in https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py#574

We check for the root shell here: https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py#646

It looks like we are requesting root, but don't have a root shell and neither su 0 or su -c are supported. I would run with debug on and verbose True to see what the debugging and logging output are when ADBAndroid is being initialized.

I would say this device isn't rooted. kbrosnan?
Flags: needinfo?(bob)
Hey, I just hit this locally on a rooted device. Investigating.
Correct this is an unrooted device.
In my case, I believe this is related to attempting to set SELinux Permissive too soon after the device is listed in adb devices. Autophone failed to initialize the device initially but I was able to use ap.sh autophone-add-device to successfully add it moments later.

I noticed this while investigating improving is_device_ready intending to reduce the time we wait for a device to be ready and wondered if there was a possibility of a race condition. Perhaps my test patch is responsible for my immediate issue or perhaps it is just fortuitous.

I filed bug 1245900 to track the work but if your device isn't rooted, that would explain the issue you are seeing.
kbrosnan: Do you think this is an issue with adb_android.py where it should not always fail fatally if it is used on a non-rooted device?
Flags: needinfo?(kbrosnan)
I don't know of any reason mozregression needs root. It executes two commands as I understand. 'adb uninstall org.mozilla.fennec' and 'adb install /path/to/firefox.apk'. Requiring a rooted device would be a significant hurdle to test regression ranges.
Flags: needinfo?(kbrosnan)
Ok. I guess the question is if mozregression needs abd_android or if it can do with something less heavy weight.

parkouss, let me know what you would like to do. If you want to keep adb_android.py, it may get complicated to support situations where root is not required or maybe not. If you want me to fix this in adb_android.py, move this over to mozdevice and I'll look into the case where only a limited set of functionality is required.
Flags: needinfo?(j.parkouss)
Depends on: 1246844
It seems to me that the best thing to do would be to keep mozdevice and adb_android.py, else we would probably end up in duplicating code - so I propose that we try to fix that in mozdevice.

I just created bug 1246844 for that purpose, so we can still track this on mozregression side and update the mozdevice required dependency when it is fixed.
Flags: needinfo?(j.parkouss)
Ok. I'll look into it, but if all you need is install and uninstall, using a simple subprocess.call or something similar would be very easy. It may not be nearly as simple to segregate the use of commands which require root in adb.py and adb_android.py.
I put up a pull request to pin Mozregression to mozdevice 0.47 as a quick fix. I have tested locally and this allowed Mozregression to work on non-rooted devices. https://github.com/mozilla/mozregression/pull/425
(In reply to Bob Clary [:bc:] from comment #11)
> Ok. I'll look into it, but if all you need is install and uninstall, using a
> simple subprocess.call or something similar would be very easy. It may not
> be nearly as simple to segregate the use of commands which require root in
> adb.py and adb_android.py.

mozdevice should not require root. If the user performs an operation that does require it, it's fine if we fail. But there are lots of cases where we might want to run a test on an unrooted device.
Now that the temporary fix has landed, I just wanted to note that reverting to mozdevice 0.47 has also fixed another issue: Even though my phone *is* rooted, ADB would usually hang during the root check if the ADB server was already running from some previous command. At least I could keep mozregression going by killing the adb server process.
Attached patch bug-1245347-1-v1.patch (obsolete) — Splinter Review
Attachment #8809975 - Flags: feedback?(dietrich)
These two patches solve some of the problems with an unrooted device. You can initialize ADBAndroid, call install_app, launch_fennec and uninstall_app. It may be enough. I'm still testing this but it appears to work so far.

dietrich: Let me know if this is sufficient for you.
Flags: needinfo?(dietrich)
Attachment #8809976 - Flags: feedback?(dietrich)
Attachment #8809975 - Flags: review?(wlachance)
Comment on attachment 8809976 [details] [diff] [review]
bug-1245347-2-v1.patch

This appears to work on my Pixel devices. I have identical patches for Autophone's version.

wlach: Can you point me to instructions on how to do a mozregression release?
Attachment #8809976 - Flags: review?(wlachance)
Attachment #8809975 - Flags: review?(wlachance) → review+
Comment on attachment 8809976 [details] [diff] [review]
bug-1245347-2-v1.patch

Looks good. Can you also bump the version of mozdevice in `testing/mozbase/mozdevice/setup.py`? I can take care of releasing new versions of mozdevice and mozregression once this is landed.
Attachment #8809976 - Flags: review?(wlachance) → review+
I went to apply this change to my installed mozdevice and it was already like this (no root=True bits). All I did was pip install this stuff per the mozregression website.
Flags: needinfo?(dietrich)
slight tweak to the original patch to not use string interpolation in the logging warning. Carrying forward r+.
Attachment #8809975 - Attachment is obsolete: true
Attachment #8809975 - Flags: feedback?(dietrich)
Attachment #8810922 - Flags: review+
Attachment #8810923 - Flags: review?(wlachance)
(In reply to Dietrich Ayala (:dietrich) from comment #19)
> I went to apply this change to my installed mozdevice and it was already
> like this (no root=True bits). All I did was pip install this stuff per the
> mozregression website.

The root=True change was introduced in Bug 1245900 which was definitely not in mozdevice 0.47 but does appear to be in mozdevice 0.48.

mozregression doesn't appear to have a requirement set for mozdevice. We may need to revisit that.
Assignee: nobody → bob
Status: NEW → ASSIGNED
Duplicate of this bug: 1246844
I had forced MozRegession to use MozDevice 0.47 in https://github.com/mozilla/mozregression/commit/63c7ed4b6a677b964697250d875a606d657a0faa MozRegession 0.9.0 was the first release with the pinning.
Attachment #8809976 - Flags: feedback?(dietrich)
Comment on attachment 8810923 [details] [diff] [review]
bug-1245347-3-v1.patch

No need to ask for review on version bumps, but ok. :)
Attachment #8810923 - Flags: review?(wlachance) → review+
Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/950e7c87e151
[Autophone|mozdevice] - catch initial ADBRootError during initialization if root is not supported, r=wlach.
https://hg.mozilla.org/integration/mozilla-inbound/rev/129f96353f01
[Autophone|mozdevice] - is_device_ready should not require root to check the ready_path, r=wlach.
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a6d43c9c7a5
[Autophone|mozdevice] - increase mozdevice version to 0.49, r=wlach.
https://hg.mozilla.org/mozilla-central/rev/950e7c87e151
https://hg.mozilla.org/mozilla-central/rev/129f96353f01
https://hg.mozilla.org/mozilla-central/rev/6a6d43c9c7a5
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
deployed 2016-11-18 ~10:00 AM
You need to log in before you can comment on or make changes to this bug.