Closed Bug 1281179 Opened 8 years ago Closed 8 years ago

mochitest-media jobs on treeherder should be run in ubuntu 16.04, not 12.04

Categories

(Testing :: General, defect, P2)

Unspecified
Linux
defect

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

Attachments

(1 file, 4 obsolete files)

It came to my attention in London that we should be running the 'mda' jobs in Ubuntu 16.04 vs 12.04. Upgrading the OS for all our tests is quite a burden, in fact so much a burden that this is something we would schedule a full time person for 4 months on. Now that our linux jobs run in taskcluster (debug is only on taskcluster, opt/pgo are in parallel with buildbot), we have the opportunity to run jobs on an arbitrary docker image. The way forward here is to create a compatible ubuntu 16.04 docker image (not so trivial, but much easier than impossible), then run the tests on it. I have done a start to this, but it looks as though my image doesn't deal with pulseaudio so well. In the log: https://g3wmz2aaaaavk42xwspx4pgasfla7ekf3r4ok7tulw7kuo5l.taskcluster-worker.net:32772/log/j9eE7jTjTHO8WAMdZCUF4g I see: JavaScript error: http://mochi.test:8888/tests/dom/media/mediasource/test/test_MultipleInitSegments_mp4.html, line 18: NotSupportedError: Operation is not supported which is a call to |addSourceBuffer()|. In the above log file, i see: + pulseaudio --fail --daemonize --start E: [pulseaudio] client-conf-x11.c: [1;31mxcb_connection_has_error() returned true[0m + pactl load-module module-null-sink xcb_connection_has_error() returned true 17 I assume I messed that up in my config. Many other tests are running successfully, so there is hope.
this patch is simple and a good start at solving this. A few things concern me in the ubuntu1604-test/system-setup.sh- I had to hack out a lot of things that are hardcoded on the tooltool side for the precise repo, I suspect we just need to update things to be xenial and then life is good.
:dminor, is this something that you can pick up? This would gain us a first step into 16.04 testing and updated ffmpeg tools for the media tests.
Flags: needinfo?(dminor)
Comment on attachment 8763903 [details] [diff] [review] patch used to create docker image and use it on try server Review of attachment 8763903 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/docker/desktop1604-test/dot-files/pulse/default.pa @@ +13,5 @@ > +# General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public License > +# along with PulseAudio; if not, write to the Free Software Foundation, > +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. I know this is just copy-pasted, but .. is this licencse a problem? ::: testing/docker/ubuntu1604-test/system-setup.sh @@ +128,5 @@ > + > +# sha256: wJnELXTi1SC2HdNyzZlrD6dgXAZheDT9exPHm5qaWzA > +mercurial==3.7.3 > +EOF > +#TODO: JMAHER: figure out how to install or remove this What is the issue here? These should be installable on Xenial @@ +167,5 @@ > +} > +] > +EOF > +tar -zxf xcb-repo-*.tgz > +# TODO: JMAHER: figure out the xenial version For this and the other Ubuntu repos, you can probably just create a new tarball using a modified version of the update.sh mentioned above. Ideally, though, we can use the upstream xcb packages and not install custom-built stuff anymore
(In reply to Joel Maher (:jmaher) from comment #2) > :dminor, is this something that you can pick up? This would gain us a first > step into 16.04 testing and updated ffmpeg tools for the media tests. I'll discuss this with Maire when she's back from PTO but I'm not sure if this would be a high priority for us.
Flags: needinfo?(dminor)
I'm moving this to Testing::General as Core::Audio/Video is usually a home for untriaged bugs and none of the subcategories seem suitable.
Rank: 27
Component: Audio/Video → General
OS: Unspecified → Linux
Priority: -- → P2
Product: Core → Testing
(In reply to Dustin J. Mitchell [:dustin] from comment #3) > Comment on attachment 8763903 [details] [diff] [review] > patch used to create docker image and use it on try server > > Review of attachment 8763903 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: testing/docker/desktop1604-test/dot-files/pulse/default.pa > @@ +13,5 @@ > > +# General Public License for more details. > > +# > > +# You should have received a copy of the GNU Lesser General Public License > > +# along with PulseAudio; if not, write to the Free Software Foundation, > > +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. > > I know this is just copy-pasted, but .. is this licencse a problem? yeah, probably is. > > ::: testing/docker/ubuntu1604-test/system-setup.sh > @@ +128,5 @@ > > + > > +# sha256: wJnELXTi1SC2HdNyzZlrD6dgXAZheDT9exPHm5qaWzA > > +mercurial==3.7.3 > > +EOF > > +#TODO: JMAHER: figure out how to install or remove this > > What is the issue here? These should be installable on Xenial the virtualenv and mecurial wouldn't install, I need to figure out the history of why we have these specific versions and why it is setup in a requirements.txt. I did a pip install virtualenv and that works > > @@ +167,5 @@ > > +} > > +] > > +EOF > > +tar -zxf xcb-repo-*.tgz > > +# TODO: JMAHER: figure out the xenial version > > For this and the other Ubuntu repos, you can probably just create a new > tarball using a modified version of the update.sh mentioned above. Ideally, > though, we can use the upstream xcb packages and not install custom-built > stuff anymore yeah, this just takes a little bit of time and finding the right bits. There are different tools and libraries available/required for 16.04. Another thing is I removed all the :i386 files from the 12.04 system-setup.sh file. Thanks for the quick drive by review- maybe another cycle or two I can get this running ok.
ok, the error I am seeing in running the tests is because we don't have the codecs installed. I installed locally: apt_packages+=('ubuntu-restricted-extras') and it worked! I also had to move the pulseaudio after the xvfb startup in test-linux.sh- that could cause some problems in deployment. Now to make an official image and see if I can make things work on try server
ok, running on my latest 16.04 image (and on m3.large), these seem to run better- a few issues outside of my main domain: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c566e0d36254e482e70fd57b250a22d9c037623e :k17e, can you look at these, possibly find someone to green them up?
Flags: needinfo?(ajones)
Depends on: 1242268
I've retriggered the jobs to see if the window sharing related failures are intermittent or now permanently failing. I'll follow up with a bug if the latter.
Depends on: 1282425
Attached patch files for building ubuntu 16.04 (obsolete) (deleted) — Splinter Review
a few things are iffy here: 1) I copied all the files from desktop-test and ubuntu-test, so if we change anything it needs to be duplicated 2) test-linux.sh will probably break 12.04, maybe this needs to be specific to the os version? 3) system-setup.sh- a few comments in there, I would like your feedback on that. Likewise, I removed xcb and mesa and we have newer versions- possibly I need gfx guys to sign off on that. Lastly in this file is a check for specific package versions, not sure if that is a good idea or not 4) 12.04 has 3 docker images, previous irc chat was discussing this and we agreed on 2, please confirm 5) how do I ensure these get updated? I have myself as the maintainer, and desktop1604-test depending on elvis314/ubuntu1604-test, how do I get taskcluster/ubuntu1604-test to be possible?
Attachment #8763903 - Attachment is obsolete: true
Attachment #8765449 - Flags: feedback?(dustin)
Comment on attachment 8765449 [details] [diff] [review] files for building ubuntu 16.04 Review of attachment 8765449 [details] [diff] [review]: ----------------------------------------------------------------- Aside from references to elvis314, this looks good. Is the idea to eventually migrate everything to 16.04 (in which case the duplication between docker directories isn't such a big deal)? ::: testing/docker/ubuntu1604-test/system-setup.sh @@ +110,5 @@ > +} > +] > +EOF > +pip install --upgrade pip > +pip install peep-2.4.1.tar.gz It looks like peep is never used. @gps is pretty insistent that we use the latest-and-greatest mercurial everywhere, rather than that available from the distro. Was there some reason that peep installing mercurial didn't work? @@ +189,5 @@ > +cp sources.list.orig /etc/apt/sources.list > +apt-get update > + > +# clean up > +apt_packages+=('mesa-common-dev') ?? this variable is never used after this point..
Attachment #8765449 - Flags: feedback?(dustin) → feedback+
Blocks: 1283161
Blocks: 1223198
my latest push yields more success: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce1a8a818381de587a4e25774b3272e7cd9e72d4 most likely only an issue with bug 1242268. :dminor, possibly we can close some of the other bugs out. If you can push to try with this and bug 1242268 or get bug 1242268 landed, I can pick up the next steps when I get back from PTO on Friday.
I rebased Joel's and Karl's patches on top of central and pushed to try here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b621c41638b010a321a065203c8346cd868ed38 It looks like Karl is back from PTO on the 18th, I don't think it makes sense to try to land his patches on 1242268 in his absence. He has a "bandaid" patch at the top of that bug, if it looks like things are still held up next week (e.g. he found a problem after review), maybe we can land that just so we can move forward here.
Attached patch add ubuntu 16.04 docker type (obsolete) (deleted) — Splinter Review
this is looking really nice, a few things to consider when reviewing: 1) I have addressed all previous feedback, but please be picky 2) test-linux.sh applies to both 12.04 and 16.04- are there concerns here? 3) I enable compiz specifically- I am not sure how to check for it 4) I am not sure if desktop1604-test can magically get built with image.tar, if so- please verify that looks right 5) I marked the docker image registry as taskcluster, technically this doesn't exist on there, what do we need to do? 6) for the .yml files, they depended on fx_docker_desktop_test.yml, I skipped that and went straight for fx_docker1604_test_base.yml That is a lot of items to consider, but I want to do this correctly! As try is closed, I need to do a full run on 12.04, and a final run with 16.04- this will probably happen this weekend or Monday.
Assignee: nobody → jmaher
Attachment #8765449 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: needinfo?(ajones)
Attachment #8771312 - Flags: review?(dustin)
hmm, this fails on try due to missing ubuntu1604 image: https://treeherder.mozilla.org/#/jobs?repo=try&revision=73706f7f8bb76e5166688494d8aa8bf1d6ebd0c6 I am not sure how to get taskcluster to automatically generate this image, :dustin?
You'll need to whitelist it in docker_image.py.
Attached patch add ubuntu 16.04 docker type (obsolete) (deleted) — Splinter Review
image works as expected, here is the mochitest-media (mda) jobs running in ubuntu 12.04 as they were: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f2590f98c74189cbb35ff5f302933678b79263c and here is where they are running in ubuntu 16.04 with the one failure which has a fix waiting to land in bug 1242268: https://treeherder.mozilla.org/#/jobs?repo=try&revision=893eb4f8b57fa0d8b20ccb7b396f41da6e89f00f once bug 1242268 is fixed, we can switch the mochitest-media yml files to use 16.04 via: $inherits: - from: 'tasks/tests/fx_docker_desktop_generic.yml' + from: 'tasks/tests/fx_docker1604_test_base.yml'
Attachment #8771312 - Attachment is obsolete: true
Attachment #8771312 - Flags: review?(dustin)
Attachment #8772385 - Flags: review?(dustin)
as a note, I had to create a new version of test-linux.sh for 16.04 as we need a window manager and I use compiz. With compiz we get a lot of failure in 12.04: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bed5c48429406d00fe2b7b12e5b05b2216179d8e from my original list of question in comment 14, here is what remains to be answered in the review: 1) I have addressed all previous feedback, but please be picky 2) I enable compiz specifically- I am not sure how to check for it 3) for the .yml files, they depended on fx_docker_desktop_test.yml, I skipped that and went straight for fx_docker1604_test_base.yml
Comment on attachment 8772385 [details] [diff] [review] add ubuntu 16.04 docker type Review of attachment 8772385 [details] [diff] [review]: ----------------------------------------------------------------- I think this looks good. The duplication bothers me a bit, both of test-*.sh and of the docker ephemera, but neither is easy to fix without introducing more complexity, so it's fine as-is. It's even more fine if, eventually, we expect Ubuntu-16.04 to be the only platform (as we can then let 1204 moulder until we kill it) ::: taskcluster/ci/legacy/tasks/tests/fx_docker1604_test_base.yml @@ +1,2 @@ > +--- > +$inherits: I know you'll miss $inherits and its ilk, but this will need to be rebased atop the new test-generation framework. It shouldn't be too hard! (It's currently in autoland, hopefully merging to inbound soon) ::: taskcluster/scripts/tester/test-linux.sh @@ -63,5 @@ > fail "mozharness zip did not contain mozharness/" > fi > > # start up the pulseaudio daemon. Note that it's important this occur > -# before the Xvfb startup. Thoughts on renaming this to test-ubuntu1204.sh too?
Attachment #8772385 - Flags: review?(dustin) → review+
Thanks Dustin! The plan is to eventually replace 12.04 with 16.04 everywhere- getting one job on it is a good start. I will rebase and rename test-linux.sh -> test-ubuntu1204.sh for parity. It might be 6 months before we fully retire 1204 :)
I updated this because of the in-tree kind refactoring (and the small nits from the previous r+)- it would be good to get a new review on this.
Attachment #8772385 - Attachment is obsolete: true
Attachment #8774340 - Flags: review?(dustin)
Comment on attachment 8774340 [details] [diff] [review] add ubuntu 16.04 docker type and run mda jobs on it Review of attachment 8774340 [details] [diff] [review]: ----------------------------------------------------------------- I'm assuming the Dockerfile and associated files haven't changed much. The task definition stuff looks great!
Attachment #8774340 - Flags: review?(dustin) → review+
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b1bc27d8f8a5 Create a docker image for Ubuntu 16.04 for use in tests. r=dustin
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Depends on: 1289424
Depends on: 1289209
as this isn't working on ASAN due to bug 1289209 and bug 1289424, I am reopening this. Linux64 opt/debug are working on 16.04 successfully though. Personally I am done with my involvement here- If someone who is familiar with the mochitest-media tests proper wants to debug the leaks, I do encourage that as it will pave the way forward for a modern operating system and the possibility for the tests to use a newer ffmpeg and toolchains. To test a fix with asan, you will need to remove this code: https://reviewboard.mozilla.org/r/67230/diff/1#index_header
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Paul, those issues like something up your alley?
Flags: needinfo?(padenot)
Maybe dminor ? I'm about to go on PTO
Flags: needinfo?(padenot) → needinfo?(dminor)
Blocks: 1290183
Blocks: 1292696
(In reply to Paul Adenot (:padenot) (in PTO until early September) from comment #27) > Maybe dminor ? I'm about to go on PTO I filed Bug 1293658 to track the cubeb leak. There might be other leaks, it's hard to tell, some have no module associated with them. I discussed this with mreavy, we've decided to make this a P2, so we might not get to this until Q4.
Flags: needinfo?(dminor)
these are all running in 16.04
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: