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

PeepholePermalink RunListenerImpl oncompleted should be triggered before downstream builds are triggered

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None

      Scenario
      ProjectA triggers ProjectB
      ProjectB has a RunParameter for ProjectA with a filter set for Successful Builds Only. The Run Parameter uses the PeepholePermalink to determine the LastSuccessful build.

      When ProjectA completes successfully, it will trigger a build of ProjectB before the PeepholePermalink cache is updated. This causes the RunParameter to default to the previous successful build of ProjectA, not the one which has just completed.

          [JENKINS-20989] PeepholePermalink RunListenerImpl oncompleted should be triggered before downstream builds are triggered

          ikedam added a comment -

          It seems that you need an access not to the latest build, but to the build that triggered the downstream build.
          If so, I know it can be resolved by adding RunParameter support to parameterized build plugin, but you may also consider using UpstreamCause to access to the upstream build.
          For example, Copy Artifact plugin provides an option to retrieve artifacts from the upstream build that triggered that downstream build.

          ikedam added a comment - It seems that you need an access not to the latest build, but to the build that triggered the downstream build. If so, I know it can be resolved by adding RunParameter support to parameterized build plugin, but you may also consider using UpstreamCause to access to the upstream build. For example, Copy Artifact plugin provides an option to retrieve artifacts from the upstream build that triggered that downstream build.

          Some of the downstream builds might be configured for All, lastCompleted, lastSuccessful and others for lastStable. Multiple downstream builds might have different requirements. It would be better to have this config in one place, the downstream jobs, and not have to work around this change and require additional logic in the upstream build to determine what RunParameter to pass to the downstream job.

          It worked as expected when the Run#getLastStableBuild used to be like:

              public RunT getLastStableBuild() {
                  RunT r = getLastBuild();
                  while (r != null
                          && (r.isBuilding() || r.getResult().isWorseThan(Result.SUCCESS)))
                      r = r.getPreviousBuild();
                  return r;
              }
          

          It no longer works after it was changed to use the Permalink

              public RunT getLastStableBuild() {
                  return (RunT)Permalink.LAST_STABLE_BUILD.resolve(this);
              }
          

          The State.POST_PRODUCTION seems to be for this very scenario, where the build result is now final and it is now ok to trigger other builds as described in JENKINS-980

          private static enum State {
              /**
               * Build is completed now, and the status is determined,
               * but log files are still being updated.
               *
               * The significance of this state is that Jenkins
               * will now see this build as completed. Things like
               * "triggering other builds" requires this as pre-condition.
               * See JENKINS-980.
               */
              POST_PRODUCTION,
          

          In the Run#execute method, I think we should update the PeepholePermalinks as soon as we change state and before we trigger fireCompleted on the listeners????
          Currently the Permalinks are updated in a fireCompleted event on a listener, the same as triggering downstream jobs. But they seem to be updated after the other builds are triggered.

              // advance the state.
              // the significance of doing this is that Jenkins
              // will now see this build as completed.
              // things like triggering other builds requires this as pre-condition.
              // see issue #980.
              //
              state = State.POST_PRODUCTION;
          
              if (listener != null) {
                  try {
                      job.cleanUp(listener);
                  } catch (Exception e) {
                      handleFatalBuildProblem(listener,e);
                      // too late to update the result now
                  }
                  RunListener.fireCompleted(this,listener);
                  listener.finished(result);
                  listener.closeQuietly();
              }
          

          Geoff Cummings added a comment - Some of the downstream builds might be configured for All, lastCompleted, lastSuccessful and others for lastStable. Multiple downstream builds might have different requirements. It would be better to have this config in one place, the downstream jobs, and not have to work around this change and require additional logic in the upstream build to determine what RunParameter to pass to the downstream job. It worked as expected when the Run#getLastStableBuild used to be like: public RunT getLastStableBuild() { RunT r = getLastBuild(); while (r != null && (r.isBuilding() || r.getResult().isWorseThan(Result.SUCCESS))) r = r.getPreviousBuild(); return r; } It no longer works after it was changed to use the Permalink public RunT getLastStableBuild() { return (RunT)Permalink.LAST_STABLE_BUILD.resolve( this ); } The State.POST_PRODUCTION seems to be for this very scenario, where the build result is now final and it is now ok to trigger other builds as described in JENKINS-980 private static enum State { /** * Build is completed now, and the status is determined, * but log files are still being updated. * * The significance of this state is that Jenkins * will now see this build as completed. Things like * "triggering other builds" requires this as pre-condition. * See JENKINS-980. */ POST_PRODUCTION, In the Run#execute method, I think we should update the PeepholePermalinks as soon as we change state and before we trigger fireCompleted on the listeners???? Currently the Permalinks are updated in a fireCompleted event on a listener, the same as triggering downstream jobs. But they seem to be updated after the other builds are triggered. // advance the state. // the significance of doing this is that Jenkins // will now see this build as completed. // things like triggering other builds requires this as pre-condition. // see issue #980. // state = State.POST_PRODUCTION; if (listener != null ) { try { job.cleanUp(listener); } catch (Exception e) { handleFatalBuildProblem(listener,e); // too late to update the result now } RunListener.fireCompleted( this ,listener); listener.finished(result); listener.closeQuietly(); }

          Jesse Glick added a comment -

          Makes sense, file a pull request with a test.

          Jesse Glick added a comment - Makes sense, file a pull request with a test.

          I am also interested in a fix for this. I've just forked the jenkins repo and I'm still trying to get my head around the project setup.

          Horia Constantin added a comment - I am also interested in a fix for this. I've just forked the jenkins repo and I'm still trying to get my head around the project setup.

          Oliver Gondža added a comment - Giving this a try in https://github.com/jenkinsci/jenkins/pull/1628

          Code changed in jenkins
          User: Oliver Gondža
          Path:
          test/src/test/java/hudson/tasks/BuildTriggerTest.java
          http://jenkins-ci.org/commit/jenkins/021c1b381deec7cab185db4ccaa59ad2b68c4b7a
          Log:
          JENKINS-20989 Reproduce

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oliver Gondža Path: test/src/test/java/hudson/tasks/BuildTriggerTest.java http://jenkins-ci.org/commit/jenkins/021c1b381deec7cab185db4ccaa59ad2b68c4b7a Log: JENKINS-20989 Reproduce

          Code changed in jenkins
          User: Oliver Gondža
          Path:
          core/src/main/java/hudson/model/Run.java
          http://jenkins-ci.org/commit/jenkins/ca5b9b667231fb4805e6001910dee1a26c5d4c68
          Log:
          [FIXED JENKINS-20989] PeepholePermalink RunListenerImpl oncompleted should be triggered before downstream builds are triggered

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oliver Gondža Path: core/src/main/java/hudson/model/Run.java http://jenkins-ci.org/commit/jenkins/ca5b9b667231fb4805e6001910dee1a26c5d4c68 Log: [FIXED JENKINS-20989] PeepholePermalink RunListenerImpl oncompleted should be triggered before downstream builds are triggered

          Code changed in jenkins
          User: Oliver Gondža
          Path:
          core/src/main/java/hudson/model/Run.java
          test/src/test/java/hudson/tasks/BuildTriggerTest.java
          http://jenkins-ci.org/commit/jenkins/ad56599e5c54d9a5f850f365ca0ff969d7c7f87d
          Log:
          Merge pull request #1628 from olivergondza/JENKINS-27635

          JENKINS-20989 PeepholePermalink RunListenerImpl oncompleted should be triggered before downstream builds are triggered

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oliver Gondža Path: core/src/main/java/hudson/model/Run.java test/src/test/java/hudson/tasks/BuildTriggerTest.java http://jenkins-ci.org/commit/jenkins/ad56599e5c54d9a5f850f365ca0ff969d7c7f87d Log: Merge pull request #1628 from olivergondza/ JENKINS-27635 JENKINS-20989 PeepholePermalink RunListenerImpl oncompleted should be triggered before downstream builds are triggered

          dogfood added a comment -

          Integrated in jenkins_main_trunk #4065

          Result = SUCCESS

          dogfood added a comment - Integrated in jenkins_main_trunk #4065 Result = SUCCESS

            olivergondza Oliver Gondža
            gcummings Geoff Cummings
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: