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

Multibranch pipeline with Helix Swarm source doesn't sync to latest

    Details

    • Type: Bug
    • Status: Open (View Workflow)
    • Priority: Minor
    • Resolution: Unresolved
    • Component/s: p4-plugin
    • Labels:
      None
    • Environment:
      Jenkins Core 2.176.2
      P4 plugin 1.10.5
    • Similar Issues:

      Description

      I have a multibranch pipeline configured with only a Helix Swarm branch source and using all default options.  When a review is created the pipeline finds it and syncs to the CL of the review.  The issue here is that typically we use reviews to verify if a change will work when its submitted.  If it uses old code then the pipeline may succeed building when the change will actually break when submitted.

       

      I expect reviews triggering a new pipeline branch to sync to head rather than the CL of the review.

        Attachments

          Activity

          Hide
          p4karl Karl Wirth added a comment -

          Hi William Brode - Thanks for highlighting this.

          I can see that we will need to cater for delayed builds so we probably want the review to build at HEAD when review was generated/updated/committed. I'll do some testing here and get back to you.

          Karl

          Show
          p4karl Karl Wirth added a comment - Hi William Brode - Thanks for highlighting this. I can see that we will need to cater for delayed builds so we probably want the review to build at HEAD when review was generated/updated/committed. I'll do some testing here and get back to you. Karl
          Hide
          wbrode William Brode added a comment -

          Hi Karl Wirth - I may be able to fix it and send back a pull request.  I mostly wanted to make sure you guys agreed on the desired behavior first.

          Why do you think we would want to build at HEAD only when the review was generated/updated/committed?  I think even in that case I would prefer it to run against head.  Better to catch any issues in the review than when its submitted.

          The primary argument I see against running at head is that if someone else broke the build after you open the review it could cause the build of the review to fail for unrelated issues.  However, if you open a review right after someone breaks the build you actually can't do anything to get it to work (which is the issue that prompted me to look into this).  Since it will just keep running at the CL of the review.  And if we sync to head, then when someone fixes the build - just rerunning the review branch should fix it.

          Show
          wbrode William Brode added a comment - Hi Karl Wirth - I may be able to fix it and send back a pull request.  I mostly wanted to make sure you guys agreed on the desired behavior first. Why do you think we would want to build at HEAD only when the review was generated/updated/committed?  I think even in that case I would prefer it to run against head.  Better to catch any issues in the review than when its submitted. The primary argument I see against running at head is that if someone else broke the build after you open the review it could cause the build of the review to fail for unrelated issues.  However, if you open a review right after someone breaks the build you actually can't do anything to get it to work (which is the issue that prompted me to look into this).  Since it will just keep running at the CL of the review.  And if we sync to head, then when someone fixes the build - just rerunning the review branch should fix it.
          Hide
          p4karl Karl Wirth added a comment -

          Hi William Brode -I agree. I've been trying to get my head around the basic scenario's and the edge cases to those.

          I'll discuss with the developers and get back to you.

          My notes for me and devs.

          Options:

          • Pin at good build label? - A possible option but paramaterized builds or code in jenkinsfile could be used to implement this if really needed (not multibranch).
          • Pin at current head?  - Valid for the time the review is created updated and committed (pre-commit). Cannot see a negative.
          • Pin at review CL? - What if the code has moved on and the review is then updated. Need to sync to the later code.
          • Pin at head when review created/updated/comitted? - Would allow for delayed builds but why is this better than current head?

           

          Reproduction steps for sync at review CL

          (1) Create a multibranch job with a known Swarm project with branches defined that contain a Jenkinsfile as the branch sources.

          (2) Create a review.

          (3) Use 'Scan Multibranch Pipeline Now' to build review.

          (4) Submit 5 seperate changelists to move the changelist counter on.

          (5) Update the review with a new version of the file:

           

          p4 unshelve -s REVIEWNUMBER
          echo test >> FileInReview
          p4 shelve FileInReview
             Use the text #review-REVIEWNUMBER in the shelve description text.

          (6) Use 'Scan Multibranch Pipeline Now' to build review.

          (7) The review will sync to the REVIEWNUMBER. For example my review number was '2138' but my head changelist was 2224 for this branch (and whole server).

          P4 Task: syncing files at change: 2138
          ... p4 sync /var/lib/jenkins/workspace/SwarmMultiBranch_2138-ABD46HS2B5QL7YUAEOKK7ZF263___ -p4 sync /var/lib/jenkins/workspace/SwarmMultiBranch_2138-ABD46HS2B5QL7YUAEOKK7ZF263QXSASZ4PXEGIEWBMTBRK3QG5AQ/...@2138
          
          /var/lib/jenkins/workspace/SwarmMultiBranch_2138-ABD46HS2B5QL7YUAEOKK7ZF263QXSASZ4PXEGIEWBMTBRK3QG5AQ/...@2138 - file(s) up-to-date.
          
          duration: (3ms)
          
          P4 Task: unshelve review: 2138
          ... p4 unshelve -f -s2138 -cdefault +//depot/Project1/f2136
          

           

           

          Show
          p4karl Karl Wirth added a comment - Hi William Brode -I agree. I've been trying to get my head around the basic scenario's and the edge cases to those. I'll discuss with the developers and get back to you. My notes for me and devs. Options: Pin at good build label? - A possible option but paramaterized builds or code in jenkinsfile could be used to implement this if really needed (not multibranch). Pin at current head?  - Valid for the time the review is created updated and committed (pre-commit). Cannot see a negative. Pin at review CL? - What if the code has moved on and the review is then updated. Need to sync to the later code. Pin at head when review created/updated/comitted? - Would allow for delayed builds but why is this better than current head?   Reproduction steps for sync at review CL (1) Create a multibranch job with a known Swarm project with branches defined that contain a Jenkinsfile as the branch sources. (2) Create a review. (3) Use 'Scan Multibranch Pipeline Now' to build review. (4) Submit 5 seperate changelists to move the changelist counter on. (5) Update the review with a new version of the file:   p4 unshelve -s REVIEWNUMBER echo test >> FileInReview p4 shelve FileInReview    Use the text #review-REVIEWNUMBER in the shelve description text. (6) Use 'Scan Multibranch Pipeline Now' to build review. (7) The review will sync to the REVIEWNUMBER. For example my review number was '2138' but my head changelist was 2224 for this branch (and whole server). P4 Task: syncing files at change: 2138 ... p4 sync / var /lib/jenkins/workspace/SwarmMultiBranch_2138-ABD46HS2B5QL7YUAEOKK7ZF263___ -p4 sync / var /lib/jenkins/workspace/SwarmMultiBranch_2138-ABD46HS2B5QL7YUAEOKK7ZF263QXSASZ4PXEGIEWBMTBRK3QG5AQ/...@2138 / var /lib/jenkins/workspace/SwarmMultiBranch_2138-ABD46HS2B5QL7YUAEOKK7ZF263QXSASZ4PXEGIEWBMTBRK3QG5AQ/...@2138 - file(s) up-to-date. duration: (3ms) P4 Task: unshelve review: 2138 ... p4 unshelve -f -s2138 -cdefault + //depot/Project1/f2136    
          Hide
          wbrode William Brode added a comment -

          Thanks for being so diligent Karl Wirth!  Your options and notes look good to me.  I will say that I've seen some teams use an option like "Pin at a good build label" before (this was a custom built system using shelved CLs - not going through swarm).  It is another viable way to think of the problem (verify their change was good at some point in time) - but I'm not really sure how you would implement that for multibranch pipelines with helix swarm source.  I'm also not convinced that way of approaching the problem is really that useful - I think its just a way of getting around your master branch being broken for way too long (the real problem).

          Show
          wbrode William Brode added a comment - Thanks for being so diligent Karl Wirth !  Your options and notes look good to me.  I will say that I've seen some teams use an option like "Pin at a good build label" before (this was a custom built system using shelved CLs - not going through swarm).  It is another viable way to think of the problem (verify their change was good at some point in time) - but I'm not really sure how you would implement that for multibranch pipelines with helix swarm source.  I'm also not convinced that way of approaching the problem is really that useful - I think its just a way of getting around your master branch being broken for way too long (the real problem).
          Hide
          p4paul Paul Allen added a comment -

          Hi Guys, just back from vacation so may need a bit of time to get up speed on this issue...

          Using 'head' (the latest change on that branch) might be an alternative, but there could be side effects if the 'head' is newer than the shelf/review.  The P4 plugin may need to perform a resolve step (forcing the accept of the shelve's content) - I seem to remember writing something like this, but can't remember if it was released or in a dev branch.

          I'll take a look and refresh myself on the Swarm review part of the code.

          Show
          p4paul Paul Allen added a comment - Hi Guys, just back from vacation so may need a bit of time to get up speed on this issue... Using 'head' (the latest change on that branch) might be an alternative, but there could be side effects if the 'head' is newer than the shelf/review.  The P4 plugin may need to perform a resolve step (forcing the accept of the shelve's content) - I seem to remember writing something like this, but can't remember if it was released or in a dev branch. I'll take a look and refresh myself on the Swarm review part of the code.
          Hide
          wbrode William Brode added a comment -

          Good point on needing to resolve.  I hadn't considered how that makes things more complicated.  But as you mentioned the best way to handle it is probably to just take the shelved content (as opposed to trying to do some safe resolve or something).  I suppose another option would be to fail?  Return an error saying a resolve is required so the person can resolve it locally and change the files in the review.  That may actually be better - since they will have to resolve before submitting anyways.

          Show
          wbrode William Brode added a comment - Good point on needing to resolve.  I hadn't considered how that makes things more complicated.  But as you mentioned the best way to handle it is probably to just take the shelved content (as opposed to trying to do some safe resolve or something).  I suppose another option would be to fail?  Return an error saying a resolve is required so the person can resolve it locally and change the files in the review.  That may actually be better - since they will have to resolve before submitting anyways.

            People

            • Assignee:
              p4karl Karl Wirth
              Reporter:
              wbrode William Brode
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: