Having multiple sets of release builders/schedulers for different branches won't work with schedulerdb, since the builders and schedulers for different branches all use the same name. A quick fix is to have the scheduler and builder names include the branch name so we get uniqueness. A better solution is to have the release builders be more data-driven...sort of related to bug 502876 and bug 505318.

Taking to test out a hypothetical fix in my head (releaseBuilderSuffix)

Ok, went the releaseBuilderPrefix route so they sort together. Changing the directories may involve some clobberer/purge_builds changes as well. This checkconfigs ok; submitting for feedback before I go through a long test cycle.

Comment on attachment 436589 [details] [diff] [review] releaseBuilder*Prefix* patch Most of the details here seem ok. A couple of things: I'm going back and forth on whether 3.6.x makes sense or using the full, exact version. What do you guys think about that? If we go with the former, I don't think it belongs in the configs -- it's easy enough to calculate in release_master.py.

The reason I'd put it in the configs is so we can have things like Lorentz with special names. Also to put mobile in the mobile name, if that's desired. But not really feeling strongly either way.

(In reply to comment #4) > The reason I'd put it in the configs is so we can have things like Lorentz with > special names. Also to put mobile in the mobile name, if that's desired. But > not really feeling strongly either way. Things like Lorentz also have special version numbers, so it's not necessary because of them IMHO. If names are preferred to version numbers, maybe we could use the last part of sourceRepoPath as the prefix. As for Mobile, those builders are already prefixed with "mobile", so my suggestion of using 'version' would work without the possibility of mobile/desktop colliding. Either way, I don't think there's any reason to introduce yet another variable here -- there's already at least 2 or 3 pieces of information in the configs that could be used for this.

Comment on attachment 436589 [details] [diff] [review] releaseBuilder*Prefix* patch So, I'm marking this feedback- because I don't see any reason why we need to introduce yet another config variable. Any approach that creates the prefix based on existing data would be fine with me, in principle.

Comment on attachment 436589 [details] [diff] [review] releaseBuilder*Prefix* patch I like using the 'releaseBuilderPrefix', but I'd have it use some other variable to start with. maybe releaseBuilderPrefix = 'release_%s' % sourceRepoName is a good starting point? The release clobberer code will have to be updated as well.

Adding release_config skeletons and release portions of master.cfg to the mozilla bucket in buildbot-configs.

Necessary changes to make buildbot-080 branch work with release jobs. In order to make this work, the following couple of change sets from the default branch were rolled in: 889,907,910,940,941

Applied patch 6e717ac10e28059b0315 from upstream to fix issue with sourcestamp revision being dropped in buildbot-080

Comment on attachment 479479 [details] [diff] [review] [tested]buildbotcustom-release-080 Looks good to me. Needs some extra eyes though.

Comment on attachment 479473 [details] [diff] [review] [tested]buildbot-configs_release-080 We need to get rid of the "if True:" clause. I think putting some variable in master_localconfig.py which lets us turn the release stuff on/off per master would be the best way to go right now.

Something like this? Seems a bit clunky to put these in master_localconfig. Perhaps it might be more intuitive to put them in release_config, (but not in the releaseConfig dict), since 'staging' doesn't seem release-specific

Comment on attachment 479895 [details] [diff] [review] buildbot-configs_release-080 Yeah, that's better...We can't put it in release_config, since we don't know if we _should_ be loading release_config yet. Maybe just change the names to make it more clear that these are 'special' variables? ENABLE_RELEASES, and ENABLE_STAGING_RELEASES (or maybe ENABLE_PRODUCTION_RELEASES instead?) Initially all the localconfigs, except maybe the staging ones, should have releases disabled.

Comment on attachment 479479 [details] [diff] [review] [tested]buildbotcustom-release-080 Tagging is going to change really soon, when I land bug 508896, but I'll take care of porting that to 0.8.0. Looks good though!

Comment on attachment 479895 [details] [diff] [review] buildbot-configs_release-080 I see staging=True in a bunch of production localconfig's, definitely don't want that! Just to make sure I understand correctly: the schedulers and change sources for a release will live on builder masters -- not scheduler masters, so that the release stays on a single master? Why do the scheduler master localconfigs need to be touched at all? It doesn't look like the scheduler masters are going to be aware at all of releases.

(In reply to comment #17) > Comment on attachment 479895 [details] [diff] [review] > buildbot-configs_release-080 > > I see staging=True in a bunch of production localconfig's, definitely don't > want that! > yup, removed. ENABLE_RELEASES and ENABLE_STAGING_RELEASES which control that functionality default to false in all localconfigs now. I assume we don't want release builders to be on by default in staging? > Just to make sure I understand correctly: the schedulers and change sources for > a release will live on builder masters -- not scheduler masters, so that the > release stays on a single master? > > Why do the scheduler master localconfigs need to be touched at all? It doesn't > look like the scheduler masters are going to be aware at all of releases. I didn't look too closely at that before, since having those declarations in there doesn't do anything by themselves, hence can't break anything. You're right though, no need for it to be there if it isn't needed.

(In reply to comment #19) > Created attachment 480624 [details] [diff] [review] > [tested]buildbot-configs_release-080 > > (In reply to comment #17) > > Comment on attachment 479895 [details] [diff] [review] [details] > > buildbot-configs_release-080 > > > > I see staging=True in a bunch of production localconfig's, definitely don't > > want that! > > > yup, removed. ENABLE_RELEASES and ENABLE_STAGING_RELEASES which control that > functionality default to false in all localconfigs now. I assume we don't want > release builders to be on by default in staging? Actually, I think we do. > > Just to make sure I understand correctly: the schedulers and change sources for > > a release will live on builder masters -- not scheduler masters, so that the > > release stays on a single master? > > > > Why do the scheduler master localconfigs need to be touched at all? It doesn't > > look like the scheduler masters are going to be aware at all of releases. > I didn't look too closely at that before, since having those declarations in > there doesn't do anything by themselves, hence can't break anything. You're > right though, no need for it to be there if it isn't needed. Alright, let's remove them, then.

Comment on attachment 480624 [details] [diff] [review] [tested]buildbot-configs_release-080 We definitely want releases enabled on all of the builder masters, here's what I think you should do: - drop ENABLE_RELEASES altogether and always add the release stuff - rename ENABLE_STAGING_RELEASES to 'STAGING' Does that make sense?

Ok, replaced the ENABLE_STAGING_RELEASES with just STAGING, and added release-configs for production builders, and enabled production releases, and enabled staging releases on the staging masters. I didn't drop ENABLE_RELEASES, since it needs to be off on try

I'm going to try a staging run today.

Comment on attachment 481942 [details] [diff] [review] [tested]buildbot-configs_release-080 Hmmm, I just noticed that the release configs aren't named properly. release_config_[spb]m0?.py should all be symlinks that point to the real configs, which should be named according to branch like in mozilla2/. Gotta switch this to r-.

Hmm, good point. I didn't notice the branch-specific configs before, since I those symlinks were not built in setup-master. I've added the per master symlinks to be tracked in hg as well,

Adding a version of the buildbot-configs patch which has releases disabled on production masters by default. Staging releases are still enabled on staging masters in this patch

(Going to start dropping notes from my staging runs) Can't hardcode "'slavenames': branchConfig['platforms']['macosx']['slaves']" for l10n verify anymore. Probably best to change it to macosx + macosx64 'updates' builder doesn't have release prefix

(In reply to comment #28) > > 'updates' builder doesn't have release prefix Oops, this was my fault.

Comment on attachment 483569 [details] [diff] [review] reshuffle release_configs Per IRC, we may flip off ENABLE_RELEASES in production for now. The release configs are out of date, too, but I guess we can fix that at check-in time -- I don't think it's worth the effort trying to keep them up to date before that.

Comment on attachment 483481 [details] [diff] [review] fixing bitrot in buildbotcustom patch Need to fix the slave lists for l10n verify and partner repacks -- I think both should be using macosx + macosx64 slaves.

Fixed l10n_verify and partner-repacks now get slavenames for macosx + macosx64.

Comment on attachment 483569 [details] [diff] [review] reshuffle release_configs Yeah, looks good as long as ENABLE_RELEASES are off in production for now.

Comment on attachment 484498 [details] [diff] [review] extend slave lists to include 64-bit slaves for l10n-verify and partner-repack changeset: 1088:1501598455c8 toot toot, here comes the 0.8.0 release train!

Comment on attachment 483968 [details] [diff] [review] disable releases on production amsters for now changeset: 3198:420ddf8f7adc

Switch to using reserved slaves for release-builds, reading the number of slaves to reserve from `reserved_slaves_${master_name}`.

Moving watchReservedFile() call into the config file, based on IRC conversation with catlee.

Comment on attachment 485797 [details] [diff] [review] set up reservedSlaves files for release masters I think you need to protect the watchReservedFile call behind an 'if RESERVED_SLAVES:'. Looks fine to me otherwise.

Good point. I was letting this slide since _readReservedFile just returns 0 if the name doesn't exist, but it's a waste of time to call it every minute if the filename isn't even set.

I think watchReservedFile may need some work to be reconfig-safe (or testing at least)

The global LoopingCall was getting overwritten on reconfig, but reactor still had a reference to it and kept it running. This version of the patch forgoes a looping call and opportunistically reads the file when _nextReservedSlaves is called if the last access time is > 60 seconds ago.

Comment on attachment 486184 [details] [diff] [review] make readReservedSlaves reconfig-safe What are all the changes on ftppoller from?

Comment on attachment 486185 [details] [diff] [review] use reconfig-safe method of polling reserved-slaves file unrelated changes in this patch too?

sorry for the error, previous patch got mangled with other work

sorry, accidentally mangled this patch with other work

Comment on attachment 486347 [details] [diff] [review] un-mangle buildbotcustom patch to add reserved slaves I think we need some code to handle the case here _reservedFileName is None. Inside _readReservedFile should work.

Comment on attachment 486347 [details] [diff] [review] un-mangle buildbotcustom patch to add reserved slaves Waiting on new patch, removing review.

Right, I mistakenly assumed the os.path.exists() call would catch that case, and that the guard to set RESERVED_SLAVES would otherwise.

landing the reserved-slaves patch should only require a reconfig.

Comment on attachment 486478 [details] [diff] [review] guard _readReservedFile for case where a Nonetype is passed changeset: 1197:996956fdbe83

Comment on attachment 486344 [details] [diff] [review] un-mangle configs patch changeset: 3333:2bc23626e94e

All done here. Turning on in production will be tracked elsewhere.