Closed
Bug 468731
Opened 16 years ago
Closed 16 years ago
talos testing of builds using sendchange
Categories
(Release Engineering :: General, defect, P3)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: anodelman, Assigned: catlee)
References
Details
Attachments
(8 files, 7 obsolete files)
(deleted),
patch
|
bhearsum
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
anodelman
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
anodelman
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhearsum
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
anodelman
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
With the ftpPoller (put in use to help fix bug 457885) it should be possible to test an arbitrary build by using a sendchange to the appropriate builder. This should be tested and documentation written up.
Reporter | ||
Comment 1•16 years ago
|
||
From looking over the code, it seems like a more reasonable approach would be through the forcebuild code. This is already used by l10n to force building of specific locales and we could adapt the code to talos' requirements.
bhearsum, what do you think?
Summary: force talos testing of builds using sendchange → force talos testing of builds using forcebuild
Comment 2•16 years ago
|
||
If you want the actual 'Force Build' and 'Rebuild' buttons to work you must remove all code that pulls in data from Change objects. Builds triggered through Schedulers often have these (and always do, in the Talos case) and that's where we pull the URL and filename from. 'Force Build' and 'Rebuild' do indeed use the forcebuild code, but there is currently no way to pass in any additional data - so a step like MozillaWget will fail when trying to pull the filePath out. I'm not entirely sure what the right approach for this is - perhaps passing in the filePath to TalsoFactory so it can pass it to MozillaWget instead of pulling it from the Change.
On the other hand, if you only want sendchange to work....you may not even need any code changes. Try something like:
buildbot sendchange --username="http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fx-linux-tbox-mozilla1.9.0/1229432940/" --master=localhost:9988 --branch=HEAD_LINUX -m"forced build" firefox-3.0.6pre.en-US.linux-i686.tar.bz2
Reporter | ||
Updated•16 years ago
|
Priority: -- → P3
Assignee | ||
Comment 3•16 years ago
|
||
I think we can do this with sendchange.
Assignee: anodelman → catlee
Assignee | ||
Comment 4•16 years ago
|
||
My work so far on sendchange support for Talos.
In talos-staging, we don't need the ftppollers any more, so they get removed, and replaced with a single PBChangeSource.
perfrunner.py now expects that the full URL is passed as the filename in the change object, and it derives the filename from the URL.
In mozilla2-staging we add which talos master to send sendchange requests to, and then pass that in to the NightlyBuildFactory.
Assignee | ||
Comment 5•16 years ago
|
||
if talosMasters has been set, we do a buildbot sendchange to each master after successfully uploading a build.
This relies on changes to post_upload.py to print out the URL where the build can be downloaded from.
Assignee | ||
Comment 6•16 years ago
|
||
Needed to add d:\mozilla-build\python25\scripts to $PATH to get this working on windows.
Assignee | ||
Comment 7•16 years ago
|
||
This uses a new build step called SendChangeStep to do the sendchange directly from the master.
The step uses twisted deferreds, so it won't block in the case where the remote master isn't accessible.
Attachment #362101 -
Attachment is obsolete: true
Attachment #364162 -
Flags: review?(bhearsum)
Assignee | ||
Comment 8•16 years ago
|
||
The changes to mozilla2-staging merely add the staging talos master to the list of talos masters to notify when new builds are completed.
The changes to talos-staging are more involved:
- The complete URL to the build is stored as the first (and only) entry in the changes.files property.
- ftppoller.py is updated to store the complete URL only in the files property, and to not use the links property
- perfrunner.py determines the filename from the URL with os.path.basenamae
- A new argument, 'workdirBase' is added to the TalosFactory, which represents where the talos data should be placed in relation to the build directory.
- master.cfg contains the configuration for the new slaves, as well as the new builders and schedulers to handle having the slaves doing tests on multiple branches.
Attachment #362099 -
Attachment is obsolete: true
Attachment #364172 -
Flags: review?(bhearsum)
Assignee | ||
Updated•16 years ago
|
Attachment #364172 -
Flags: review?(anodelman)
Assignee | ||
Comment 9•16 years ago
|
||
We have to print out to stderr since upload.py in the source tree eats stdout from post_upload.py.
Attachment #364174 -
Flags: review?(bhearsum)
Assignee | ||
Updated•16 years ago
|
Summary: force talos testing of builds using forcebuild → talos testing of builds using sendchange
Updated•16 years ago
|
Attachment #364162 -
Flags: review?(bhearsum) → review-
Comment 10•16 years ago
|
||
Comment on attachment 364162 [details] [diff] [review]
do buildbot sendchange after a successful build
>diff -r 625b8ed5603b process/factory.py
> class NightlyBuildFactory(MercurialBuildFactory):
>+ def __init__(self, talosMasters=None, **kwargs):
>+ if talosMasters is None:
>+ self.talosMasters = []
Why not just set [] as the default?
>- self.addStep(ShellCommand,
>+ def get_url(rc, stdout, stderr):
>+ m = re.search("^(http://.*?\.(tar\.bz2|dmg|zip))", "\n".join([stdout, stderr]), re.M)
>+ if m:
>+ return {'packageUrl': m.group(1)}
>+ return {}
>+
>+ self.addStep(SetProperty,
> command=['make', 'upload'],
> env=uploadEnv,
>- workdir='build/%s' % self.objdir
>+ workdir='build/%s' % self.objdir,
>+ extract_fn = get_url,
> )
>
>+ talosBranch = "%s-%s" % (self.branchName, self.platform)
>+ for m in self.talosMasters:
>+ self.addStep(SendChangeStep(
>+ master = m,
>+ branch = talosBranch,
>+ files = [WithProperties('%(packageUrl)s')],
>+ user = "sendchange")
>+ )
>+
style nit: please use the same formatting as the rest of the steps (no space on either side of the '=', one space indent for parameters).
>+class SendChangeStep(BuildStep):
>+ flunkOnFailure = True
I think warnOnFailure is more appropriate here. This step failing is bad, but it doesn't indicate a problem with compilation or one of the post compilation tests. Can you use warnOnFailure instead?
One overall thing I noticed is that it seems to be possible for a sendchange to get done that has no files in it. If for some reason SetProperty() doesn't find a match it will quietly return an empty dict, and it appears SendChangeStep will happily pass that along. It doesn't look like the Talos side of things copes with it either:
> + self.fileURL = self.changes[-1].files[0]
would cause an exception if files is empty. I could be missing something here, though.
This looks great overall, though, and would be a fantastic improvement over our current system.
r- because of the flunkOnFailure and the empty files. Would like the style nits fixed too.
Comment 11•16 years ago
|
||
Comment on attachment 364174 [details] [diff] [review]
Print out URL of build after upload
>diff -r 2ba74001ce81 stage/post_upload.py
>--- a/stage/post_upload.py Tue Feb 24 12:10:02 2009 -0500
>+++ b/stage/post_upload.py Wed Feb 25 17:08:06 2009 -0500
>@@ -4,16 +4,17 @@
> # followed by a list of filenames.
>
> import sys, os, os.path, shutil
> from optparse import OptionParser
> from time import mktime, strptime
>
> NIGHTLY_PATH = "/home/ftp/pub/%(product)s/nightly"
> TINDERBOX_BUILDS_PATH = "/home/ftp/pub/%(product)s/tinderbox-builds/%(tinderbox_builds_dir)s"
>+TINDERBOX_URL_PATH = "http://staging-stage.build.mozilla.org/pub/mozilla.org/%(product)s/tinderbox-builds/%(tinderbox_builds_dir)s"
Obviously this has to change to stage.mozilla.org later on. This looks fine. Please make sure to update the checkouts on stage.m.o and staging-stage.b.m.o when you land this.
Attachment #364174 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 12•16 years ago
|
||
Attachment #364162 -
Attachment is obsolete: true
Attachment #364182 -
Flags: review?(bhearsum)
Assignee | ||
Comment 13•16 years ago
|
||
(In reply to comment #10)
> (From update of attachment 364162 [details] [diff] [review])
> >diff -r 625b8ed5603b process/factory.py
>
> > class NightlyBuildFactory(MercurialBuildFactory):
> >+ def __init__(self, talosMasters=None, **kwargs):
> >+ if talosMasters is None:
> >+ self.talosMasters = []
>
> Why not just set [] as the default?
Using [] (or any mutable object) as the default is risky. See http://docs.python.org/reference/compound_stmts.html#function-definitions
> >+ talosBranch = "%s-%s" % (self.branchName, self.platform)
> >+ for m in self.talosMasters:
> >+ self.addStep(SendChangeStep(
> >+ master = m,
> >+ branch = talosBranch,
> >+ files = [WithProperties('%(packageUrl)s')],
> >+ user = "sendchange")
> >+ )
> >+
>
> style nit: please use the same formatting as the rest of the steps (no space on
> either side of the '=', one space indent for parameters).
Fixed
> >+class SendChangeStep(BuildStep):
> >+ flunkOnFailure = True
>
> I think warnOnFailure is more appropriate here. This step failing is bad, but
> it doesn't indicate a problem with compilation or one of the post compilation
> tests. Can you use warnOnFailure instead?
Fixed
> One overall thing I noticed is that it seems to be possible for a sendchange to
> get done that has no files in it. If for some reason SetProperty() doesn't find
> a match it will quietly return an empty dict, and it appears SendChangeStep
> will happily pass that along. It doesn't look like the Talos side of things
> copes with it either:
> > + self.fileURL = self.changes[-1].files[0]
>
> would cause an exception if files is empty. I could be missing something here,
> though.
I think we'll get an exception in the SendChangeStep when it tries to render the property that doesn't exist. Not ideal either, but I don't think talos would receive an empty list of files.
Maybe SendChangeStep could catch the error and report failed instead of raising an exception?
Comment 14•16 years ago
|
||
(In reply to comment #13)
> Maybe SendChangeStep could catch the error and report failed instead of raising
> an exception?
That sounds good.
Assignee | ||
Comment 15•16 years ago
|
||
Attachment #364182 -
Attachment is obsolete: true
Attachment #364182 -
Flags: review?(bhearsum)
Assignee | ||
Updated•16 years ago
|
Attachment #364320 -
Flags: review?(bhearsum)
Updated•16 years ago
|
Attachment #364320 -
Flags: review?(bhearsum) → review+
Updated•16 years ago
|
Attachment #364172 -
Flags: review?(bhearsum) → review+
Comment 16•16 years ago
|
||
Comment on attachment 364172 [details] [diff] [review]
changes to buildbot-configs for talos sendchange support
Everything looks in order here to me...
Assignee | ||
Comment 17•16 years ago
|
||
Comment on attachment 364174 [details] [diff] [review]
Print out URL of build after upload
changeset: 232:eb58ab4a33a8
Updated the TINDERBOX_URL_PATH to the http://stage.mozilla.org url, and added a commented-out line with the staging URL.
Attachment #364174 -
Flags: checked‑in+
Reporter | ||
Comment 18•16 years ago
|
||
Comment on attachment 364172 [details] [diff] [review]
changes to buildbot-configs for talos sendchange support
Doesn't this mix in a bunch of the pool of slaves code? Other than that I'm fine with it.
Assignee | ||
Comment 19•16 years ago
|
||
same as before, minus the talos pool of slaves stuff
Attachment #364172 -
Attachment is obsolete: true
Attachment #364582 -
Flags: review?(bhearsum)
Attachment #364172 -
Flags: review?(anodelman)
Assignee | ||
Updated•16 years ago
|
Attachment #364582 -
Flags: review?(anodelman)
Reporter | ||
Updated•16 years ago
|
Attachment #364582 -
Flags: review?(anodelman) → review+
Updated•16 years ago
|
Attachment #364582 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 20•16 years ago
|
||
Comment on attachment 364582 [details] [diff] [review]
changes to buildbot-configs for talos sendchange support
changeset: 968:8eff7e0d4026
Attachment #364582 -
Flags: checked‑in+
Assignee | ||
Comment 21•16 years ago
|
||
Comment on attachment 364320 [details] [diff] [review]
do buildbot sendchange after a successful build
changeset: 207:92c831b902c3
Attachment #364320 -
Flags: checked‑in+
Assignee | ||
Comment 22•16 years ago
|
||
Attachment #365288 -
Flags: review?(bhearsum)
Updated•16 years ago
|
Attachment #365288 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 23•16 years ago
|
||
Comment on attachment 365288 [details] [diff] [review]
Do buildbot sendchange from production to staging talos
Checked in with one minor change: a new port number on qm-buildbot01.mozilla.org
changeset: 993:59b331539b33
Attachment #365288 -
Flags: checked‑in+
Assignee | ||
Comment 24•16 years ago
|
||
This depends on 458243 is as much as we don't want the talos linux slaves trying to test the 64-bit linux builds.
Depends on: 458243
Assignee | ||
Comment 25•16 years ago
|
||
Change to use a list of talos masters, since we're going to be doing that on production soon.
Stop using ftppoller on talos staging. The branch renames are required so that the branch parameter sent via the sendchange lands in the proper scheduler on talos.
Attachment #367096 -
Flags: review?(bhearsum)
Assignee | ||
Updated•16 years ago
|
Attachment #367096 -
Flags: review?(anodelman)
Comment 26•16 years ago
|
||
Comment on attachment 367096 [details] [diff] [review]
sendchange for talos staging
Looks good to me.
Attachment #367096 -
Flags: review?(bhearsum) → review+
Reporter | ||
Comment 27•16 years ago
|
||
Comment on attachment 367096 [details] [diff] [review]
sendchange for talos staging
Belated r+ from me as well.
Attachment #367096 -
Flags: review?(anodelman) → review+
Assignee | ||
Comment 28•16 years ago
|
||
Attachment #367632 -
Flags: review?(bhearsum)
Updated•16 years ago
|
Attachment #367632 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 29•16 years ago
|
||
Comment on attachment 367632 [details] [diff] [review]
Do sendchange to both staging talos masters from production-master
changeset: 1016:7ffc26cf6b6d
Attachment #367632 -
Flags: checked‑in+
Assignee | ||
Comment 30•16 years ago
|
||
One of the major technical aspects of this bug has been fixed - getting sendchange to work to test builds with Talos. This will work fine for RelEng folks who have access to the build network.
There has been some discussion about creating a web interface to allow developers to do testing of arbitrary builds as well.
If we allow this, where should we send the test results for those tests? Should they go to the same graph server, and be reported under the appropriate branch for the build? Or should they get reported to a different branch, to keep the production data clean? Or do the results need to be reported to graph server at all?
Assignee | ||
Comment 31•16 years ago
|
||
This is the final step in getting sendchange working for the automated builds. production-master will do sendchange to qm-rhel02 to notify of new builds that are available.
The ftp poller is disabled for everything except Firefox 3.0 builds.
The scheduler names for mozilla-central, mozilla-1.9.1 and tracemonkey branches are updated to match what will be sent from production-master.
The treeStableTimer for those branches is reduced to 0 since we're guaranteed that the files are in place by the time we get the sendchange notification.
Attachment #368017 -
Flags: review?(bhearsum)
Comment 32•16 years ago
|
||
Comment on attachment 368017 [details] [diff] [review]
Do sendchange from production-master to qm-rhel02
Looks fine to me.
All these sendchange patches remind me that it would be nice to get perfrunner.py stuff into buildbotcustom, though.
Attachment #368017 -
Flags: review?(bhearsum) → review+
Assignee | ||
Updated•16 years ago
|
Attachment #368017 -
Flags: review?(anodelman)
Assignee | ||
Comment 33•16 years ago
|
||
Attachment #368303 -
Flags: review?(bhearsum)
Assignee | ||
Comment 34•16 years ago
|
||
Attachment #368017 -
Attachment is obsolete: true
Attachment #368304 -
Flags: review?(bhearsum)
Attachment #368017 -
Flags: review?(anodelman)
Assignee | ||
Updated•16 years ago
|
Attachment #368304 -
Flags: review?(anodelman)
Updated•16 years ago
|
Attachment #368304 -
Flags: review?(bhearsum) → review+
Comment 35•16 years ago
|
||
Comment on attachment 368303 [details] [diff] [review]
Specify if sendchange failure should generate a build warning or not
Looks fine to me.
Attachment #368303 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 36•16 years ago
|
||
This includes support for having the build lie about its start time.
Attachment #368304 -
Attachment is obsolete: true
Attachment #368326 -
Flags: review?(anodelman)
Attachment #368304 -
Flags: review?(anodelman)
Reporter | ||
Comment 37•16 years ago
|
||
Comment on attachment 368326 [details] [diff] [review]
Do sendchange from production-master to qm-rhel02
Looks good.
Attachment #368326 -
Flags: review?(anodelman) → review+
Assignee | ||
Comment 38•16 years ago
|
||
Comment on attachment 368303 [details] [diff] [review]
Specify if sendchange failure should generate a build warning or not
changeset: 226:8f28fce52b4d
Attachment #368303 -
Flags: checked‑in+
Assignee | ||
Comment 39•16 years ago
|
||
Comment on attachment 368326 [details] [diff] [review]
Do sendchange from production-master to qm-rhel02
changeset: 1030:6dea85195978
Attachment #368326 -
Flags: checked‑in+
Assignee | ||
Comment 40•16 years ago
|
||
changeset: 1032:4f05e8a34fc5
was also checked in to fix some bustage.
Assignee | ||
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•