purge_builds belongs in runner, not mozharness

RESOLVED FIXED

Status

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: aki, Assigned: aki)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments, 1 obsolete attachment)

1) verify that runner is running purge_builds for every platform
(see bug 1191352 etc)

2) verify that runner is purging the maximum amount of space needed for the most disk-hungry build/test on that platform

3) remove purge_builds support from mozharness.
this is more of a production infrastructure need rather than a job-specific need, and causes developers to nuke their homedirs when they run mozharness jobs locally.  see bug 1131872
I'm not 100% sure how to do steps 1 and 2.

I think runner is in https://github.com/mozilla/build-runner
I think runner is run and configured on the build slaves via puppet.

builds?
* windows builds 35gb
** https://github.com/mozilla/build-puppet/blob/master/modules/toplevel/manifests/slave/releng/build.pp#L46
** https://bug1191352.bmoattachments.org/attachment.cgi?id=8662020 seems to suggest that windows purge is 10gb though?
* non-windows builds 20gb
** https://github.com/mozilla/build-puppet/blob/master/modules/toplevel/manifests/slave/releng/build.pp#L71

tests:
* ubuntu tests 4gb
** https://github.com/mozilla/build-puppet/blob/master/modules/toplevel/manifests/slave/releng/test.pp#L24
* darwin tests 4gb
** https://github.com/mozilla/build-puppet/blob/master/modules/toplevel/manifests/slave/releng/test.pp#L34

what about non-ubuntu non-darwin tests?  Do we need purge_builds there?
I don't think tests have purge_builds in mozharness.
The max purge_builds size in mozharness is 18, so runner is sufficient, assuming my assumed 35gb and 20gb in builds is accurate.
:markco, which assumption in comment 1 is correct, if any?
* windows builds purge_builds 35gb, or
* windows builds purge_builds 10gb ?
Flags: needinfo?(mcornmesser)
Posted patch untested mozharness patch (obsolete) — Splinter Review
I need to learn how to use reviewboard.
Assignee: nobody → aksasaki
35gb is needed on Windows. During testing in AWS we ran into an issue with PGO builds that needed more than 10gb cleared.
Flags: needinfo?(mcornmesser)
Posted file unit_sh_output
I'm going to clean up unit.sh as well.  I'll try to keep these as separate patches for easier review.
Comment on attachment 8714970 [details]
MozReview Request: bug 1244781 - stop running purge_builds in mozharness. r?jlund

https://reviewboard.mozilla.org/r/33289/#review30043

code lgtm.

my primary concern is if we left some rogue platform / job out there that doesn't use runner. Or if it does use runner, if it uses the purge builds step. I'm not too familiar with that so that should get confirmed from someone if it hasn't been already.
Attachment #8714970 - Flags: review?(jlund) → review+
Comment on attachment 8714974 [details]
MozReview Request: bug 1244781 - fix unit.sh error when mercurial isn't an old version. r?jlund

https://reviewboard.mozilla.org/r/33297/#review30045

iirc - this issue went away with tox. how are you calling these tests? I wonder what tox means for unit.sh's future..
Attachment #8714974 - Flags: review?(jlund)
Comment on attachment 8714975 [details]
MozReview Request: bug 1244781 - mozharness has 3 post_run listeners now. r?jlund

https://reviewboard.mozilla.org/r/33299/#review30047

lgtm. thanks for this.

I'm going to pause reviewing the other patches that involve unit.sh. I notice that there might be an overlap with tox here as it does some coverage / unittests for us and is self contained.

context:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/tox.ini
https://bugzilla.mozilla.org/show_bug.cgi?id=1076810#c69
Attachment #8714975 - Flags: review?(jlund) → review+
(In reply to Jordan Lund (:jlund) from comment #15)
> Comment on attachment 8714974 [details]
> MozReview Request: bug 1244781 - fix unit.sh error when mercurial isn't an
> old version. r?jlund
> 
> https://reviewboard.mozilla.org/r/33297/#review30045
> 
> iirc - this issue went away with tox. how are you calling these tests? I
> wonder what tox means for unit.sh's future..

* I'm runnning with ./unit.sh
* Tox probably means I should change unit.sh to run tox, but tox runs coverage + nosetests, and unit.sh runs coverage + nosetests + pyflakes + pylint, so overall unit.sh is running more stuff.
* This issue went away with tox because it forces mercurial to be 2.6.2.  This patch allows the test to run successfully in mercurial 2.6.2, or 3.6.3.  I can open a new bug for this patch if you prefer.
Attachment #8714407 - Attachment is obsolete: true
Attachment #8714971 - Flags: review?(jlund) → review+
Comment on attachment 8714971 [details]
MozReview Request: bug 1244781 - mozharness cleanup - update mozharness .hgignore. r?jlund

https://reviewboard.mozilla.org/r/33291/#review30115
Attachment #8714972 - Flags: review?(jlund) → review+
Comment on attachment 8714972 [details]
MozReview Request: bug 1244781 - silence mozharness pyflakes warnings. r?jlund

https://reviewboard.mozilla.org/r/33293/#review30119
Comment on attachment 8714973 [details]
MozReview Request: bug 1244781 - mozharness pylint + pyflakes cleanup. r?jlund

https://reviewboard.mozilla.org/r/33295/#review30123

looks good. one comment that I think is worth discussing as a higher level decision.

::: testing/mozharness/requirements.txt:9
(Diff revision 1)
> -mercurial==2.6.3
> +mercurial==3.7

new hotness..

::: testing/mozharness/requirements.txt:11
(Diff revision 1)
> +../mozbase/mozdevice

hm, this would make a dep on all of gecko right? Do you think there is still value in allowing us to grab mozharness as a subdir archive from gecko and not worry about gecko deps?
Attachment #8714973 - Flags: review?(jlund) → review+
(In reply to Jordan Lund (:jlund) from comment #20)
> Comment on attachment 8714973 [details]
> MozReview Request: bug 1244781 - mozharness pylint + pyflakes cleanup.
> r?jlund
> 
> https://reviewboard.mozilla.org/r/33295/#review30123
> 
> looks good. one comment that I think is worth discussing as a higher level
> decision.
> 
> ::: testing/mozharness/requirements.txt:9
> (Diff revision 1)
> > -mercurial==2.6.3
> > +mercurial==3.7
> 
> new hotness..
> 
> ::: testing/mozharness/requirements.txt:11
> (Diff revision 1)
> > +../mozbase/mozdevice
> 
> hm, this would make a dep on all of gecko right? Do you think there is still
> value in allowing us to grab mozharness as a subdir archive from gecko and
> not worry about gecko deps?

Ah.  So we were discussing this after I posted the patch.
I think I'm going to create a requirements-intree.txt and a requirements.txt.
* requirements-intree.txt will contain relative paths to ../mozbase
* requirements.txt will contain versioned mozbase modules that are installable via pypi.

I still need to get the google play info from sylvestre to fix all the import warnings anyway, so we can either skip this patch for now, or land as-is with a couple followups (requirements-intree and sylvestre's dependencies).
(In reply to Aki Sasaki [:aki] from comment #21)
> (In reply to Jordan Lund (:jlund) from comment #20)
> > ::: testing/mozharness/requirements.txt:11
> > (Diff revision 1)
> > > +../mozbase/mozdevice
> > 
> > hm, this would make a dep on all of gecko right? Do you think there is still
> > value in allowing us to grab mozharness as a subdir archive from gecko and
> > not worry about gecko deps?
> 
> Ah.  So we were discussing this after I posted the patch.
> I think I'm going to create a requirements-intree.txt and a requirements.txt.
> * requirements-intree.txt will contain relative paths to ../mozbase
> * requirements.txt will contain versioned mozbase modules that are
> installable via pypi.
> 
> I still need to get the google play info from sylvestre to fix all the
> import warnings anyway, so we can either skip this patch for now, or land
> as-is with a couple followups (requirements-intree and sylvestre's
> dependencies).

Fwiw, I use mozharness, as a standalone archive (downloaded via archiver) for the openh264 builds. And even when I transition it to TC scheduling/building was planned to still be essentially the same thing -- grabbed as a standalone since we don't need anything in gecko other than mozharness to build openh264.

So there is certainly value in letting mozharness itself and related stuff work just fine as a downloaded package.
(In reply to Justin Wood (:Callek) from comment #22)
> So there is certainly value in letting mozharness itself and related stuff
> work just fine as a downloaded package.

I agree, which is why the requirements.txt + requirements-intree.txt solution.
Sylvestre: do you remember where the apiclient.discovery module comes from in mozharness.mozilla.googleplay ?
Flags: needinfo?(sledru)
> Sylvestre: do you remember where the apiclient.discovery module comes from
> in mozharness.mozilla.googleplay ?
Sure, this is declared here:
http://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/requirements.txt#l24
google-api-python-client

Hope this helps!
Flags: needinfo?(sledru)
Comment on attachment 8714970 [details]
MozReview Request: bug 1244781 - stop running purge_builds in mozharness. r?jlund

http://hg.mozilla.org/integration/mozilla-inbound/rev/f9649610cd9c
Attachment #8714970 - Flags: checked-in+
Comment on attachment 8714972 [details]
MozReview Request: bug 1244781 - silence mozharness pyflakes warnings. r?jlund

http://hg.mozilla.org/integration/mozilla-inbound/rev/4026ceb5a083
Attachment #8714972 - Flags: checked-in+
Comment on attachment 8714975 [details]
MozReview Request: bug 1244781 - mozharness has 3 post_run listeners now. r?jlund

http://hg.mozilla.org/integration/mozilla-inbound/rev/64305a23963a
Attachment #8714975 - Flags: checked-in+
I landed the patches with no questions around them.  I'll revisit the others.

Oh, I guess the revisions got auto-commented, nice!
Resolving and opening a new bug for mh cleanup.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Comment on attachment 8714974 [details]
MozReview Request: bug 1244781 - fix unit.sh error when mercurial isn't an old version. r?jlund

Since you've pinged me about it twice, I figured you may be motivated to review.
Attachment #8714974 - Flags: review?(bugspam.Callek)
Comment on attachment 8714974 [details]
MozReview Request: bug 1244781 - fix unit.sh error when mercurial isn't an old version. r?jlund

https://reviewboard.mozilla.org/r/33297/#review34217

The second ping was more me telling gps about it ;-)  But sure
Attachment #8714974 - Flags: review?(bugspam.Callek) → review+
You need to log in before you can comment on or make changes to this bug.