1244781 - purge_builds belongs in runner, not mozharness

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Aki Sasaki (not active)]

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

Comment 1

[Aki Sasaki (not active)]

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?

Comment 2

[Aki Sasaki (not active)]

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.

Comment 3

[Aki Sasaki (not active)]

:markco, which assumption in comment 1 is correct, if any?
* windows builds purge_builds 35gb, or
* windows builds purge_builds 10gb ?

Comment 4

[Aki Sasaki (not active)]

Attachment: untested mozharness patch

I need to learn how to use reviewboard.

Comment 5

[Mark Cornmesser [:markco] OOO 2024/04/15]

35gb is needed on Windows. During testing in AWS we ran into an issue with PGO builds that needed more than 10gb cleared.

Comment 6

[Aki Sasaki (not active)]

Attachment: 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 7

[Aki Sasaki (not active)]

Attachment: MozReview Request: bug 1244781 - stop running purge_builds in mozharness. r?jlund

Review commit: https://reviewboard.mozilla.org/r/33289/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33289/

Comment 8

[Aki Sasaki (not active)]

Attachment: MozReview Request: bug 1244781 - mozharness cleanup - update mozharness .hgignore. r?jlund

Review commit: https://reviewboard.mozilla.org/r/33291/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33291/

Comment 9

[Aki Sasaki (not active)]

Attachment: MozReview Request: bug 1244781 - silence mozharness pyflakes warnings. r?jlund

Review commit: https://reviewboard.mozilla.org/r/33293/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33293/

Comment 10

[Aki Sasaki (not active)]

Attachment: MozReview Request: bug 1244781 - mozharness pylint + pyflakes cleanup. r?jlund

Review commit: https://reviewboard.mozilla.org/r/33295/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33295/

Comment 11

[Aki Sasaki (not active)]

Attachment: MozReview Request: bug 1244781 - fix unit.sh error when mercurial isn't an old version. r?jlund

Review commit: https://reviewboard.mozilla.org/r/33297/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33297/

Comment 12

[Aki Sasaki (not active)]

Attachment: MozReview Request: bug 1244781 - mozharness has 3 post_run listeners now. r?jlund

Review commit: https://reviewboard.mozilla.org/r/33299/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33299/

Comment 13

[Jordan Lund (:jlund)]

Comment on attachment 8714809 [details]
unit_sh_output

iirc, this is https://bugzilla.mozilla.org/show_bug.cgi?id=1230150#c18

Comment 14

[Jordan Lund (:jlund)]

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.

Comment 15

[Jordan Lund (:jlund)]

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..

Comment 16

[Jordan Lund (: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

Comment 17

[Aki Sasaki (not active)]

(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.

Comment 18

[Jordan Lund (:jlund)]

Comment on attachment 8714971 [details]
MozReview Request: bug 1244781 - mozharness cleanup - update mozharness .hgignore. r?jlund

https://reviewboard.mozilla.org/r/33291/#review30115

Comment 19

[Jordan Lund (:jlund)]

Comment on attachment 8714972 [details]
MozReview Request: bug 1244781 - silence mozharness pyflakes warnings. r?jlund

https://reviewboard.mozilla.org/r/33293/#review30119

Comment 20

[Jordan Lund (:jlund)]

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?

Comment 21

[Aki Sasaki (not active)]

(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).

Comment 22

[Justin Wood (:Callek)]

(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.

Comment 23

[Aki Sasaki (not active)]

(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.

Comment 24

[Aki Sasaki (not active)]

Sylvestre: do you remember where the apiclient.discovery module comes from in mozharness.mozilla.googleplay ?

Comment 25

[Sylvestre Ledru [:Sylvestre]]

> 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!

Comment 26

[Aki Sasaki (not active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=03a993394c79

Comment 27

[Aki Sasaki (not active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9649610cd9ce5d145a7de1e0a091381222ed570
bug 1244781 - stop running purge_builds in mozharness. r=jlund

https://hg.mozilla.org/integration/mozilla-inbound/rev/4026ceb5a08324e451ca3177153abe46dcc6d683
bug 1244781 - silence mozharness pyflakes warnings. r=jlund

https://hg.mozilla.org/integration/mozilla-inbound/rev/64305a23963a6ce00cc88f6eba95b207e54575ce
bug 1244781 - mozharness has 3 post_run listeners now. r=jlund

Comment 28

[Aki Sasaki (not active)]

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

Comment 29

[Aki Sasaki (not active)]

Comment on attachment 8714972 [details]
MozReview Request: bug 1244781 - silence mozharness pyflakes warnings. r?jlund

http://hg.mozilla.org/integration/mozilla-inbound/rev/4026ceb5a083

Comment 30

[Aki Sasaki (not active)]

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

Comment 31

[Aki Sasaki (not active)]

I landed the patches with no questions around them.  I'll revisit the others.

Oh, I guess the revisions got auto-commented, nice!

Comment 32

[Aki Sasaki (not active)]

Resolving and opening a new bug for mh cleanup.

Comment 33

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f9649610cd9c
https://hg.mozilla.org/mozilla-central/rev/4026ceb5a083
https://hg.mozilla.org/mozilla-central/rev/64305a23963a

Comment 34

[Aki Sasaki (not active)]

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.

Comment 35

[Justin Wood (: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

Comment 36

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a463f2cf51e3

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a463f2cf51e3