[CI] In-progress PR comments (#72211) (#72743)

vars/githubPr.groovy

@@ -15,25 +15,43 @@
 */
 def withDefaultPrComments(closure) {
   catchErrors {
+    // sendCommentOnError() needs to know if comments are enabled, so lets track it with a global
+    // isPr() just ensures this functionality is skipped for non-PR builds
+    buildState.set('PR_COMMENTS_ENABLED', isPr())
     catchErrors {
       closure()
     }
+    sendComment(true)
+  }
+}
 
-    if (!params.ENABLE_GITHUB_PR_COMMENTS || !isPr()) {
-      return
-    }
+def sendComment(isFinal = false) {
+  if (!buildState.get('PR_COMMENTS_ENABLED')) {
+    return
+  }
 
-    def status = buildUtils.getBuildStatus()
-    if (status == "ABORTED") {
-      return;
-    }
+  def status = buildUtils.getBuildStatus()
+  if (status == "ABORTED") {
+    return
+  }
 
-    def lastComment = getLatestBuildComment()
-    def info = getLatestBuildInfo(lastComment) ?: [:]
-    info.builds = (info.builds ?: []).takeRight(5) // Rotate out old builds
+  def lastComment = getLatestBuildComment()
+  def info = getLatestBuildInfo(lastComment) ?: [:]
+  info.builds = (info.builds ?: []).takeRight(5) // Rotate out old builds
 
-    def message = getNextCommentMessage(info)
-    postComment(message)
+  // If two builds are running at the same time, the first one should not post a comment after the second one
+  if (info.number && info.number.toInteger() > env.BUILD_NUMBER.toInteger()) {
+    return
+  }
+
+  def shouldUpdateComment = !!info.builds.find { it.number == env.BUILD_NUMBER }
+
+  def message = getNextCommentMessage(info, isFinal)
+
+  if (shouldUpdateComment) {
+    updateComment(lastComment.id, message)
+  } else {
+    createComment(message)
 
     if (lastComment && lastComment.user.login == 'kibanamachine') {
       deleteComment(lastComment.id)
@@ -41,6 +59,19 @@ def withDefaultPrComments(closure) {
   }
 }
 
+def sendCommentOnError(Closure closure) {
+  try {
+    closure()
+  } catch (ex) {
+    // If this is the first failed step, it's likely that the error hasn't propagated up far enough to mark the build as a failure
+    currentBuild.result = 'FAILURE'
+    catchErrors {
+      sendComment(false)
+    }
+    throw ex
+  }
+}
+
 // Checks whether or not this currently executing build was triggered via a PR in the elastic/kibana repo
 def isPr() {
   return !!(env.ghprbPullId && env.ghprbPullLink && env.ghprbPullLink =~ /\/elastic\/kibana\//)
@@ -66,7 +97,7 @@ def getLatestBuildInfo() {
 }
 
 def getLatestBuildInfo(comment) {
-  return comment ? getBuildInfoFromComment(comment) : null
+  return comment ? getBuildInfoFromComment(comment.body) : null
 }
 
 def createBuildInfo() {
@@ -137,14 +168,25 @@ def getTestFailuresMessage() {
   return messages.join("\n")
 }
 
-def getNextCommentMessage(previousCommentInfo = [:]) {
+def getNextCommentMessage(previousCommentInfo = [:], isFinal = false) {
   def info = previousCommentInfo ?: [:]
   info.builds = previousCommentInfo.builds ?: []
 
+  // When we update an in-progress comment, we need to remove the old version from the history
+  info.builds = info.builds.findAll { it.number != env.BUILD_NUMBER }
+
   def messages = []
   def status = buildUtils.getBuildStatus()
 
-  if (status == 'SUCCESS') {
+  if (!isFinal) {
+    def failuresPart = status != 'SUCCESS' ? ', with failures' : ''
+    messages << """
+      ## :hourglass_flowing_sand: Build in-progress${failuresPart}
+      * [continuous-integration/kibana-ci/pull-request](${env.BUILD_URL})
+      * Commit: ${getCommitHash()}
+      * This comment will update when the build is complete
+    """
+  } else if (status == 'SUCCESS') {
     messages << """
       ## :green_heart: Build Succeeded
       * [continuous-integration/kibana-ci/pull-request](${env.BUILD_URL})
@@ -172,7 +214,9 @@ def getNextCommentMessage(previousCommentInfo = [:]) {
       * [Pipeline Steps](${env.BUILD_URL}flowGraphTable) (look for red circles / failed steps)
       * [Interpreting CI Failures](https://www.elastic.co/guide/en/kibana/current/interpreting-ci-failures.html)
     """
+  }
 
+  if (status != 'SUCCESS' && status != 'UNSTABLE') {
     try {
       def steps = getFailedSteps()
       if (steps?.size() > 0) {
@@ -186,7 +230,10 @@ def getNextCommentMessage(previousCommentInfo = [:]) {
   }
 
   messages << getTestFailuresMessage()
-  messages << ciStats.getMetricsReport()
+
+  if (isFinal) {
+    messages << ciStats.getMetricsReport()
+  }
 
   if (info.builds && info.builds.size() > 0) {
     messages << getHistoryText(info.builds)
@@ -208,7 +255,7 @@ def getNextCommentMessage(previousCommentInfo = [:]) {
     .join("\n\n")
 }
 
-def postComment(message) {
+def createComment(message) {
   if (!isPr()) {
     error "Trying to post a GitHub PR comment on a non-PR or non-elastic PR build"
   }
@@ -224,6 +271,20 @@ def getComments() {
   }
 }
 
+def updateComment(commentId, message) {
+  if (!isPr()) {
+    error "Trying to post a GitHub PR comment on a non-PR or non-elastic PR build"
+  }
+
+  withGithubCredentials {
+    def path = "repos/elastic/kibana/issues/comments/${commentId}"
+    def json = toJSON([ body: message ]).toString()
+
+    def resp = githubApi([ path: path ], [ method: "POST", data: json, headers: [ "X-HTTP-Method-Override": "PATCH" ] ])
+    return toJSON(resp)
+  }
+}
+
 def deleteComment(commentId) {
   withGithubCredentials {
     def path = "repos/elastic/kibana/issues/comments/${commentId}"

vars/jenkinsApi.groovy

@@ -10,7 +10,7 @@ def getSteps() {
 
 def getFailedSteps() {
   def steps = getSteps()
-  def failedSteps = steps?.findAll { it.iconColor == "red" && it._class == "org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode" }
+  def failedSteps = steps?.findAll { (it.iconColor == "red" || it.iconColor == "red_anime") && it._class == "org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode" }
   failedSteps.each { step ->
     step.logs = "${env.BUILD_URL}execution/node/${step.id}/log".toString()
   }

vars/kibanaPipeline.groovy

@@ -35,7 +35,9 @@ def functionalTestProcess(String name, Closure closure) {
       "JOB=${name}",
       "KBN_NP_PLUGINS_BUILT=true",
     ]) {
-      closure()
+      githubPr.sendCommentOnError {
+        closure()
+      }
     }
   }
 }
@@ -163,26 +165,32 @@ def bash(script, label) {
 }
 
 def doSetup() {
-  retryWithDelay(2, 15) {
-    try {
-      runbld("./test/scripts/jenkins_setup.sh", "Setup Build Environment and Dependencies")
-    } catch (ex) {
+  githubPr.sendCommentOnError {
+    retryWithDelay(2, 15) {
       try {
-        // Setup expects this directory to be missing, so we need to remove it before we do a retry
-        bash("rm -rf ../elasticsearch", "Remove elasticsearch sibling directory, if it exists")
-      } finally {
-        throw ex
+        runbld("./test/scripts/jenkins_setup.sh", "Setup Build Environment and Dependencies")
+      } catch (ex) {
+        try {
+          // Setup expects this directory to be missing, so we need to remove it before we do a retry
+          bash("rm -rf ../elasticsearch", "Remove elasticsearch sibling directory, if it exists")
+        } finally {
+          throw ex
+        }
       }
     }
   }
 }
 
 def buildOss() {
-  runbld("./test/scripts/jenkins_build_kibana.sh", "Build OSS/Default Kibana")
+  githubPr.sendCommentOnError {
+    runbld("./test/scripts/jenkins_build_kibana.sh", "Build OSS/Default Kibana")
+  }
 }
 
 def buildXpack() {
-  runbld("./test/scripts/jenkins_xpack_build_kibana.sh", "Build X-Pack Kibana")
+  githubPr.sendCommentOnError {
+    runbld("./test/scripts/jenkins_xpack_build_kibana.sh", "Build X-Pack Kibana")
+  }
 }
 
 def runErrorReporter() {

vars/workers.groovy

@@ -126,7 +126,9 @@ def intake(jobName, String script) {
   return {
     ci(name: jobName, size: 's-highmem', ramDisk: true) {
       withEnv(["JOB=${jobName}"]) {
-        runbld(script, "Execute ${jobName}")
+        githubPr.sendCommentOnError {
+          runbld(script, "Execute ${jobName}")
+        }
       }
     }
   }