wikilog: Complete porting to new buildbot codebase and py3

Finish the porting work started by Joshua Lock, accounting for changes
in buildbot APIs/data model and changes from py3, particular around
character encoding.

This also changes the behaviour of the plugin slightly. We now
use the build URL in the header to match builds. With the new codebase
we can walk the parent tree of triggers builds to ensure we always have
the correct parent build url. This means we can drop a lot of the older
more imprecise build matching logic.

Also simplify the format in the wiki log to one output format which lists
all step failures for each build, even in the parent case.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

TODO

@@ -1,6 +1,5 @@
 
  * Add nightly-checkuri
- * finish port of wikilog [Joshua]
  * Add wikilog link on console page
  * implement buildhistory writeback step in -helper (can then drop from builders.py) [Richard]
  * per worker auth (workers.py & config.py)

lib/wiki.py

@@ -73,8 +73,6 @@ class YPWiki(object):
         # whereas in requests 2.1.10 (Fedora 23) Response.content is a str
         # Ensure that bom is the same type as the content, codecs.BOM_UTF8 is
         # a str
-        if type(response.content) == unicode:
-            bom = unicode(codecs.BOM_UTF8, 'utf8')
 
         # If we discover a BOM set the encoding appropriately so that the
         # built in decoding routines in requests work correctly.
@@ -137,9 +135,8 @@ class YPWiki(object):
             return None, None
 
         parsed = self.parse_json(req)
-        pageid = sorted(parsed['query']['pages'].keys())[-1].encode('utf-8')
+        pageid = sorted(parsed['query']['pages'].keys())[-1]
         content = parsed['query']['pages'][pageid]['revisions'][0]['*']
-        content = content.encode('utf-8')
         blurb, entries = content.split('==', 1)
         # ensure we keep only a single newline after the blurb
         blurb = blurb.strip() + "\n"
@@ -166,13 +163,13 @@ class YPWiki(object):
             return False
 
         parsed = self.parse_json(req)
-        pageid = sorted(parsed['query']['pages'].keys())[-1].encode('utf-8')
+        pageid = sorted(parsed['query']['pages'].keys())[-1]
         edit_token = parsed['query']['pages'][pageid]['edittoken']
-        edit_token = edit_token.encode('utf-8')
-
         edit_cookie = cookies.copy()
         edit_cookie.update(req.cookies)
 
+        content = content.encode('utf-8')
+
         content_hash = hashlib.md5(content).hexdigest()
 
         payload = {
@@ -200,7 +197,7 @@ class YPWiki(object):
             return False
         else:
             result = self.parse_json(req)
-            status = result.get('edit', {}).get('result', '').encode('utf-8')
+            status = result.get('edit', {}).get('result', '')
             if status == 'Success':
                 return True
             return False

reporters/wikilog.py

@@ -2,13 +2,19 @@ from buildbot.reporters import utils
 from buildbot.util import service
 from twisted.internet import defer
 from twisted.python import log
+from buildbot.process.results import SUCCESS
 
 from yoctoabb.lib.wiki import YPWiki
 
+import time
+import pprint
+import re
 
 class WikiLog(service.BuildbotService):
     name = "WikiLog"
     wiki = None
+    # wantPreviousBuilds wantLogs
+    neededDetails = dict(wantProperties=True, wantSteps=True)
 
     def checkConfig(self, wiki_uri, wiki_un, wiki_pass, wiki_page,
                     identifier=None, **kwargs):
@@ -19,11 +25,11 @@ class WikiLog(service.BuildbotService):
                         identifier=None, **kwargs):
         yield service.BuildbotService.reconfigService(self)
         self.wiki_page = wiki_page
-        tagfmt = " on {}"
+        self.identifier = None
+        self.idstring = ""
         if identifier:
-            self.tag = tagfmt(identifier)
+            self.identifier = identifier.replace(" ", "-")
-        else:
+            self.idstring = " on " + self.identifier
-            self.tag = ""
         self.wiki = YPWiki(wiki_uri, wiki_un, wiki_pass)
 
     @defer.inlineCallbacks
@@ -39,67 +45,35 @@ class WikiLog(service.BuildbotService):
             self.buildStarted,
             ('builds', None, 'new'))
 
-        # TODO: stepFinished? Or do we no longer need that now that the build
-        # is much simpler?
-
     def stopService(self):
         self._buildCompleteConsumer.stopConsuming()
         self._buildStartedConsumer.stopConsuming()
 
+    @defer.inlineCallbacks
     def buildStarted(self, key, build):
-        yield utils.getDetailsForBuild(self.master, build,
+        yield utils.getDetailsForBuild(self.master, build, **self.neededDetails)
-                                       **self.needed)
+        #log.err("wkl: buildStarted %s %s" % (key, pprint.pformat(build)))
-        builderName = build['builder']['name']
+
-        # TODO: this all seems a bit overly complex?
+        # Only place initial entries in the wiki for builds with no parents
-        if builderName != "nightly" and not self.isTriggered(build)\
+        if not build['buildset']['parent_buildid']:
-           and not self.isNightlyRunning():
-            # If a build has been started which isn't nightly and a nightly
-            # build isn't running, chances are a single builder has been
-            # started in order to test something and it just finished.
             if not self.logBuild(build):
                 log.err("wkl: Failed to log build %s on %s" % (
-                    build.getNumber(), builderName))
+                    build['buildid'], build['builder']['name']))
-        elif builderName == "nightly":
-            if not self.logBuild(build):
-                log.err("wkl: Failed to log build %s on %s" % (
-                    build.getNumber(), builderName))
-
-    def stepFinished(self, key, build):
-        blurb, content = self.wiki.get_content(self.wiki_page)
-        if not blurb:
-            log.err("wkl: Couldn't get wiki content in stepFinished()")
-            return
-        update = self.updateBuildInfo(content, build)
-
-        if not update:
-            # log.msg("wkl: No update, nothing to POST")
-            return
-
-        if content == update:
-            # log.msg("wkl: No update made, no need to make a POST")
-            return
-
-        cookies = self.wiki.login()
-        if not cookies:
-            log.err("wkl: Failed to login to wiki")
-            return
-
-        summary = "Updating branch and commitish for %s" %\
-                  build.getNumber()
-        if not self.wiki.post_entry(self.wiki_page, blurb+update, summary,
-                                    cookies):
-            # log.err("wkl: Failed to update wikilog with summary: '{}'".format
-                # (summary))
-            return
 
+    # Assume we only have a parent, doesn't handle builds nested more than one level.
+    @defer.inlineCallbacks
     def buildFinished(self, key, build):
-        # TODO: we don't have a result var
+        yield utils.getDetailsForBuild(self.master, build, **self.neededDetails)
-        if result == SUCCESS:
+        #log.err("wkl: buildFinished %s %s" % (key, pprint.pformat(build)))
-            return
 
-        if not self.updateBuild(build):
+        parent = None
+        if build['buildset']['parent_buildid']:
+            parent = yield self.master.data.get(("builds", build['buildset']['parent_buildid']))
+            yield utils.getDetailsForBuild(self.master, parent, **self.neededDetails)
+
+        if not self.updateBuild(build, parent):
             log.err("wkl: Failed to update wikilog with build %s failure" %
-                    build.getNumber())
+                    build['buildid'])
 
     def logBuild(self, build):
         """
@@ -108,36 +82,36 @@ class WikiLog(service.BuildbotService):
         @type   build:  buildbot.status.build.BuildStatus
         """
 
-        builder = build.getBuilder().getName()
+        log.err("wkl: logbuild %s" % (build))
-        reason = build.getReason()
+
-        buildid = str(build.getNumber())
+        builder = build['builder']['name']
-        start, _ = build.getTimes()
+        reason = "No reason given"
-        url = self.status.getURLForThing(build)
+        if 'reason' in build['properties'] and build['properties']['reason'][0]:
-        buildbranch = build.getProperty('branch').strip()
+            reason = build['properties']['reason'][0]
-        if not buildbranch or len(buildbranch) < 1:
+        buildid = build['buildid']
-            buildbranch = "YP_BUILDBRANCH"
+        start = build['started_at']
-        chash = build.getProperty('commit_poky').strip()
+        url = build['url']
+        buildbranch = build['properties']['branch_poky'][0]
+
+        chash = build['properties']['commit_poky'][0]
         if not chash or len(chash) < 1 or chash == "HEAD":
             chash = "YP_CHASH"
 
-        reason_list = reason.split(':', 1)
+        forcedby = "Unknown"
-        forcedby = reason_list[0].strip()
+        if 'owner' in build['properties']:
-        description = 'No reason given.'
+            forcedby = build['properties']['owner'][0]
-        if len(reason_list) > 1 and reason_list[1] != ' ':
+        starttime = start.ctime()
-            description = reason_list[1].strip()
-        starttime = time.ctime(start)
 
-        sectionfmt = '==[{} {} {} - {} {}]=='
+        sectionfmt = '==[{} {} {} - {} {}{}]=='
-        section_title = sectionfmt.format(url, builder, buildid, buildbranch,
+        section_title = sectionfmt.format(url, builder, buildid, buildbranch, chash, self.idstring)
-                                          chash)
         summaryfmt = 'Adding new BuildLog entry for build %s (%s)'
         summary = summaryfmt % (buildid, chash)
-        summary = summary + self.tag
+        summary = summary + self.idstring
         content = "* '''Build ID''' - %s" % chash
-        content = content + self.tag + "\n"
+        content = content + self.idstring
-        content = content + '* Started at: %s\n' % starttime
+        content = content + '\n* Started at: %s\n' % starttime
-        content = content + '* ' + forcedby + '\n* ' + description + '\n'
+        content = content + '* ' + forcedby + '\n* ' + reason + '\n'
-        new_entry = '{}\n{}\n'.format(section_title, content).encode('utf-8')
+        new_entry = '{}\n{}\n'.format(section_title, content)
 
         blurb, entries = self.wiki.get_content(self.wiki_page)
         if not blurb:
@@ -159,7 +133,7 @@ class WikiLog(service.BuildbotService):
         log.msg("wkl: Posting wikilog entry for %s" % buildid)
         return True
 
-    def updateEntryBuildInfo(self, entry, build):
+    def updateEntryBuildInfo(self, entry, title, build):
         """
         Extract the branch and commit hash from the properties of the 'build'
         and update the 'entry' string with extracted values
@@ -167,144 +141,71 @@ class WikiLog(service.BuildbotService):
         @type   entry: string
         @type   build:   buildbot.status.build.BuildStatus
         """
-        # We only want to update the commit and branch info for the
+
-        # primary poky build
+        chash = None
-        # FIXME: this is quite poky specific. Can we handle this in
+        if "yp_build_revision" in build['properties']:
-        # a more generic manner?
+            chash = build['properties']['yp_build_revision'][0]
-        repo = build.getProperty("repourl_poky")
-        if not repo:
-            return entry
-        buildbranch = build.getProperty('branch').strip()
-        if not buildbranch or len(buildbranch) < 1:
-            buildbranch = "YP_BUILDBRANCH"
-        chash = build.getProperty('commit_poky').strip()
         if not chash or len(chash) < 1 or chash == "HEAD":
             chash = "YP_CHASH"
 
-        new_entry = entry.replace("YP_BUILDBRANCH", buildbranch, 1)
+        new_entry = entry.replace("YP_CHASH", chash, 2)
-        new_entry = new_entry.replace("YP_CHASH", chash, 2)
+        new_title = title.replace("YP_CHASH", chash, 2)
 
-        return new_entry
+        return new_entry, new_title
 
-    def updateBuildInfo(self, content, build):
+    @defer.inlineCallbacks
-        """
+    def updateBuild(self, build, parent):
-        Extract the branch and commit hash from the properties of the 'build'
-        and update the 'content' string with extracted values
-
-        @type   content: string
-        @type   build:   buildbot.status.build.BuildStatus
-        """
-
-        # Try to find an entry that matches this build, rather than blindly
-        # updating all instances of the template value in the content
-        buildid = build.getProperty('buildnumber', '0')
-        builder = build.getProperty('buildername', 'nobuilder')
-        entry_list = re.split('\=\=\[(.+)\]\=\=', content)
-        title_idx = -1
-        # Start at the beginning of entry list and keep iterating until we find
-        # a title which looks ~right
-        for idx, ent in enumerate(entry_list):
-            # The matched title contents should always start with a http*
-            # schemed URI
-            if ent.startswith('http'):
-                # format of the title is:
-                # ==[url builder buildid - buildbranch commit_hash]==
-                title_components = ent.split(None, 6)
-                if builder == title_components[1] and \
-                   str(buildid) == title_components[2]:
-                    title_idx = idx
-                    break
-
-        if title_idx < 0:
-            errmsg = ("wkl: Failed to update entry for {0} couldn't find a "
-                      "matching title with builder {1}")
-            log.err(errmsg.format(buildid, builder))
-            return content
-
-        entry = entry_list[title_idx + 1]
-        title = entry_list[title_idx]
-
-        combined = "==[{0}]=={1}".format(title, entry)
-        new_entry = self.updateEntryBuildInfo(combined, build)
-        new_entry = new_entry.encode('utf-8')
-
-        it = re.finditer('\=\=\[(.+)\]\=\=', content)
-        entry_title = it.next()
-        while entry_title.group(1) != title:
-            entry_title = it.next()
-        next_title = it.next()
-        head = content[:entry_title.start()]
-        tail = content[next_title.start():]
-        update = head + new_entry + tail
-
-        # log.msg("wkl: Updating commit info YP_BUILDBRANCH=%s YP_CHASH=%s" %
-        #         (buildbranch, chash))
-
-        return update
-
-    def updateBuild(self, build):
         """
         Extract information about 'build' and update an entry in the wiki
 
         @type   build:  buildbot.status.build.BuildStatus
         """
-        builder = build.getBuilder().getName()
+        if not parent:
-        buildid = str(build.getNumber())
+            parent = build
-        reason = build.getReason()
+
+        url = build['url']
+        log_entries = []
+        logfmt = '[%s %s]'
+        for s in build['steps']:
+
+            # Ignore logs for steps which succeeded
+            result = s['results']
+            if result == SUCCESS:
+                continue
+
+            step_name = s['name']
+            step_number = s['number']
+            logs = yield self.master.data.get(("steps", s['stepid'], 'logs'))
+            logs = list(logs)
+            for l in logs:
+                log_url = '%s/steps/%s/logs/%s' % (url, step_number, l['name'])
+                log_entry = logfmt % (log_url, step_name)
+                log_entries.append(log_entry)
+
         blurb, entries = self.wiki.get_content(self.wiki_page)
         if not blurb:
             log.err("wkl: Unexpected content retrieved from wiki!")
             return False
 
-        url = self.status.getURLForThing(build)
-        log_entries = []
-        logfmt = '[%s %s]'
-        for l in build.getLogs():
-            # Ignore logs for steps which succeeded
-            result, _ = l.getStep().getResults()
-            if result == SUCCESS:
-                continue
-
-            step_name = l.getStep().getName()
-            log_url = '%s/steps/%s/logs/%s' % (url,
-                                               step_name,
-                                               l.getName())
-            log_url = log_url.replace(' ', '%20')
-            log_entry = logfmt % (log_url, step_name)
-            log_entries.append(log_entry)
-        buildbranch = build.getProperty('branch').strip()
-        if not buildbranch or len(buildbranch) < 1:
-            buildbranch = "YP_BUILDBRANCH"
-        chash = build.getProperty('commit_poky').strip()
-        if not chash or len(chash) < 1 or chash == "HEAD":
-            chash = "YP_CHASH"
-
         entry_list = re.split('\=\=\[(.+)\]\=\=', entries)
         entry = ''
         title = ''
+        foundmatch = False
         # Start at the beginning of entry list and keep iterating until we find
-        # a title which looks ~right
+        # a title which contains our url/identifier
-        trigger = "Triggerable(trigger_main-build"
         for idx, entry in enumerate(entry_list):
             # The matched title contents should always start with a http*
             # schemed URI
             if entry.startswith('http'):
                 # format of the title is:
-                # ==[url builder buildid - buildbranch commit_hash]==
+                # ==[url builder buildid - buildbranch commit_hash on identifier]==
-                title_components = entry.split(None, 6)
+                title_components = entry.split(None, 8)
 
-                # For the primary, nightly, builder we can match on chash and
+                if title_components[0] == parent['url']:
-                # buildbranch, otherwise we have to hope that the first
+                    if self.identifier and title_components[7] == self.identifier:
-                # triggered build with matching chash and tag
+                        foundmatch = True
-                foundmatch = False
+                    elif not self.identifier:
-                if buildbranch == title_components[4] \
+                        foundmatch = True
-                   and chash == title_components[5] \
-                   and self.tag in entry_list[idx+1]:
-                    foundmatch = True
-                elif trigger in reason \
-                   and chash == title_components[5] \
-                   and self.tag in entry_list[idx+1]:
-                    foundmatch = True
 
                 if foundmatch:
                     entry = entry_list[idx+1]
@@ -312,58 +213,39 @@ class WikiLog(service.BuildbotService):
                     break
 
         if not entry or not title:
-            errmsg = ("wkl: Failed to update entry for {0} couldn't find a "
+            errmsg = ("wkl: Failed to update entry for {0} couldn't find a matching title containing url: {1}")
-                      "matching title for branch: {1} or hash: {2} "
+            log.err(errmsg.format(buildid, parent['url']))
-                      "(reason was '{3}')")
-            log.err(errmsg.format(buildid, buildbranch, chash, reason))
             return False
 
-        log_fmt = ''
         logs = ''
         new_entry = ''
-        if builder == 'nightly':
+        log_fmt = '\n** '
-            # for failures in nightly we just append extra entries to the
+        buildid = build['buildid']
-            # bullet list pointing to the failure logs
+        builder = build['builder']['name']
-            if len(log_entries) > 0:
+        builderfmt = '\n* [%s %s] failed' % (url, builder)
-                logs = '\n* '.join(log_entries) + '\n'
-                new_entry = '\n' + entry.strip() + '\n* ' + logs
-            else:
-                # We only update the buildlog for a nightly build if there
-                # are additional items to append to the log list.
-                return True
-        else:
-            # for non-nightly failures we create an entry in the list linking
-            # to the failed builder and indent the logs as a child bullet list
-            log_fmt = '\n* '
-            builderfmt = log_fmt
-            if self.isTriggered(build) or self.isNightlyRunning():
-                log_fmt = '\n** '
-                builderfmt = '\n* [%s %s] failed' % (url, builder)
 
-            if len(log_entries) > 0:
+        if len(log_entries) > 0:
-                if self.isTriggered(build) or self.isNightlyRunning():
+            builderfmt = builderfmt + ': ' + log_fmt
-                    builderfmt = builderfmt + ': ' + log_fmt
+            logs = log_fmt.join(log_entries)
-                logs = log_fmt.join(log_entries)
+        logs = logs + '\n'
-            logs = logs + '\n'
+        new_entry = '\n' + entry.strip() + builderfmt + logs
-            new_entry = '\n' + entry.strip() + builderfmt + logs
 
         summary = 'Updating entry with failures in %s' % builder
-        summary = summary + self.tag
+        summary = summary + self.idstring
 
-        new_entry = self.updateEntryBuildInfo(new_entry, build)
+        new_entry, new_title = self.updateEntryBuildInfo(new_entry, title, parent)
-        new_entry = new_entry.encode('utf-8')
 
         # Find the point where the first entry's title starts and the second
         # entry's title begins, then replace the text between those points
         # with the newly generated entry.
         it = re.finditer('\=\=\[(.+)\]\=\=', entries)
-        entry_title = it.next()
+        entry_title = next(it)
         while entry_title.group(1) != title:
-            entry_title = it.next()
+            entry_title = next(it)
-        next_title = it.next()
+        next_title = next(it)
-        head = entries[:entry_title.end()]
+        head = entries[:entry_title.start()]
         tail = entries[next_title.start():]
-        update = head + new_entry + tail
+        update = head + "==[" + new_title + "]==\n" + new_entry + tail
 
         cookies = self.wiki.login()
         if not cookies:
@@ -378,30 +260,3 @@ class WikiLog(service.BuildbotService):
         log.msg("wkl: Updating wikilog entry for %s" % buildid)
         return True
 
-    def isNightlyRunning(self):
-        """
-        Determine whether there's a nightly build in progress
-        """
-        nightly = self.master.getBuilder("nightly")
-        if not nightly:
-            return False
-        build = nightly.getBuild(-1)  # the most recent build
-        if not build:
-            return False
-        running = not build.isFinished()
-        return running
-
-    def isTriggered(self, build):
-        """
-        build.isFinished() can return True when buildsteps triggered by
-        nightly are still running, therefore we provide a method to check
-        whether the 'build' was triggered by a nightly build.
-
-        @type   build:  buildbot.status.build.BuildStatus
-        """
-        reason = build.getReason()
-        reason_list = reason.split(':', 1)
-        forcedby = reason_list[0].strip()
-        if forcedby.startswith("Triggerable"):
-            return True
-        return False

services.py

@@ -28,6 +28,6 @@ services = []
 # from yoctoabb.reporters import wikilog
 # services.append(
 #     wikilog.WikiLog("https://wiki.yoctoproject.org/wiki/api.php",
-#                     "User", "password", "LogPage"
+#                     "User", "password", "LogPage",
-#                     "production cluster")
+#                     "Production Cluster")
 # )