473415 - Mercurial release automation needs to monitor for the signing log, and continue on when it appears

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[[e-mail] (:bhearsum)]

In 1.8/1.9 automation we have a BuildFactory which sits and waits for the win32 signing log to appear. When it does appear, the build will finish and l10n verify and update generation builders will start. We need an equivalent for Mercurial automation.

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Could we use the same code that we used in 1.8/1.9? After all, its just a buildbot step calling a script which waits for a specific file to be created in a specific location.

Triaging to Future while we sort out what to do, and who's working on it.

Comment 2

[[e-mail] (:bhearsum)]

Maybe.

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: ftpPoller which waits for file to trigger a build step

Tested locally, was able to:

* trigger a build when the file being searched for arrives in the ftp directory.  
* not trigger a build on a restart if the file is already there

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 359760 [details]
ftpPoller which waits for file to trigger a build step

>import time
>import re
>
>from urllib2 import urlopen, unquote
>from twisted.python import log, failure
>from twisted.internet import defer, reactor
>from twisted.internet.task import LoopingCall
>
>from buildbot.changes import base, changes
>
>class InvalidResultError(Exception):
>    def __init__(self, value="InvalidResultError"):
>        self.value = value
>    def __str__(self):
>        return repr(self.value)
>
>class EmptyResult(Exception):
>    pass
>
>class FtpPoller(base.ChangeSource):
>    """This source will poll an ftp directory for changes and submit
>    them to the change master."""
>    
>    compare_attrs = ["ftpURLs", "pollInterval", "tree", "branch"]
>    
>    parent = None # filled in when we're added
>    loop = None
>    volatile = ['loop']
>    working = 0
>    
>    def __init__(self, branch="", tree="Firefox", pollInterval=30, ftpURLs=[], searchString="", gotFile=1):

gotFile shouldn't be overrideable, and thereofrce shouldn't be in the constructor. Just set to self.gotFile = 1 to initialize it.

What's tree used for?

>        """
>        @type   ftpURLs:            list of strings
>        @param  ftpURLs:            The ftp directories to monitor
>
>        @type   tree:               string
>        @param  tree:               The tree to look for changes in. 
>                                    For example, Firefox trunk is 'Firefox'
>        @type   branch:             string
>        @param  branch:             The branch to look for changes in. This must
>                                    match the 'branch' option for the Scheduler.
>        @type   pollInterval:       int
>        @param  pollInterval:       The time (in seconds) between queries for 
>                                    changes
>        @type   searchString:       string
>        @param  searchString:       file type of the build we are looking for
>        """
>        
>        self.ftpURLs = ftpURLs
>        self.tree = tree
>        self.branch = branch
>        self.pollInterval = pollInterval
>        self.lastChanges = {}
>        for url in self.ftpURLs:
>          self.lastChanges[url] = time.time()

Is lastChanges used at all? If not, remove it.


>    def describe(self):
>        str = ""
>        str += "Getting changes from directory %s " \
>                % str(self.ftpURLs)
>        str += "<br>Using tree: %s, branch %s" % (self.tree, self.branch)

The second line of this description is totally bogus, just remove it.

>    def _process_changes(self, query, forceDate):
>    
>        try:
>          counter = 0
>          url = query.geturl()
>          pageContents = query.read()
>          query.close()
>          lines = pageContents.split('\n')
>          """ Check through lines to see if file exists """
>          # scenario 1:
>          # buildbot restarts or file already exists, so we don't want to trigger anything
>          if self.gotFile == 1:
>            for line in lines:
>              if self.searchString in line:
>                log.msg("Found a match when gotFile is already 1, not triggering.")

Let's not log this stuff out now that you're done most of you're testing - it'll clutter the logs up.

>          # gotFile is 0, we are waiting for a match to trigger build
>          if self.gotFile == 0:
>            for line in lines:
>              # if we have a new file, trigger a build
>              if self.searchString in line:
>                self.gotFile = 1
>                log.msg("Triggering a build...")

Keep this msg, it's useful. Can you include the filename in it, though?

>                c = changes.Change(who = url,
>                               comments = "success",
>                               files = [],
>                               branch = self.branch)
>                self.parent.addChange(c)
>                log.msg("Found a build to test.")

Toss this one, it's just clutter IMHO.


Great work overall, just fix the comments and it's ready to go.

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Cleaned up ftpPoller.py

Followed comments, and also made some small changes to comments in the script regarding the function of this poller.

Comment 6

[[e-mail] (:bhearsum)]

Comment on attachment 360152 [details]
Cleaned up ftpPoller.py

Looks good.

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add post signing schedulers to the release process

If this works, then what's left is to work on getting ftpPoller the right URL & search string with less hard-coding.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add post signing schedulers to the release process

Sorry, this one has the hard-coding in the diff.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Add post signing schedulers to the release process

This one takes the hard-coding out of release_config.py and uses what already exists to build the ftp polling URL.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 360773 [details] [diff] [review]
Add post signing schedulers to the release process

> ##### Change sources and Schedulers
> change_source.append(PBChangeSource())
>+change_source.append(FtpPoller(
>+	branch="post_signing",
>+	ftpURLs='http://' + stagingServer + '/pub/mozilla.org/' + productName + '/nightly/' + appVersion + '-candidates/build' + buildNumber + '/',

Please use the 'http://%s...' style here, for consistency.

I still don't like hardcoding this here but I don't know where we could put it that would allow us to share it with the factories.

>+	pollInterval=5,

Bump this waaaaaaaaaaaaaaaay up. At least 10 minutes (60*10). It would be nice to only have 5 second lag after pushing the signing log, but after a release is done we don't want to be hitting stage every 5 seconds.

>-    schedulers.append(repack_scheduler)
>+    schedulers.append(repack_scheduler) 

You added a trailing space here, please remove it.
  
>+l10n_verify_scheduler = Scheduler(
>+    name='l10n_verification',
>+    branch='post_signing',
>+    builderNames=['l10n_verification']
>+)

You need to pass treeStableTimer here.

>+schedulers.append(l10n_verify_scheduler)
>+updates_scheduler = Scheduler(
>+    name='updates',
>+    branch='post_signing',
>+    builderNames=['updates']
>+)

And here.


Please run this through checkconfig before you attach the next version, it will catch things like missing parameters.


This is almost there, other than the minor things mentioned above.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Staging patch for adding ftppoller post_signing steps

Tested on staging and working.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: Production ftppoller post_signing steps

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

just a note that both release_master.py patches import FtpPoller from buildbotcustom/changes/ftppoller.py

Comment 14

[Aki Sasaki (not active)]

Comment on attachment 360152 [details]
Cleaned up ftpPoller.py

I successfully switched over to this ftppoller.py from the talos ftppoller.py... I just had to remove the 'tree' option from my master.cfg.

However, since self.lastChanges[url] is gone, it looks like my file is changing every single time I poll for it... in my case, every single minute the changes column picks up the same file, even if it hasn't been changed recently.

Comment 15

[[e-mail] (:bhearsum)]

(In reply to comment #14)
> (From update of attachment 360152 [details])
> I successfully switched over to this ftppoller.py from the talos
> ftppoller.py... I just had to remove the 'tree' option from my master.cfg.
> 
> However, since self.lastChanges[url] is gone, it looks like my file is changing
> every single time I poll for it... in my case, every single minute the changes
> column picks up the same file, even if it hasn't been changed recently.

This works for the purposes of release automation. Do you want to file a follow-up and add that part back in once this lands?

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 362068 [details] [diff] [review]
Staging patch for adding ftppoller post_signing steps

>--- a/mozilla2-staging/release_master.py	Wed Feb 11 16:18:35 2009 -0500
>+++ b/mozilla2-staging/release_master.py	Thu Feb 12 12:01:30 2009 -0800

> ##### Change sources and Schedulers
> change_source.append(PBChangeSource())
>+change_source.append(FtpPoller(
>+	branch="post_signing",
>+	ftpURLs=["http://%s/pub/mozilla.org/%s/nightly/%s-candidates/build%s/" % (stagingServer, productName, appVersion, buildNumber)],

style nit: please limit line length to 80 characters.

r=bhearsum with that change.

Comment 17

[[e-mail] (:bhearsum)]

Comment on attachment 362071 [details] [diff] [review]
Production ftppoller post_signing steps

>diff -r 469349ff4a86 mozilla2/release_master.py
>--- a/mozilla2/release_master.py	Wed Feb 11 16:18:35 2009 -0500
>+++ b/mozilla2/release_master.py	Thu Feb 12 12:12:17 2009 -0800

>+change_source.append(FtpPoller(
>+	branch="post_signing",
>+	ftpURLs=["http://%s/pub/mozilla.org/%s/nightly/%s-candidates/build%s/" % (stagingServer, productName, appVersion, buildNumber)],

same thing here.

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: All in one staging/prod signing log monitoring patch

With nits addressed and pollInterval upped to 60*10.

Comment 19

[[e-mail] (:bhearsum)]

Comment on attachment 363308 [details] [diff] [review]
All in one staging/prod signing log monitoring patch

changeset:   953:65ac70628893

Comment 20

[[e-mail] (:bhearsum)]

Comment on attachment 360152 [details]
Cleaned up ftpPoller.py

changeset:   198:05a13941dfd7

Comment 21

[[e-mail] (:bhearsum)]

Master has been reconfig'ed for this.