1011486 - b2g balrog submission should point at dated dirs, not latest-*

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[[e-mail] (:bhearsum)]

Otherwise the dated blobs end up wrong, and we also probably end up serving bad updates between the time new uploads happen, and when balrog submission does.

Comment 1

[[e-mail] (:bhearsum)]

Seems like we're going to have to resort to post_upload.py parsing to fix this - I'm not sure how else to discover the dated directory name...

Comment 2

[[e-mail] (:bhearsum)]

So, there's two main things here:
1) Logic to parse the output. We already have a function to do this in factory.py (https://github.com/mozilla/build-buildbotcustom/blob/master/process/factory.py#L205). If I move this to somewhere in build/tools (or somewhere else), is there some standard way of re-using it in Mozharness (besides making a copy of it). Once we have that it's easy to subclass OutputParser to use it.
2) Return stuff from run_command. Right now, the only thing it can return is the return code, or number of errors. Maybe it should be able to return an OutputParser instance or some other type of result object instead? Or is there some other way I should be looking at this (a custom log handler?). I'm open to suggestions here, I'd appreciate some input before I go down an undesirable path, Aki.

Comment 3

[Aki Sasaki (not active)]

(In reply to Ben Hearsum [:bhearsum] from comment #2)
> So, there's two main things here:
> 1) Logic to parse the output. We already have a function to do this in
> factory.py
> (https://github.com/mozilla/build-buildbotcustom/blob/master/process/factory.
> py#L205). If I move this to somewhere in build/tools (or somewhere else), is
> there some standard way of re-using it in Mozharness (besides making a copy
> of it). Once we have that it's easy to subclass OutputParser to use it.

Not in an elegant way.
We can put it in build/tools, then clone tools in an action, and then munge our sys.path and import it in an action that then creates an OutputParser.  Probably easier to check something into the mozharness repo... we've already checked 3 mozbase modules into the mozharness repo to get mozprocess idle timeouts for free.

...Or you can just build off Jordan's work :) http://hg.mozilla.org/build/mozharness/file/acb3e29f40cf/mozharness/mozilla/building/buildbase.py#l89  I recommend this!

> 2) Return stuff from run_command. Right now, the only thing it can return is
> the return code, or number of errors. Maybe it should be able to return an
> OutputParser instance or some other type of result object instead? Or is
> there some other way I should be looking at this (a custom log handler?).
> I'm open to suggestions here, I'd appreciate some input before I go down an
> undesirable path, Aki.

There are two approaches here:
1) a custom OutputParser that you send to run_command().  You'll still have this object after the run_command() exits, and it can save whatever information you want, either via object.method() or object.variable .
2) You can run self.get_output_from_command().  If you want this parsed, you can then run the output you receive back through an OutputParser, or you can do whatever else you want with anything you can do to text via python.

Does this all make sense?

Comment 4

[[e-mail] (:bhearsum)]

We talked on IRC a bit. I'm going to try using MakeUploadOutputParser to get the URL. I also need to add --mar-url so that submit-to-balrog can still be run without needing to run upload in the some invocation. Also, we can get rid of all of the mar URL stuff from the gecko configs because we won't be using it anymore (we're getting it from the post_upload.py output instead).

Comment 5

[[e-mail] (:bhearsum)]

Attachment: get mar name from upload step or config

Not too much to say here. MakeUploadOutputParser pretty much worked out of box, except I had to adjust the complete MAR matching (also, eval() - wtf!). Dropping the "complete" from this should be fine, because the partial matching is more precise.

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 8426998 [details] [diff] [review]
get mar name from upload step or config

>diff --git a/mozharness/mozilla/building/buildbase.py b/mozharness/mozilla/building/buildbase.py
>index 106fbad..4cfdf4b 100755
>--- a/mozharness/mozilla/building/buildbase.py
>+++ b/mozharness/mozilla/building/buildbase.py
>@@ -94,17 +94,17 @@ class MakeUploadOutputParser(OutputParser):
>         # key: property name, value: condition
>         ('symbolsUrl', "m.endswith('crashreporter-symbols.zip') or "
>                        "m.endswith('crashreporter-symbols-full.zip')"),
>         ('testsUrl', "m.endswith(('tests.tar.bz2', 'tests.zip'))"),
>         ('unsignedApkUrl', "m.endswith('apk') and "
>                            "'unsigned-unaligned' in m"),
>         ('robocopApkUrl', "m.endswith('apk') and 'robocop' in m"),
>         ('jsshellUrl', "'jsshell-' in m and m.endswith('.zip')"),
>-        ('completeMarUrl', "m.endswith('.complete.mar')"),
>+        ('completeMarUrl', "m.endswith('.mar')"),
>         ('partialMarUrl', "m.endswith('.mar') and '.partial.' in m"),
>     ]

With this change, don't we need to put partialMarUrl before completeMarUrl?
Otherwise completeMarUrl may match partial.

>+    def query_complete_mar_url(self):
>+        if "complete_mar_url" in self.config:
>+            return self.config["complete_mar_url"]
>+        if "completeMarUrl" in self.package_urls:
>+            return self.package_urls["completeMarUrl"]
>+        self.fatal("Couldn't find complete mar url in config or package_urls")

Awesome, this fits right into the other helper methods.

r+ with the partial/complete mar lines switched.

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 8426998 [details] [diff] [review]
get mar name from upload step or config

(In reply to Aki Sasaki [:aki] from comment #6)
> Comment on attachment 8426998 [details] [diff] [review]
> get mar name from upload step or config
> 
> >diff --git a/mozharness/mozilla/building/buildbase.py b/mozharness/mozilla/building/buildbase.py
> >index 106fbad..4cfdf4b 100755
> >--- a/mozharness/mozilla/building/buildbase.py
> >+++ b/mozharness/mozilla/building/buildbase.py
> >@@ -94,17 +94,17 @@ class MakeUploadOutputParser(OutputParser):
> >         # key: property name, value: condition
> >         ('symbolsUrl', "m.endswith('crashreporter-symbols.zip') or "
> >                        "m.endswith('crashreporter-symbols-full.zip')"),
> >         ('testsUrl', "m.endswith(('tests.tar.bz2', 'tests.zip'))"),
> >         ('unsignedApkUrl', "m.endswith('apk') and "
> >                            "'unsigned-unaligned' in m"),
> >         ('robocopApkUrl', "m.endswith('apk') and 'robocop' in m"),
> >         ('jsshellUrl', "'jsshell-' in m and m.endswith('.zip')"),
> >-        ('completeMarUrl', "m.endswith('.complete.mar')"),
> >+        ('completeMarUrl', "m.endswith('.mar')"),
> >         ('partialMarUrl', "m.endswith('.mar') and '.partial.' in m"),
> >     ]
> 
> With this change, don't we need to put partialMarUrl before completeMarUrl?
> Otherwise completeMarUrl may match partial.

Whoops, good catch! Landed with that changed.

Comment 8

[Justin Wood (:Callek)]

Something here went live today

Comment 9

[[e-mail] (:bhearsum)]

Looks like this worked. Eg:
<updates><update type="minor" version="32.0a1" extensionVersion="32.0a1" buildID="20140523040203"><patch type="complete" URL="http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/2014/05/2014-05-23-04-02-03-mozilla-central-flame/b2g-flame-gecko-update.mar" hashFunction="sha512" hashValue="7cab3ff906134125556058998fba7a4b1dd96ebc78f8efed83d95594eaf4296553a95693d5d946dfb2bc31426f240a18ee38c3b2a63da9ae32d66ecb6594e04f" size="62143903"/></update></updates>

Comment 10

[[e-mail] (:bhearsum)]

This blew up hamachi and nexus nightlies because balrog submission couldn't find any mar url (because we don't upload those mars publicly yet). Bustage fix incoming.

Comment 11

[[e-mail] (:bhearsum)]

Attachment: fallback to even older style mar url generation

Untested, but I think this should do it. This is a nightly only change, so I don't think there's any point is spinning an entire staging build before landing...

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 8427790 [details] [diff] [review]
fallback to even older style mar url generation

Landed on default+production. Retriggered the failed nightlies.

Comment 13

[[e-mail] (:bhearsum)]

The patch fixed them \o/