The default bug view has changed. See this FAQ.

Hazard builds uploading to wrong FTP dir

RESOLVED FIXED

Status

Release Engineering
General Automation
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
I just noticed that hazard builds upload to a directory name containing the full hash, whereas other builds use the short hash. For example,

https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/sfink@mozilla.com-1decc1b983197f97861b834797a1078d77f3647d/

instead of

https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/sfink@mozilla.com-1decc1b98319/
(Assignee)

Comment 1

3 years ago
Created attachment 8450445 [details] [diff] [review]
Use short revisions to fix try upload dir path

Sorry jlund for making you my reviewmonkey while aki's out. :-)

This turns out to be a little more important, because I'm going to need to infer the hazard output directory from the log directory, and it would be nice if they were the same.
Attachment #8450445 - Flags: review?(jlund)
(Assignee)

Comment 2

3 years ago
Created attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

Not really the same problem, but related, and it sort of matches the bug title: non-try builds do not pass through the buildid, which means that when the builds want to upload something, they have to generate their own buildid (based on the current time). I need the buildid to be predictable (specifically from information in a pulse message). Or rather, it's much better if it is predictable so I don't have to grovel through the logs.

So just pass it through. b2g_build.py grabs it out of BuildID and so will be unaffected by this change. spidermonkey_build.py and hazard_build.py both need to switch from using buildtime to buildid, and then they'll get it from here.
Attachment #8450636 - Flags: review?(jlund)
(Assignee)

Comment 3

3 years ago
Created attachment 8450637 [details] [diff] [review]
Switch from using buildtime to buildid

here's the switch to buildid

I could do this in buildb2gbase.py instead, but I don't know if maybe b2g_build.py prefers to get it from platform.ini. I could see an argument for continuing to use that.
Attachment #8450637 - Flags: review?(jlund)
(Assignee)

Updated

3 years ago
Attachment #8450636 - Attachment is obsolete: true
Attachment #8450636 - Flags: review?(jlund)
(Assignee)

Comment 4

3 years ago
Comment on attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

Oops, name collision. All 3 patches are still valid.
Attachment #8450636 - Flags: review?(jlund)
(Assignee)

Updated

3 years ago
Attachment #8450636 - Attachment is obsolete: false

Comment 5

3 years ago
Comment on attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

Review of attachment 8450636 [details] [diff] [review]:
-----------------------------------------------------------------

in-line need-info before I finish review.

::: misc.py
@@ +1579,5 @@
>                  # finished all the builders needed for this platform and
>                  # there is nothing left to do
> +                factory = makeMHFactory(config, pf,
> +                                        signingServers=secrets.get(pf.get('dep_signing_servers')),
> +                                        copy_properties=['buildid', 'builduid'],

so are we sure we want this here? Looking at copy_properties, I think what it will do is pass the same buildid and builduid to any builds that are triggered from this, e.g. nightly builds that trigger l10n schedulers: http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#6248

AFAIK, you should get the buildid and builduid passed on to spider/hazard builds if it exists. If I look at say this job: https://tbpl.mozilla.org/php/getParsedLog.php?id=43212519&full=1&branch=mozilla-central and then check out the properties in buildprops.json, the buildid and builduid are already there. Sorry if I'm mistaken, just sanity checking.

Comment 6

3 years ago
Comment on attachment 8450445 [details] [diff] [review]
Use short revisions to fix try upload dir path

Review of attachment 8450445 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm. once all the build things land in mozharness, it will be nice to clean up duplicate code, e.g. http://mxr.mozilla.org/build/search?string=def%20query_revision

::: scripts/spidermonkey_build.py
@@ +207,2 @@
>  
> +        return revision[0:12] if revision else None

I think I am going to need this splice in my BuildScript.query_revision() too. thanks for discovering this for me :)
Attachment #8450445 - Flags: review?(jlund) → review+

Comment 7

3 years ago
Comment on attachment 8450637 [details] [diff] [review]
Switch from using buildtime to buildid

Review of attachment 8450637 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8450637 - Flags: review?(jlund) → review+
(Assignee)

Comment 8

3 years ago
Comment on attachment 8450636 [details] [diff] [review]
Pass through the build ID to mozharness builds

(In reply to Jordan Lund (:jlund) from comment #5)
> Comment on attachment 8450636 [details] [diff] [review]
> Pass through the build ID to mozharness builds
> 
> Review of attachment 8450636 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> in-line need-info before I finish review.
> 
> ::: misc.py
> @@ +1579,5 @@
> >                  # finished all the builders needed for this platform and
> >                  # there is nothing left to do
> > +                factory = makeMHFactory(config, pf,
> > +                                        signingServers=secrets.get(pf.get('dep_signing_servers')),
> > +                                        copy_properties=['buildid', 'builduid'],
> 
> so are we sure we want this here?

Not sure at all.

> Looking at copy_properties, I think what
> it will do is pass the same buildid and builduid to any builds that are
> triggered from this, e.g. nightly builds that trigger l10n schedulers:
> http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#6248
> 
> AFAIK, you should get the buildid and builduid passed on to spider/hazard
> builds if it exists. If I look at say this job:
> https://tbpl.mozilla.org/php/getParsedLog.
> php?id=43212519&full=1&branch=mozilla-central and then check out the
> properties in buildprops.json, the buildid and builduid are already there.
> Sorry if I'm mistaken, just sanity checking.

I was totally cargo-culting. I searched for 'buildid' and copied everything I found without bothering to check whether I needed it. Thank you very much for the catch!
Attachment #8450636 - Attachment is obsolete: true
Attachment #8450636 - Flags: review?(jlund)
(Assignee)

Comment 9

3 years ago
(In reply to Jordan Lund (:jlund) from comment #6)
> Comment on attachment 8450445 [details] [diff] [review]
> Use short revisions to fix try upload dir path
> 
> Review of attachment 8450445 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lgtm. once all the build things land in mozharness, it will be nice to clean
> up duplicate code, e.g.
> http://mxr.mozilla.org/build/search?string=def%20query_revision

spidermonkey_build.py is badly named and hazard_build.py is totally cut & paste from it. It's a total mess, but it was hard enough getting it working that I wanted to do it separately, without worrying about generalizing things to work for the existing hazard builds too. So yes, you're absolutely right, as soon as all of this stuff works it'll be time to merge it all together.
(Assignee)

Updated

3 years ago
Attachment #8450445 - Flags: checked-in+
(Assignee)

Updated

3 years ago
Attachment #8450637 - Flags: checked-in+
(Assignee)

Comment 10

3 years ago
http://hg.mozilla.org/build/mozharness/rev/c6dd0055188d
http://hg.mozilla.org/build/mozharness/rev/081bbea1a00b
About to roll this out to production...
In production
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.