Closed Bug 1334167 Opened 3 years ago Closed 3 years ago

.cron.yml follow-on work

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla54

People

(Reporter: dustin, Assigned: dustin)

References

Details

Attachments

(5 files, 1 obsolete file)

A few issues from bug 1252948, in a new bug so that mozreview doesn't get confused.

 * The existing jobs, scheduled at {hour: 16, minute: 0}, happily ran at 16:30 in https://tools.taskcluster.net/task-inspector/#VGh6DQqpQNCdWnIgyLYM7Q/

 * The cron task uses `hg log -r $GECKO_HEAD_REF -T {node}` to determine the head rev, but when $GECKO_HEAD_REF is `tip` this pulls the tip of the unified repo -- not what we want.  Instead, that command should use `-r .` to get whatever revision is running the cron task; then it's up to run-task to have checked out the right version.

 * We should be specifying GECKO_HEAD_REF=default in the hooks, not =tip

 * The cron.yml `projects` property should use the same expansions as `run-on-projects` (in fact, it should probably be named the same) -- specifically, 'integration' and 'release' expand to a number of projects.

 * We will almost always want a different schedule for each project, for a given job.  So we should support a `by-project` attribute in the `when` property, similar to the `by-test-platform` support in taskgraph generation.

 * Remove the `when` bits from .cron.yml so that we can deploy the hooks fully without worrying that bunches of broken jobs will start running
(In reply to Dustin J. Mitchell [:dustin] from comment #0)
>  * We should be specifying GECKO_HEAD_REF=default in the hooks, not =tip

Fixed.
(In reply to Dustin J. Mitchell [:dustin] from comment #0)
>  * Remove the `when` bits from .cron.yml so that we can deploy the hooks
> fully without worrying that bunches of broken jobs will start running

This piece is very close to Bug 1332421 -- though I expect that bug can be wontfixed in favor of this because we actually want m-c nightlies via taskcluster now
Duplicate of this bug: 1332421
Comment on attachment 8832293 [details]
Bug 1334167: calculate head revision with -r .;

https://reviewboard.mozilla.org/r/108626/#review109860

::: taskcluster/taskgraph/cron/util.py:29
(Diff revision 1)
>          return False
>      return True
>  
>  
>  def calculate_head_rev(options):
> -    return subprocess.check_output(['hg', 'log', '-r', options['head_ref'], '-T', '{node}'])
> +    return subprocess.check_output(['hg', 'log', '-r', '.', '-T', '{node}'])

would be great to have a comment here saying that the gecko checkout will get us the head rev already, and we're using `.` to get the used value of ambiguous remote revisions (e.g. default)

I'm not sure the best way to write this comment though.
Attachment #8832293 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8832294 [details]
Bug 1334167: match times correctly;

https://reviewboard.mozilla.org/r/108628/#review109862

::: taskcluster/taskgraph/test/test_cron_util.py:17
(Diff revision 1)
> +from taskgraph.cron.util import (
> +    match_utc,
> +)
> +
> +
> +class TestMatchUtc(unittest.TestCase):

This is a great case where `hypothesis` would have been wonderful (but since its not vendored in m-c thats not as big a deal.)

Can we also get a match_utc where hour=0 in each section too?  Ideally including a test case where hour=0 *should* match something.
Attachment #8832294 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8832295 [details]
Bug 1334167: use run-on-projects to parallel task graph generation;

https://reviewboard.mozilla.org/r/108630/#review109868
Attachment #8832295 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8832296 [details]
Bug 1334167: allow by-project for cron jobs' when property;

https://reviewboard.mozilla.org/r/108632/#review109872

this new extra values piece could allow me to cleanup some of the nightly taskgraph code (where I set some description values JUST to make that work)
Attachment #8832296 - Flags: review?(bugspam.Callek) → review+
Comment on attachment 8832296 [details]
Bug 1334167: allow by-project for cron jobs' when property;

https://reviewboard.mozilla.org/r/108632/#review109874

::: .cron.yml:30
(Diff revision 1)
>            target-tasks-method: nightly_fennec
>        run-on-projects:
>            - mozilla-central
>            - date
>        when:
>            - {hour: 16, minute: 0}

This should also be by-project with teh same schedule as desktop
Comment on attachment 8832297 [details]
Bug 1334167: temporarily do not run jobs at all;

https://reviewboard.mozilla.org/r/108634/#review109876

actually this supercedes the comment with android nightly, no need to adjust on that cset.
Attachment #8832297 - Flags: review?(bugspam.Callek) → review+
Attachment #8832298 - Attachment is obsolete: true
oh, ugh, autoland failed.  I wish there was some notification of that!
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f31ddcc843e8
calculate head revision with -r .; r=Callek
https://hg.mozilla.org/integration/autoland/rev/f3ebbf9445ab
match times correctly; r=Callek
https://hg.mozilla.org/integration/autoland/rev/ae6c320b791b
use run-on-projects to parallel task graph generation; r=Callek
https://hg.mozilla.org/integration/autoland/rev/3cb163fd021d
allow by-project for cron jobs' when property; r=Callek
https://hg.mozilla.org/integration/autoland/rev/d33d1836e354
temporarily do not run jobs at all; r=Callek
In bug 1305139 Mihai mentions that the work here should uplift to aurora. Dustin, can you take a look and land whatever needs to be landed? Or request uplift if that seems best. Thanks!
Flags: needinfo?(dustin)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #32)
> In bug 1305139 Mihai mentions that the work here should uplift to aurora.
> Dustin, can you take a look and land whatever needs to be landed? Or request
> uplift if that seems best. Thanks!

:dustin 

Sorry to insist here, was wondering if you got the chance to uplift the above patches to aurora?
If not, can I get the blessing to request that in your behalf? :) They're pre-req for some uplifts I need to make in bug 1305139.
Yes, you have my blessing.  As we discussed last week, I don't know how to uplift things, so have at it!
Flags: needinfo?(dustin)
(In reply to Wes Kocher (:KWierso) from comment #29)
> https://hg.mozilla.org/mozilla-central/rev/f31ddcc843e8
> https://hg.mozilla.org/mozilla-central/rev/f3ebbf9445ab
> https://hg.mozilla.org/mozilla-central/rev/ae6c320b791b
> https://hg.mozilla.org/mozilla-central/rev/3cb163fd021d
> https://hg.mozilla.org/mozilla-central/rev/d33d1836e354

(In reply to Wes Kocher (:KWierso) from comment #31)
> https://hg.mozilla.org/mozilla-central/rev/e74dc930625c

@dustin: thanks! sorry for misunderstanding last week, didn't realize that was a cheque in white to uplift all-the-things! :)

I don't know how to "officialy uplift" either other than graft & push so I'll leave it to the pros :P
/me kindly summons lizzard - can you help us uplift these 5 + 1 patches to aurora ?

And then the two follow-up patches from bug 1305139?
Alternatively, I can do those myself, just please confirm me the right steps.
Thanks!
Flags: needinfo?(lhenry)
If you can fill out the little form that pops up when you add the flag "approval-mozilla-aurora:?" to the patches you want uplifted, that will help us keep track of what's landed where. Generally a release manager will change the flag to + and then one of the sheriffs will help land it and watch the tree for problems or test failures.    

I think because this bugzilla component doesn't have flags for firefox version, things are a bit strange. We may want to add the flags to the component.
Flags: needinfo?(lhenry)
Wes, what do you think about the component flags?  Can you go ahead and land these patches on aurora for us?
Flags: needinfo?(wkocher)
I had to leave early today for pto. I can try to grab this tomorrow if no one else gets it before then. Missing flags would be a bmo  administration bug.

Uplifts are just graft & push, then mark flags in the bug.
Comment on attachment 8832293 [details]
Bug 1334167: calculate head revision with -r .;

Approval Request Comment
[Feature/Bug causing the regression]:
-
[User impact if declined]:
-
[Is this code covered by automated tests?]:
not sure
[Has the fix been verified in Nightly?]:
Yes - all requested changes are already landed and tested in nightly.
[Needs manual test from QE? If yes, steps to reproduce]: 
no
[List of other uplifts needed for the feature/fix]:
I'll paste below the entire set of changeses so that I don't request approval for each of these patches.
[Is the change risky?]:
I don't think so.
[Why is the change risky/not risky?]:
-
[String changes made/needed]:
-

On behalf of :dustin, I'm requesting this set of patches for approval-mozillar-aurora:

* https://hg.mozilla.org/mozilla-central/rev/f31ddcc843e8
* https://hg.mozilla.org/mozilla-central/rev/f3ebbf9445ab
* https://hg.mozilla.org/mozilla-central/rev/ae6c320b791b
* https://hg.mozilla.org/mozilla-central/rev/3cb163fd021d
* https://hg.mozilla.org/mozilla-central/rev/d33d1836e354
* https://hg.mozilla.org/mozilla-central/rev/e74dc930625c

If approved, I'll take care of uplifting them to aurora.
Flags: needinfo?(wkocher)
Attachment #8832293 - Flags: approval-mozilla-aurora?
Comment on attachment 8832293 [details]
Bug 1334167: calculate head revision with -r .;

Tested in nightly, should help releng with in-tree taskcluster stuff.
Attachment #8832293 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.