Last Comment Bug 785636 - Automated Blocklist Update (for tree checkin) failing on all approval-required trees
: Automated Blocklist Update (for tree checkin) failing on all approval-require...
Status: RESOLVED FIXED
:
Product: Release Engineering
Classification: Other
Component: General Automation (show other bugs)
: other
: All Windows 7
: P2 major (vote)
: ---
Assigned To: Chris Cooper [:coop]
: Chris AtLee [:catlee]
Mentors:
Depends on: 791492
Blocks: 637444
  Show dependency treegraph
 
Reported: 2012-08-25 10:57 PDT by Justin Wood (:Callek)
Modified: 2013-08-12 21:54 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
-
+
fixed


Attachments
Add distinct -a option for setting blocklist update approval (1.68 KB, patch)
2012-09-11 09:57 PDT, Chris Cooper [:coop]
no flags Details | Diff | Review
Set blocklist_update approval by default (buildbot-configs) (1.68 KB, patch)
2012-09-11 09:57 PDT, Chris Cooper [:coop]
bugspam.Callek: review+
coop: checked‑in+
Details | Diff | Review
Pass -a option when blocklist_update_set_approval is set (buildbotcustom) (882 bytes, patch)
2012-09-11 09:58 PDT, Chris Cooper [:coop]
bugspam.Callek: review+
coop: checked‑in+
Details | Diff | Review
Add distinct -a option for setting blocklist update approval (2.83 KB, patch)
2012-09-11 10:00 PDT, Chris Cooper [:coop]
bugspam.Callek: review+
coop: checked‑in+
Details | Diff | Review

Description Justin Wood (:Callek) 2012-08-25 10:57:28 PDT
We currently have the automated blocklist update script failing on all our approval required trees, that would be beta and aurora. For both TB and Firefox.

Consequences of this being that end-users who install Firefox/TB may be vulnerable to crashy/malicious plugins for the time it takes the application to automatically contact AMO and download the up to date blocklist version.

e.g.

hg -R blocklist commit -u ffxbld -m Automated blocklist update from host linux-ix-slave13
hg -R blocklist push -e ssh -l ffxbld -i ~cltbld/.ssh/ffxbld_dsa ssh://hg.mozilla.org/releases/mozilla-beta
pushing to ssh://hg.mozilla.org/releases/mozilla-beta
searching for changes
remote: Warning: Permanently added the RSA host key for IP address '63.245.215.25' to the list of known hosts.
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: Pushing to an APPROVAL REQUIRED tree requires your top changeset comment to include: a=... (or, more accurately, a\S*=...)
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.a_treeclosure hook failed
ERROR hg push exited with exit code: 1, probably raced another changeset
hg -R blocklist rollback
rolling back to revision 100558 (undo commit)
Comment 1 Phil Ringnalda (:philor) 2012-08-25 11:33:28 PDT
That'd be because bug 637444 stopped doing CLOSED TREE, and sync-hg-blocklist.sh has a single flag for adding "CLOSED TREE a=blocklist-update".

Assuming that was accidental rather than intentional (aurora and beta and release didn't actually get mentioned there), there's three problems:

* approval and closed need to be separate flags

* release would need both, except

* release really needs to stop being CLOSED at all times, because that makes it impossible to close it - there are times when it should be closed not only to accidental pushes to it, but also to things which are explicitly approved for it (and to blocklist updates), which is impossible with the way it misuses CLOSED to mean APPROVAL REALLY REALLY REQUIRED
Comment 2 Alex Keybl [:akeybl] 2012-08-26 16:12:41 PDT
Can you all give us an idea for how long this has been happening (since bug 637444?), and whether this also affected the FF15 release build?

In the meantime, can we easily update the blocklist on beta/release manually prior to building?

(In reply to Justin Wood (:Callek) from comment #0)
> Consequences of this being that end-users who install Firefox/TB may be
> vulnerable to crashy/malicious plugins for the time it takes the application
> to automatically contact AMO and download the up to date blocklist version.

My biggest concern from the release standpoint is not those ~24 hours till the first blocklist ping (leaving the user vulnerable), but rather add-ons/plugins that we've blocklisted because they cause the user to startup crash after installation.

(In reply to Phil Ringnalda (:philor) from comment #1)
> * release really needs to stop being CLOSED at all times, because that makes
> it impossible to close it - there are times when it should be closed not
> only to accidental pushes to it, but also to things which are explicitly
> approved for it (and to blocklist updates), which is impossible with the way
> it misuses CLOSED to mean APPROVAL REALLY REALLY REQUIRED

If changing the mozilla-release branch to APPROVAL REQUIRED helps in any way, I have no issue with doing that.
Comment 3 Justin Wood (:Callek) 2012-08-26 17:13:41 PDT
(In reply to Alex Keybl [:akeybl] from comment #2)
> Can you all give us an idea for how long this has been happening (since bug
> 637444?), and whether this also affected the FF15 release build?

Lets see:

Last automated checkin for this on what is now m-release (Firefox 15) was |2012-06-02 03:12 -0700| 

There was an automated landing on m-c for blocklist on |2012-06-09 03:12 -0700| (which was backed out for orange, as well as other m-c landings since then)

Last automated checkin for this on what is now m-aurora (Firefox 16) was |2012-07-14 03:11 -0700|

(next on central after that was |2012-08-04 03:24 -0700|)

So it seems like quite a while. I believe philor was right re: broken since Bug 637444

> In the meantime, can we easily update the blocklist on beta/release manually
> prior to building?

It should be quite easy for someone to update the blocklist manually, until we properly fix this bug.

> (In reply to Justin Wood (:Callek) from comment #0)
> > Consequences of this being that end-users who install Firefox/TB may be
> > vulnerable to crashy/malicious plugins for the time it takes the application
> > to automatically contact AMO and download the up to date blocklist version.
> 
> My biggest concern from the release standpoint is not those ~24 hours till
> the first blocklist ping (leaving the user vulnerable), but rather
> add-ons/plugins that we've blocklisted because they cause the user to
> startup crash after installation.

Good point, I didn't think of that situation.

> (In reply to Phil Ringnalda (:philor) from comment #1)
> > * release really needs to stop being CLOSED at all times, because that makes
> > it impossible to close it - there are times when it should be closed not
> > only to accidental pushes to it, but also to things which are explicitly
> > approved for it (and to blocklist updates), which is impossible with the way
> > it misuses CLOSED to mean APPROVAL REALLY REALLY REQUIRED
> 
> If changing the mozilla-release branch to APPROVAL REQUIRED helps in any
> way, I have no issue with doing that.

It won't fix this bug, since we never automatically [ever tried to] checkin blocklist updates to m-release anyway, and this has been failing on aurora/beta.
Comment 4 Justin Wood (:Callek) 2012-08-26 18:41:40 PDT
I just manually updated Firefox and Thunderbird on aurora with a local run of the script

hg -R blocklist push -e ssh -l Callek@gmail.com -i ~/.ssh/id_dsa ssh://hg.mozilla.org/releases/mozilla-aurora
pushing to ssh://hg.mozilla.org/releases/mozilla-aurora
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: You can view your change at the following URL:
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/2ec0163a9aa9
remote: Trying to insert into pushlog.
remote: Please do not interrupt...
remote: Inserted into the pushlog db successfully.

AND:

hg -R blocklist push -e ssh -l Callek@gmail.com -i ~/.ssh/id_dsa ssh://hg.mozilla.org/releases/comm-aurora
pushing to ssh://hg.mozilla.org/releases/comm-aurora
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: You can view your change at the following URL:
remote:   https://hg.mozilla.org/releases/comm-aurora/rev/f0f70426d010
remote: Trying to insert into pushlog.
remote: Please do not interrupt...
remote: Inserted into the pushlog db successfully.
Comment 5 Phil Ringnalda (:philor) 2012-08-26 20:04:17 PDT
We don't try to update esr10, either, but particularly in light of the "we blocklist things that cause startup crashes" thing, I'm not sure bug 663820 comment 8 is a persuasive reason to not update them both.

Though given the Friday release builds, they might be better off with either manual pushes, or better yet Thursday automatic pushes - along with minimizing the chances of getting into a push race, Saturday updates also minimize the chances of anyone noticing bustage, which thanks to our permasheriff in GMT a late Thursday/early Friday push would not.
Comment 6 Alex Keybl [:akeybl] 2012-08-27 17:35:31 PDT
We discussed this today in crash-kill, a respin of 15.0 won't be necessary given a review of recent blocklist additions.
Comment 7 Alex Keybl [:akeybl] 2012-09-04 09:17:59 PDT
catlee - who's in the best position to look at this for the next release? Thanks!
Comment 8 Chris AtLee [:catlee] 2012-09-04 12:33:19 PDT
coop touched the blocklist stuff last
Comment 9 Chris Cooper [:coop] 2012-09-04 14:25:01 PDT
Damn blocklist cooties.

I can add separate flags for closed tree and approval. Can I get a ruling as to what the default state of those flags should be for each tree where this would be running?

mozilla-central
mozilla-aurora
mozilla-beta
mozilla-release
mozilla-esr10
Comment 10 Ed Morley [:emorley] 2012-09-04 14:30:35 PDT
I've just changed *-release to approval required (from closed), since as philor correctly said, it gives us no way to close the tree otherwise.

As such, all trees apart from mozilla-central just need "a=foo", but do not need to (& must not use) CLOSED TREE. 

mozilla-central may optionally use a=foo as well, since it won't hurt anything and I'm presuming makes your life easier :-)
Comment 11 Chris Cooper [:coop] 2012-09-11 09:57:08 PDT
Created attachment 660135 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval
Comment 12 Chris Cooper [:coop] 2012-09-11 09:57:54 PDT
Created attachment 660136 [details] [diff] [review]
Set blocklist_update approval by default (buildbot-configs)
Comment 13 Chris Cooper [:coop] 2012-09-11 09:58:52 PDT
Created attachment 660137 [details] [diff] [review]
Pass -a option when blocklist_update_set_approval is set (buildbotcustom)
Comment 14 Chris Cooper [:coop] 2012-09-11 10:00:23 PDT
Created attachment 660138 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval
Comment 15 Justin Wood (:Callek) 2012-09-12 00:46:15 PDT
Comment on attachment 660138 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval

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

coop++ for escaping the quotes in the echo while you're here
Comment 16 Chris Cooper [:coop] 2012-09-12 13:24:17 PDT
Comment on attachment 660138 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval

https://hg.mozilla.org/build/tools/rev/c364b05afc33
Comment 17 Chris Cooper [:coop] 2012-09-12 13:24:49 PDT
Comment on attachment 660136 [details] [diff] [review]
Set blocklist_update approval by default (buildbot-configs)

https://hg.mozilla.org/build/buildbot-configs/rev/9b5a483c09d6
Comment 18 Chris Cooper [:coop] 2012-09-12 13:25:14 PDT
Comment on attachment 660137 [details] [diff] [review]
Pass -a option when blocklist_update_set_approval is set (buildbotcustom)

https://hg.mozilla.org/build/buildbotcustom/rev/edbc4a9f7b5d
Comment 19 Kim Moir [:kmoir] 2012-09-13 09:30:25 PDT
In production.

Note You need to log in before you can comment on or make changes to this bug.