Create command-line API for creating Bouncer/MirrorBrain entries

RESOLVED FIXED

Status

defect
P3
normal
RESOLVED FIXED
13 years ago
6 years ago

People

(Reporter: morgamic, Assigned: rail)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [automation])

Attachments

(5 attachments, 9 obsolete attachments)

33.78 KB, image/png
Details
1.92 KB, text/plain
Details
2.98 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
15.72 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
11.10 KB, patch
rail
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
Once files are pushed to stage, a human being shouldn't have to enter these files into Bouncer.  We need to automate this process so we can avoid errors in the Bouncer input.

One problem is knowing how to get from point A (the release config) to point B (knowing all possible file paths, platforms, locales, product names).

Even then, there is the question of whether or not items in bouncer should automatically be turned on, or left off by default until a human looks at it and says, "yeah, okay, ship it."

I am considering using a published feed from the build system to advertise the basics and having the bouncer app figure out the rest.  This would require less code on the build side of things, and allow for an easier transition from v1->v2/v3/whatever on the bouncer side of things.

Thoughts?
Blocks: end2end-bld
I will be working on this for bouncer1 and bouncer2.  Will entertain a messaging API but the easiest way would be to do a DB transaction (not auto-commit) for creating these entries based on l10n and bootstrap config data.  The main task is building file paths for combinations of platform/locale/product and fitting it into the schema for the corresponding app.
Assignee: build → morgamic
Status: NEW → ASSIGNED
Blocks: 371325
No longer blocks: end2end-bld
Hey guys, I've picked this us and have been looking at using the existing configs in Bootstrap to interate then insert the proper records to B1/B2 using DBI -- but I have some questions regarding how I'm going to get this all to play nicely with the existing setup:

* Where should we store database information (so it doesn't have to get tagged/checked in to CVS)?

* Should I insert the code into Stage.pm or create a separate module?  Stage.pm loads some config info already that could be reused (platform maps, locale info) -- I could optionally create a separate Bouncer.pm and have that implement Stage.pm subs.

* What is the desired behavior if Bouncer.pm/Stage.pm fails to update the Bouncer database with location/file information?

* Is there an existing setup we use to test new modules/steps?

* Do we want bouncer to start checking for these new entries automatically or do you want it to wait until an admin says, "okay, start checking now" -- either way is easy and possible.
needed for "release automation", hence marking as critical.
Severity: normal → critical
QA Contact: mozpreed → build
Talked with morgamic last week, this is likely to happen end Nov / start Dec. Setting to P3.
Priority: -- → P3
Added dependency on bug#408674, which tracks the webdev portion of this project.
(In reply to comment #0)
> Even then, there is the question of whether or not items in bouncer should
> automatically be turned on, or left off by default until a human looks at it
> and says, "yeah, okay, ship it."

I'm not sure that's the best thing to do, actually, unless we can guarantee that the moment we configure the bouncer entries is when we're cleared to ship. As I understand things, that isn't always the case. We need to pull some more people in on this to ensure we're not bypassing a QA control step.
(In reply to comment #6)
> (In reply to comment #0)
> > Even then, there is the question of whether or not items in bouncer should
> > automatically be turned on, or left off by default until a human looks at it
> > and says, "yeah, okay, ship it."
> 
> I'm not sure that's the best thing to do, actually, unless we can guarantee
> that the moment we configure the bouncer entries is when we're cleared to ship.
> As I understand things, that isn't always the case. We need to pull some more
> people in on this to ensure we're not bypassing a QA control step.

Currently, we create bouncer entries before we ship, because it takes a while for bouncer's sentry to hit the mirrors and make sure that the files exist.

Pushing to mirrors should probably be the QA-controlled gate to actually releasing, as bouncer won't work until that's done anyway. 

However, if we coul easily have bouncer entries automatically created in a "disabled" state and only turn them on when QA signs off on live testing, that'd be cool, and an improvement on what we do now.
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #0)
> > > Even then, there is the question of whether or not items in bouncer should
> > > automatically be turned on, or left off by default until a human looks at it
> > > and says, "yeah, okay, ship it."
> > 
> > I'm not sure that's the best thing to do, actually, unless we can guarantee
> > that the moment we configure the bouncer entries is when we're cleared to ship.
> > As I understand things, that isn't always the case. We need to pull some more
> > people in on this to ensure we're not bypassing a QA control step.
> 
> Currently, we create bouncer entries before we ship, because it takes a while
> for bouncer's sentry to hit the mirrors and make sure that the files exist.
> 
> Pushing to mirrors should probably be the QA-controlled gate to actually
> releasing, as bouncer won't work until that's done anyway. 
> 
> However, if we coul easily have bouncer entries automatically created in a
> "disabled" state and only turn them on when QA signs off on live testing,
> that'd be cool, and an improvement on what we do now.

Yeah, there is a difference between: 
- controlling how/who says its ok to push bits out to mirrors
- the mechanical steps we have to follow to add entries to bouncer, once we get the "ok". 

In this bug, I didnt want to change any of the control / approval stuff, only to simplify the mechanical steps. Right now, right now, we have to manually reentering a bunch of data that we have already have elsewhere in automation build process...and usually do it under time pressure... a receipt for human error every time.
Assignee: morgamic → nobody
Status: ASSIGNED → NEW
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (In reply to comment #0)
> > However, if we coul easily have bouncer entries automatically created in a
> > "disabled" state and only turn them on when QA signs off on live testing,
> > that'd be cool, and an improvement on what we do now.
> 
> Yeah, there is a difference between: 
> - controlling how/who says its ok to push bits out to mirrors
> - the mechanical steps we have to follow to add entries to bouncer, once we get
> the "ok". 
> 
> In this bug, I didnt want to change any of the control / approval stuff, only
> to simplify the mechanical steps. Right now, right now, we have to manually
> reentering a bunch of data that we have already have elsewhere in automation
> build process...and usually do it under time pressure... a receipt for human
> error every time.
Updating summary to match.
Summary: Bouncer entries should automatically be created → Release automation should create Bouncer entries
Assignee: nobody → joduinn
Found during triage. Need dep.bug.
Havent had time to work on this - putting back in the pool.
Assignee: joduinn → nobody
Component: Release Engineering → Release Engineering: Future
QA Contact: build → release
Lukas is taking this on as a Q3 goal.
Assignee: nobody → lsblakk
Component: Release Engineering: Future → Release Engineering
bug 482160 is tracking replacing Bouncer with MirrorBrain - not sure if we
still want to spend time doing this with Bouncer.
Talked to Lukas - we will document an API then code to that API.  In Q3 we should aim to have a proof of concept going but deploying into prod will take some testing that puts us into Q4.  Will attach an image explaining why this should have a layer of abstraction, but the general idea is that we'll have more consumers of this data than just MB or Bouncer and should plan for that.
Summary: Release automation should create Bouncer entries → Create command-line API for creating Bouncer/MirrorBrain entries
Any updates on this?
Could we do some scripting to do some form-filling on the existing web page?
Assignee: lsblakk → nobody
Taking myself off this bug for now since I'm not likely to get anything done on it in Q4
Assigning to John for prioritization.
Assignee: nobody → joduinn
fwiw, i hacked up a greasemonkey script to make bouncer entries less painful in most cases: http://hg.mozilla.org/users/bhearsum_mozilla.com/gm-scripts/file/tip/bouncer
Punting to Future; We've got enough other enhancements to automation in progress right now so that goal is still on track. For this specific piece, I've not been able to spend any time on this and wont be able to for a while more.
Assignee: joduinn → nobody
Severity: critical → normal
Component: Release Engineering → Release Engineering: Future
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Assignee: nobody → raliiev
Status: NEW → ASSIGNED
Depends on: 545642
Posted file tuxedo-add (obsolete) —
Attached is a first implementation which uses Tuxedo REST API for adding products and their locations.

Example usage:

./tuxedo-add \
    -l shipped-locales \
    -p zirefox \
    -v 3.5.7 \
    -o 3.5.6 \
    -b ZireFox \
    -t https://tuxedo.stage.mozilla.com/api/ \
    --username tuxedouser \
    --password tuxedopass

Some notes.

* shipped-locales is a prefetched file (example: http://hg.mozilla.org/releases/mozilla-1.9.1/raw-file/FIREFOX_3_5_8_RELEASE/browser/locales/shipped-locales) used for creating platform->language maps. Language list may vary from one platform to another (ja for win and linux, ja-JP-mac for mac)

* Naming scheme is hardcoded (based on Firefox naming scheme)

* Tested on staging Tuxedo instance and looks like working :)
Attachment #429978 - Flags: review?(bhearsum)
(In reply to comment #23)
> * shipped-locales is a prefetched file (example:
> http://hg.mozilla.org/releases/mozilla-1.9.1/raw-file/FIREFOX_3_5_8_RELEASE/browser/locales/shipped-locales)
> used for creating platform->language maps. Language list may vary from one
> platform to another (ja for win and linux, ja-JP-mac for mac)

I thought bouncer didn't need any special locale information?

> * Naming scheme is hardcoded (based on Firefox naming scheme)

Can you lay out the exact naming scheme so we can check that it fits with what other products are using?
(In reply to comment #24)
> Can you lay out the exact naming scheme so we can check that it fits with what
> other products are using?

Sure.
------------------------------------------------
Full version:

FTP directory:
ftp_path_template = '/%(product)s/releases/%(version)s/%(ftp_platform)s/%(lang)s'
Example: /firefox/releases/3.5.7/win32/af

File name:
filename_template = {'win32' : '%(brandName)s Setup %(version)s.exe',
                         'linux' : '%(product)s-%(version)s.tar.bz2',
                         'macosx': '%(brandName)s %(version)s.dmg'}

Example: 
win32: Firefox Setup 3.5.7.exe
mac:   Firefox 3.5.7.dmg
linux: firefox-3.5.7.tar.bz2

Full path examples:
/firefox/releases/3.5.7/win32/af/Firefox%20Setup%203.5.7.exe
/firefox/releases/3.5.7/mac/et/Firefox%203.5.7.dmg
/firefox/releases/3.5.7/linux-i686/zh-TW/firefox-3.5.7.tar.bz2
------------------------------------------------
Complete MAR:

complete_mar_template = \
        '/%(product)s/releases/%(version)s/update/%(ftp_platform)s/%(lang)s/%(product)s-%(version)s.complete.mar'

Examples:
/firefox/releases/3.5.7/update/win32/af/firefox-3.5.7.complete.mar
/firefox/releases/3.5.7/update/mac/cs/firefox-3.5.7.complete.mar
/firefox/releases/3.5.7/update/linux-i686/th/firefox-3.5.7.complete.mar
------------------------------------------------
Partial MAR:

partial_mar_template = \
        '/%(product)s/releases/%(version)s/update/%(ftp_platform)s/%(lang)s/%(product)s-%(old_version)s-%(version)s.partial.mar'

Examples:
/firefox/releases/3.5.7/update/win32/af/firefox-3.5.6-3.5.7.partial.mar
/firefox/releases/3.5.7/update/mac/vi/firefox-3.5.6-3.5.7.partial.mar
/firefox/releases/3.5.7/update/linux-i686/ro/firefox-3.5.6-3.5.7.partial.mar
So happy to see this bug progressing! One thing I'm really looking forward to with this code is fixing requests like bug 549874 and bug 549627. So in the fullness of time we need to support 4 Solaris platforms, codenames like Lanikai and Namoroka, and also paths like firefox/release/devpreview/1.9.3aN.
(In reply to comment #26)
> So happy to see this bug progressing! One thing I'm really looking forward to
> with this code is fixing requests like bug 549874 and bug 549627. So in the
> fullness of time we need to support 4 Solaris platforms, codenames like Lanikai
> and Namoroka, and also paths like firefox/release/devpreview/1.9.3aN.

OS list can be passed as an optional parameter, unusual paths can be handled by a config file with predefined templates. So, both things won't be hard to implement. Hope to add these features this week.

Thanks for your feedback.
> fullness of time we need to support 4 Solaris platforms, codenames like Lanikai
> and Namoroka...

BTW, this is already supported, we have --product and --brand-name parameters. ;)
(In reply to comment #24)
> (In reply to comment #23)
> > * shipped-locales is a prefetched file (example:
> > http://hg.mozilla.org/releases/mozilla-1.9.1/raw-file/FIREFOX_3_5_8_RELEASE/browser/locales/shipped-locales)
> > used for creating platform->language maps. Language list may vary from one
> > platform to another (ja for win and linux, ja-JP-mac for mac)
> 
> I thought bouncer didn't need any special locale information?

Some recent bouncer work is changing Sentry/Bouncer to poll all locales, rather than just the final one.
Comment on attachment 429978 [details]
tuxedo-add

>#!/usr/bin/env python
>
>import optparse
>from optparse import Option, OptionParser, OptionError
>import urllib2
>import urllib

Tiny nit: It's confusing at a glance to be importing both of these. Since you're only using urlencode from urllib can you just import that function?

>
># buildbot -> bouncer platform mapping
>platform_map = {'win32': 'win', 'macosx': 'osx', 'linux': 'linux'}

It doesn't hurt to be more explicit here, so let's call this bouncer_platform_map instead.

>def getPlatformLocales(shipped_locales):
>    platform_locales = {}
>    for platform in platform_map.keys():
>        platform_locales[platform] = []
>    for line in open(shipped_locales).readlines():
>        entry = line.split()
>        lang = entry[0]
>        if len(entry)>1:
>            for platform in entry[1:]:
>                platform_locales[shippedlocales2buildbot(platform)].append(lang)
>        else:
>            for platform in platform_map.keys():
>                platform_locales[platform].append(lang)
>    return platform_locales

This looks fine, but it would be *awesome* for it, and the mappings above it to end up in a library rather than stuck in this script. http://hg.mozilla.org/build/tools/file/tip/lib was recently created for just this purpose. Can you move these functions over there? I suggest lib/python/release/platforms.py as a location, but if something else makes more sense that wfm too.

>class TuxedoOption(Option):
>    ATTRS = Option.ATTRS + ['required']
>
>    def _check_required(self):
>        if self.required and not self.takes_value():
>            raise OptionError(
>                "required flag set for option that doesn't take a value",
>                 self)
>
>    # Make sure _check_required() is called from the constructor!
>    CHECK_METHODS = Option.CHECK_METHODS + [_check_required]
>
>    def process (self, opt, value, values, parser):
>        Option.process(self, opt, value, values, parser)
>        parser.option_seen[self] = 1
>
>
>class TuxedoOptionParser(OptionParser):
>
>    def _init_parsing_state(self):
>        OptionParser._init_parsing_state(self)
>        self.option_seen = {}
>
>    def check_values(self, values, args):
>        for option in self.option_list:
>            if (isinstance(option, Option) and
>                option.required and
>                not self.option_seen.has_key(option)):
>                self.print_help()
>                self.set_usage(optparse.SUPPRESS_USAGE)
>                self.error("%s not supplied" % option)
>        return (values, args)

This is the first time I've seen this, it's nice :).

>class TuxedoEntrySubmitter(object):
>
>    ftp_path_template = '/%(product)s/releases/%(version)s/%(ftp_platform)s/%(lang)s'

nit: use locale instead of lang here for consistency with other scripts/files.



>    def __init__(self, shipped_locales, productName, bouncerProductName,
>                 version, tuxedo_api_prefix, brandName=None,
>                 no_complete_mars=False, oldVersion=None, username=None,
>                 password=None, verbose=True):

A couple of nits here:
* Be consistent with camel case vs. underscores for word separation.
* no_complete_mars=False is harder to parse than complete_mars=True (or add_complete_mars). Also, I think you want to be more generic and just use add_mars. I think it makes more sense to do that than base the partial MAR decision on 'if oldVersion', like you have further down. It's possible that for an, eg, 3.7a2 oldVersion *would* be set, but may not have MARs.

>        self.platform_langs = getPlatformLocales(self.shipped_locales)

Same thing here, s/langs/locales/g



I'm happy with the logic and major parts of this, but I'd like to see it again with the comments addressed.
Attachment #429978 - Flags: review?(bhearsum) → review-
(In reply to comment #29)
> Some recent bouncer work is changing Sentry/Bouncer to poll all locales, rather
> than just the final one.

As long as it's just for polling available files of *new* releases, that sounds OK - wouldn't want to need to backfill all the info for older versions...
Posted file tuxedo-add (obsolete) —
Ben, 
First of all, thanks for your comments. All of them accepted and implemented in this version (attached). the changeset implementing your suggestion is here:
http://bitbucket.org/rail/tuxedo-client/changeset/89eb5fc4e4ef/

Another changeset adds an ability to read credentials from a file ("user:pass"), what would be useful for cronjobs (no user/pass in ps too).
http://bitbucket.org/rail/tuxedo-client/changeset/61af38db8c1b/

lib/python part and test script are following.
Attachment #429978 - Attachment is obsolete: true
Attachment #431319 - Flags: review?(bhearsum)
Posted file test-tuxedo-add
Test script
Posted file lib/python/release/platforms.py (obsolete) —
lib part
Attachment #431321 - Flags: review?(bhearsum)
Attachment #431321 - Flags: review?(bhearsum) → review+
Comment on attachment 431319 [details]
tuxedo-add

This looks good to me.
Attachment #431319 - Flags: review?(bhearsum) → review+
Depends on: 550510
Attaching the WIP buildbotcustom and buildbot-configs parts.

Outstanding issues:

* tools/lib/python should be added to PYTHONPATH somehow (global or runtime)

* A situation when new locales (platforms) added should be investigated. It may change the logic how partial MAR link is generated (no need to add partial product, for example).


The current changes pass "test-masters.sh".
Posted patch buildbot-configs changes (obsolete) — Splinter Review
Attachment #437237 - Flags: feedback?(bhearsum)
Posted patch buildbotcustom changes (obsolete) — Splinter Review
Attachment #437238 - Flags: feedback?(bhearsum)
Comment on attachment 437238 [details] [diff] [review]
buildbotcustom changes

>+        for platform in sorted(l10nPlatforms):
>+            cmd.extend(['--platform', platform])

This won't work in all cases; what happens if we ship en-US only on a platform?

>+
>+        self.addStep(ShellCommand(
>+         name='final_verification',

I think you forgot to update this name :-).

>+         command=cmd,
>+         description=['tuxedo-add.py'],
>+         workdir='tools/release'
>+        ))

You will need to set PYTHONPATH in the environment here. Probably something like
env={'PYTHONPATH': '../lib/python'}


feedback+, but the specific points need to be addressed
Attachment #437238 - Flags: feedback?(bhearsum) → feedback+
Attachment #437237 - Flags: feedback?(bhearsum) → feedback-
Comment on attachment 437237 [details] [diff] [review]
buildbot-configs changes

>diff --git a/mozilla2-staging/TuxedoCredentials.py.template b/mozilla2-staging/TuxedoCredentials.py.template
>new file mode 100644
>--- /dev/null
>+++ b/mozilla2-staging/TuxedoCredentials.py.template
>@@ -0,0 +1,2 @@
>+tuxedoUsername = 'user'
>+tuxedoPassword = 'passwd'

I loathe to have another file like this; can you merge this with an existing one?

>-majorUpdateRepoPath = None
>\ No newline at end of file
>+majorUpdateRepoPath = None
>+
>+# Tuxedo/Bouncer related
>+tuxedoConfig        = 'firefox-tuxedo.ini'

What is this file? I don't see it referenced anywhere else in this bug. Also, if it's the same across all branches do we need it in the config?

>+tuxedoAPIPrefix     = 'https://tuxedo.stage.mozilla.com/api/' # TODO: add production API prefix

This name doesn't make a whole lot of sense to me. I think tuxedoServerUrl would be more in line with similar variables in this file.

>+brandName           = 'Firefox' # filename prefix, e.g. Firefox, MozillaDeveloperPreview, Lankai

I wish we could just not pass bouncerProductName/brandName at all, unless we need to override, but since we do can you set these up like:
bouncerProductName = product.capitalize()
brandName = product.capitalize()

That way, it works in 95% of cases without needing to change multiple entries.

>+addMARs              = True

This doesn't need to be in the config at all. It can be assumed that if oldVersion != None, that you want to add the MAR entries.


>diff --git a/mozilla2-staging/release_master.py b/mozilla2-staging/release_master.py

This file looks fine, but you'll need a couple minor changes per my other comments.
Also, the builder needs to be hooked up to a Scheduler -- there's no reason we should need to use 'force build'. It really doesn't matter much which Scheduler it uses, but let's fire it after 'updates', as it seems more appropriate to do so after we generate all the files for the release.

Another thing to think of: What happens if we try to add entries a second time? Does it fail gracefully? Does it override?


You've got a good start on this.
(In reply to comment #40)
> >+brandName           = 'Firefox' # filename prefix, e.g. Firefox, MozillaDeveloperPreview, Lankai
> 
> I wish we could just not pass bouncerProductName/brandName at all, unless we
> need to override, but since we do can you set these up like:
> bouncerProductName = product.capitalize()
> brandName = product.capitalize()
> 
> That way, it works in 95% of cases without needing to change multiple entries.

Yes, that's what we're doing in all the other cases as well - as long as setting an explicit brandName is possible, that should be alright.
And if we'd set it, the brandName should go between productName and appName, IMHO (just like where I have it for SeaMonkey, which always needs it due to the capital M in the brand name).
Thanks for the comments, all of them are reasonable.

There will be some changes in Tuxedo API in the nearest future, I'll will continue this work as soon as we have new API.

>>+# Tuxedo/Bouncer related
>>+tuxedoConfig        = 'firefox-tuxedo.ini'

>What is this file? I don't see it referenced anywhere else in this bug. 

I planned to add this file as part of tools.diff. It has exactly the same content as "Example config" in tuxedo-add.

> Also, if it's the same across all branches do we need it in the config?

Only if we decide to change the naming schema. No such requirement for now.


> What happens if we try to add entries a second time?

It fails with non 2xx HTTP error code.
(In reply to comment #42)
> Thanks for the comments, all of them are reasonable.
> 
> There will be some changes in Tuxedo API in the nearest future, I'll will
> continue this work as soon as we have new API.
> 
> >>+# Tuxedo/Bouncer related
> >>+tuxedoConfig        = 'firefox-tuxedo.ini'
> 
> >What is this file? I don't see it referenced anywhere else in this bug. 

> I planned to add this file as part of tools.diff. It has exactly the same
> content as "Example config" in tuxedo-add.
>

Okay, sounds fine then.
 
> > Also, if it's the same across all branches do we need it in the config?
> 
> Only if we decide to change the naming schema. No such requirement for now.
>

It probably *does* make sense to keep it configurable, though, come to think of it.

> 
> > What happens if we try to add entries a second time?
> 
> It fails with non 2xx HTTP error code.

Okay, that's probably the best behaviour. I was mainly worried that it would silently override, or add a second set of entries.
(In reply to comment #43)
> It probably *does* make sense to keep it configurable, though, come to think of
> it.

It is configurable already. We have tuxedoConfig in builbdot-configs, this variable is being passed to the factory and then to tuxedo-add. The file (tuxedo-config.ini) is going to be a part of tools.
(In reply to comment #39)
> (From update of attachment 437238 [details] [diff] [review])
> >+        for platform in sorted(l10nPlatforms):
> >+            cmd.extend(['--platform', platform])
> 
> This won't work in all cases; what happens if we ship en-US only on a platform?

The current version uses both enUSPlatforms.
We also pass l10nPlatforms to the factory and if it is not empty we use --shipped-locales parameter (not mandatory anymore) and prefetch shipped-locales file.
 
> >+
> >+        self.addStep(ShellCommand(
> >+         name='final_verification',
> 
> I think you forgot to update this name :-).

Silly copy/paste :). Fixed.

> >+         command=cmd,
> >+         description=['tuxedo-add.py'],
> >+         workdir='tools/release'
> >+        ))
> 
> You will need to set PYTHONPATH in the environment here. Probably something
> like
> env={'PYTHONPATH': '../lib/python'}

Fixed. Tested.

Instead of passing plain user/password to the script (and get them visible in log files) I use BuildSlave.py file (see the buildbot-configs patch) transferred to the slave by FileDownload and __import__ it (see tools.diff).
Attachment #437238 - Attachment is obsolete: true
Attachment #443832 - Flags: review?(bhearsum)
Posted patch buildbot-configs changes (obsolete) — Splinter Review
(In reply to comment #40)
> (From update of attachment 437237 [details] [diff] [review])
> >diff --git a/mozilla2-staging/TuxedoCredentials.py.template b/mozilla2-staging/TuxedoCredentials.py.template
> >new file mode 100644
> >--- /dev/null
> >+++ b/mozilla2-staging/TuxedoCredentials.py.template
> >@@ -0,0 +1,2 @@
> >+tuxedoUsername = 'user'
> >+tuxedoPassword = 'passwd'
> 
> I loathe to have another file like this; can you merge this with an existing
> one?

I reused BuildSlaves.py.
 
> This name doesn't make a whole lot of sense to me. I think tuxedoServerUrl
> would be more in line with similar variables in this file.

Fixed.
 
> >+brandName           = 'Firefox' # filename prefix, e.g. Firefox, MozillaDeveloperPreview, Lankai
> 
> I wish we could just not pass bouncerProductName/brandName at all, unless we
> need to override, but since we do can you set these up like:
> bouncerProductName = product.capitalize()
> brandName = product.capitalize()

Fixed.
 
> >+addMARs              = True
> 
> This doesn't need to be in the config at all. It can be assumed that if
> oldVersion != None, that you want to add the MAR entries.

Fixed.
 
> Also, the builder needs to be hooked up to a Scheduler -- there's no reason we
> should need to use 'force build'. It really doesn't matter much which Scheduler
> it uses, but let's fire it after 'updates', as it seems more appropriate to do
> so after we generate all the files for the release.

I added a Dependent with upstream=update_verify_scheduler.
Attachment #437237 - Attachment is obsolete: true
Attachment #443833 - Flags: review?(bhearsum)
Posted patch tools changes (obsolete) — Splinter Review
(In reply to comment #40)
> (From update of attachment 437237 [details] [diff] [review])
> >+# Tuxedo/Bouncer related
> >+tuxedoConfig        = 'firefox-tuxedo.ini'
> 
> What is this file? I don't see it referenced anywhere else in this bug. Also,
> if it's the same across all branches do we need it in the config?

Added as a part of tools.diff

I also tested the current version in staging (still available on sm01) using staging Tuxedo instance and it worked as expected. Product added, complete MARs added, partial MARs aded. Than I fetched the fiels using Tuxedo API and try to download them (just HEAD) from a mirror (I used v3.6.3 which is available of FTPs). The only untested part is setry checks which are not enabled in staging Tuxedo IIRC.
Attachment #431319 - Attachment is obsolete: true
Attachment #431321 - Attachment is obsolete: true
Attachment #443834 - Flags: review?(bhearsum)
Whiteboard: [automation]
(In reply to comment #40)
> Also, the builder needs to be hooked up to a Scheduler -- there's no reason we
> should need to use 'force build'. It really doesn't matter much which Scheduler
> it uses, but let's fire it after 'updates', as it seems more appropriate to do
> so after we generate all the files for the release.

BTW, do we really want submit every release build to Bouncer? For example, we are at build4 for 3.6.4 and seems like it's not a final one.
Posted patch tools changesSplinter Review
Add pretty version support for Mac and Windows installers ("3.7a5" becomes "3.7 Alpha 5").
Attachment #443834 - Attachment is obsolete: true
Attachment #445323 - Flags: review?(bhearsum)
Attachment #443834 - Flags: review?(bhearsum)
(In reply to comment #48)
> (In reply to comment #40)
> > Also, the builder needs to be hooked up to a Scheduler -- there's no reason we
> > should need to use 'force build'. It really doesn't matter much which Scheduler
> > it uses, but let's fire it after 'updates', as it seems more appropriate to do
> > so after we generate all the files for the release.
> 
> BTW, do we really want submit every release build to Bouncer? For example, we
> are at build4 for 3.6.4 and seems like it's not a final one.

Hmmmm...what happens if we try to add an entry that already exists?
(In reply to comment #50)
> Hmmmm...what happens if we try to add an entry that already exists?

API returns non 200 status and fails. You need to manually remove the entry.
(In reply to comment #51)
> (In reply to comment #50)
> > Hmmmm...what happens if we try to add an entry that already exists?
> 
> API returns non 200 status and fails. You need to manually remove the entry.

Okay, so we shouldn't re-run then. Your patch is fine in this regard -- let's not attach this to a Scheduler.
Posted patch buildbot-configs changes (obsolete) — Splinter Review
(In reply to comment #52)
> Okay, so we shouldn't re-run then. Your patch is fine in this regard -- let's
> not attach this to a Scheduler.

Removing bouncer_submitter_scheduler in this case, refreshed patch.
Attachment #443833 - Attachment is obsolete: true
Attachment #445687 - Flags: review?(bhearsum)
Attachment #443833 - Flags: review?(bhearsum)
Attachment #445323 - Flags: review?(bhearsum) → review+
Attachment #443832 - Flags: review?(bhearsum) → review+
Comment on attachment 445687 [details] [diff] [review]
buildbot-configs changes

This looks fine, but we need to find out what the production URL is before landing.
Comment on attachment 445687 [details] [diff] [review]
buildbot-configs changes

bounceradmin.mozilla.com is what we want for production. r=me with that change.
Attachment #445687 - Flags: review?(bhearsum)
Posted patch buildbot-configs changes (obsolete) — Splinter Review
(In reply to comment #55)
> (From update of attachment 445687 [details] [diff] [review])
> bounceradmin.mozilla.com is what we want for production. r=me with that change.

Applied.
I also refreshed the patch (it was rotten a bit) and removed not needed anymore TODO notes.
Attachment #445687 - Attachment is obsolete: true
Attachment #446147 - Flags: review+
Refreshed version. Keeping r+.
Attachment #446147 - Attachment is obsolete: true
Attachment #450150 - Flags: review+
Comment on attachment 445323 [details] [diff] [review]
tools changes

changeset:   629:8cd1010eb31b
Attachment #445323 - Flags: checked-in+
Comment on attachment 443832 [details] [diff] [review]
buildbotcustom changes

changeset:   758:733870c48b77
Attachment #443832 - Flags: checked-in+
Comment on attachment 450150 [details] [diff] [review]
buildbot-configs changes

changeset:   2492:ea287f48701d
Attachment #450150 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 573887
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.