Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-2485

BuildWrapper teardown cannot get result of build

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: plugin-proposals
    • Labels:
      None
    • Environment:
      Platform: All, OS: All
    • Similar Issues:

      Description

      I created a BuildWrapper and implemented a tearDown method.
      When I try to get the build.getResult() from within this tearDown method the
      result is always null.

      Reason see code in Build class

      protected class RunnerImpl extends AbstractRunner {
      protected Result doRun(BuildListener listener) throws Exception {
      if(!preBuild(listener,project.getBuilders()))
      return Result.FAILURE;
      if(!preBuild(listener,project.getPublishers()))
      return Result.FAILURE;

      buildEnvironments = new ArrayList<Environment>();
      try {
      List<BuildWrapper> wrappers = new ArrayList<BuildWrapper>
      (project.getBuildWrappers().values());

      ParametersAction parameters = getAction(ParametersAction.class);
      if (parameters != null)
      parameters.createBuildWrappers(Build.this,wrappers);

      for( BuildWrapper w : wrappers )

      { Environment e = w.setUp((AbstractBuild)Build.this, launcher, listener); if(e==null) return Result.FAILURE; buildEnvironments.add(e); }

      if(!build(listener,project.getBuilders()))
      return Result.FAILURE;
      } finally

      { // tear down in reverse order for( int i=buildEnvironments.size()-1; i>=0; i-- ) buildEnvironments.get(i).tearDown((AbstractBuild) Build.this,listener); buildEnvironments = null; }

      return null;
      }

      The BuildWrapper.tearDown method is called from within the finally block and
      the result is not set yet.

        Attachments

          Activity

          Hide
          robertdw robertdw added a comment -

          A similar bug impacts the Maven2 Builder - See JENKINS-6033

          Show
          robertdw robertdw added a comment - A similar bug impacts the Maven2 Builder - See JENKINS-6033
          Hide
          mindless Alan Harder added a comment -

          I updated doRun to set the result in the build if there is a failure.. so now your tearDown methods can check for failure result, and interpret null as "SUCCESS-so-far" (post-build actions may still affect final result).

          Show
          mindless Alan Harder added a comment - I updated doRun to set the result in the build if there is a failure.. so now your tearDown methods can check for failure result, and interpret null as "SUCCESS-so-far" (post-build actions may still affect final result).
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in hudson
          User: : mindless
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/Build.java
          trunk/hudson/main/core/src/main/java/hudson/tasks/BuildWrapper.java
          http://fisheye4.cenqua.com/changelog/hudson/?cs=24763
          Log:
          [FIXED JENKINS-2485] Set failure status in build before calling tearDown for build wrappers.
          Wrappers can now interpret null from getStatus() as success-so-far.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in hudson User: : mindless Path: trunk/hudson/main/core/src/main/java/hudson/model/Build.java trunk/hudson/main/core/src/main/java/hudson/tasks/BuildWrapper.java http://fisheye4.cenqua.com/changelog/hudson/?cs=24763 Log: [FIXED JENKINS-2485] Set failure status in build before calling tearDown for build wrappers. Wrappers can now interpret null from getStatus() as success-so-far.
          Hide
          ncsucodemonkey ncsucodemonkey added a comment -

          I'm working on a build wrapper that uses the VirtualBox webservice to provide a
          virtual machine for the duration of the build. If I knew the build result during
          tearDown, I could roll the virtual machine back to the previous snapshot on a
          build failure.

          Show
          ncsucodemonkey ncsucodemonkey added a comment - I'm working on a build wrapper that uses the VirtualBox webservice to provide a virtual machine for the duration of the build. If I knew the build result during tearDown, I could roll the virtual machine back to the previous snapshot on a build failure.
          Hide
          mdillon mdillon added a comment -

          We (Yahoo!) just came across this same issue with one of our internal build
          wrappers too.

          My preference is to add a new tearDown(AbstractBuild,BuildListener,Result)
          method to BuildWrapper that defaults to calling the current
          tearDown(AbstractBuild,BuildListener) for backward compatibility purposes. The
          code cited in Build.java would call the new method instead of the old one. I
          would change the code to have a Result variable to track the result, setting it
          to Result.FAILURE before all the current return statements. It would be
          initialized to null.

          The code has also changed since it was cited in this ticket. It now returns
          Result.FAILURE unconditionally in the finally block if any wrapper fails.
          Keeping track of the original Result would allow it to be logged in this case as
          well as allowing it to be passed to tearDown. I would make it so that it still
          had the current boolean "failed" flag and that tearDown always saw the "result"
          object from the try block, regardless of whether other wrappers had failed.

          Show
          mdillon mdillon added a comment - We (Yahoo!) just came across this same issue with one of our internal build wrappers too. My preference is to add a new tearDown(AbstractBuild,BuildListener,Result) method to BuildWrapper that defaults to calling the current tearDown(AbstractBuild,BuildListener) for backward compatibility purposes. The code cited in Build.java would call the new method instead of the old one. I would change the code to have a Result variable to track the result, setting it to Result.FAILURE before all the current return statements. It would be initialized to null. The code has also changed since it was cited in this ticket. It now returns Result.FAILURE unconditionally in the finally block if any wrapper fails. Keeping track of the original Result would allow it to be logged in this case as well as allowing it to be passed to tearDown. I would make it so that it still had the current boolean "failed" flag and that tearDown always saw the "result" object from the try block, regardless of whether other wrappers had failed.
          Hide
          nkeymeulen nkeymeulen added a comment -

          enhancement

          Show
          nkeymeulen nkeymeulen added a comment - enhancement

            People

            Assignee:
            mindless Alan Harder
            Reporter:
            nkeymeulen nkeymeulen
            Votes:
            2 Vote for this issue
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: