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

p4 fetches builds changes (individual changelist details) unsafely, can save wrong changes

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open (View Workflow)
    • Priority: Minor
    • Resolution: Unresolved
    • Component/s: p4-plugin
    • Labels:
      None
    • Environment:
      Jenkins 2.269
      p4-jenkins 1.11.1
    • Similar Issues:

      Description

      Summary

      The plugin uses p4 changes -m20 //CLIENT/...@CL1,LAST_CL to fetch the changelog since the previous build.

      But it uses p4 changes -l -m1 @CL to fetch the details of each change, without scoping it to the Perforce client/workspace. The @CL filter without a file pattern doesn't seem to always work properly and can return any recent CL from the Perforce server, even from different repositories.

      Respective code paths – Checkout.getChangesFull() calls:

      Symptoms

      We noticed the following on a recent build's Perforce sync:

      ...
      (p4):cmd:... p4 changes -m20 //jenkins-[WORKSPACE-ID]___
      p4 changes -m20 //jenkins-[WORKSPACE-ID]-0/...@3574547,3575421
      
      Change 3575421 on 2021/02/09 by X@Y ...
      Change 3575321 on 2021/02/09 by X@Y ...
      Change 3574547 on 2021/02/08 by X@Y ...
      (p4):stop:8
      (p4):cmd:... p4 changes -l -m1 @3575421
      p4 changes -l -m1 @3575421
      
      Change 3575420 on 2021/02/09 by swarmadmin@swarm-2c02b25f-[snip] *pending* [snip CL description]
      (p4):stop:9
      (p4):cmd:... p4 user -o swarmadmin
      p4 user -o swarmadmin
      
      (p4):stop:10
      (p4):cmd:... p4 describe -s -S 3575420
      p4 describe -s -S 3575420
      
      (p4):stop:11
      (p4):cmd:... p4 fixes -c3575420
      p4 fixes -c3575420
      
      (p4):stop:12
      (p4):cmd:... p4 changes -l -m1 @3575321
      p4 changes -l -m1 @3575321
      
      Change 3575321 on 2021/02/09 by X@Y ...
      ..
      • p4-jenkins found CLs 3575421, 3575321 for the build changes (+ 3574547 from previous build/syncID)
      • Fetching details for CL 3575421 found... CL 3575420????
      • Fetching details for CL 3575321 found 3575321 details
      • → CLs 3575420 and 3575321 are saved in the Jenkins build's "changes".

      After testing the behaviour of p4 changes -l -m1 @X I'm surprised this works at all for most CLs (3575321 above), the command seems to always return the very latest CL on the entire Perforce server when I run it locally:

      > p4 changes -l -m1 @3575321
      Change 3578492 on 2021/02/16 by Y@Z...

       

      We have configured our builds with email-ext to send notifications to "culprits", i.e. all CL authors that led to a build failure. As this relies on the changes recorded by p4-jenkins, the wrong changes can notify the wrong people.
      Our case above ended up emailing our IT department (swarmadmin) as the owner of that Swarm change that was completely unrelated to the build.

      (internal ref: stresstest#130)

        Attachments

          Activity

          Hide
          r_asiebert Adrien Siebert added a comment -

          Note that there was a bit of guesswork to find the relevant code paths (can't be certain which concrete classes were in use), I'm not sure they're accurate.

          Show
          r_asiebert Adrien Siebert added a comment - Note that there was a bit of guesswork to find the relevant code paths (can't be certain which concrete classes were in use), I'm not sure they're accurate.
          Hide
          p4karl Karl Wirth added a comment -

          Hi Adrien Siebert ,

           

          It should be using a workspace because different syncs will generate different change reporting based on what code they are run against. If it  is not I'm going to need to dig deeper.

          Can you please send an email to support@perforce.com with:

          (1) Full console log from this job.

          (2) Detailed information about this job. For example if pipeline or multibranch please include the Jenkinsfile. If freestyle a couple of screenshots showing the important definitions.

          (3) A cut down and compressed version of your P4D server log for the time of this build.

          (4) Output from 'p4 -Ztag info'.

           

          Thanks in advance.

           

          Karl

          Show
          p4karl Karl Wirth added a comment - Hi Adrien Siebert ,   It should be using a workspace because different syncs will generate different change reporting based on what code they are run against. If it  is not I'm going to need to dig deeper. Can you please send an email to support@perforce.com with: (1) Full console log from this job. (2) Detailed information about this job. For example if pipeline or multibranch please include the Jenkinsfile. If freestyle a couple of screenshots showing the important definitions. (3) A cut down and compressed version of your P4D server log for the time of this build. (4) Output from 'p4 -Ztag info'.   Thanks in advance.   Karl
          Hide
          r_asiebert Adrien Siebert added a comment -

          Hi Karl,

          copy that; I'll keep an eye out for future occurrences and engage with our Perforce server owners to get that log if/when it occurs.

          One clarification on (4) Output from 'p4 -Ztag info'.": where should we execute this command? The p4 CLI is not available from the Jenkins agent where jobs are running.

           

          Or is my local anomaly (p4 changes -l -m1 @123 returning a different CL) enough to warrant a support ticket?

           

           

          Show
          r_asiebert Adrien Siebert added a comment - Hi Karl, copy that; I'll keep an eye out for future occurrences and engage with our Perforce server owners to get that log if/when it occurs. One clarification on  (4) Output from 'p4 -Ztag info'." : where should we execute this command? The p4 CLI is not available from the Jenkins agent where jobs are running.   Or is my local anomaly ( p4 changes -l -m1 @123 returning a different CL) enough to warrant a support ticket?    
          Hide
          p4karl Karl Wirth added a comment -

          Hi Adrien Siebert - Just FYI - The 'p4 info' can be run from any machine as long as it connects to the same P4D server as the Jenkins machine.

          For the support ticket my brain jsut started working. This looks like a shelved changelist was part of the build:

           

          (p4):cmd:... p4 describe -s -S 3575420
           p4 describe -s -S 3575420
          

          Note the -'S' on the command. If this was a review build then it would build changes

          This would also tie in with the user who is running the build:

          swarmadmin@swarm-2c02b25f-

           

          If this is a pre commit review build we will pass the shelved changelist and the job will build that in amongst the changes it's actually syncing.

          Can you check if '3575420' is part of a Swarm review?

          Show
          p4karl Karl Wirth added a comment - Hi Adrien Siebert - Just FYI - The 'p4 info' can be run from any machine as long as it connects to the same P4D server as the Jenkins machine. For the support ticket my brain jsut started working. This looks like a shelved changelist was part of the build:   (p4):cmd:... p4 describe -s -S 3575420 p4 describe -s -S 3575420 Note the -'S' on the command. If this was a review build then it would build changes This would also tie in with the user who is running the build: swarmadmin@swarm-2c02b25f-   If this is a pre commit review build we will pass the shelved changelist and the job will build that in amongst the changes it's actually syncing. Can you check if '3575420' is part of a Swarm review?
          Hide
          r_asiebert Adrien Siebert added a comment - - edited

          Hey Karl,

          pulling back a bit, 3575420 is not even part of the build/sync being done; in fact it's not even related to the repo that is check out. Going back to the earlier sync log:

          (p4):cmd:... p4 changes -m20 //jenkins-[WORKSPACE-ID]___
          p4 changes -m20 //jenkins-[WORKSPACE-ID]-0/...@3574547,3575421

          Change 3575421 on 2021/02/09 by X@Y ...
          Change 3575321 on 2021/02/09 by X@Y ...
          Change 3574547 on 2021/02/08 by X@Y ...
          (p4):stop:8

          (p4):cmd:... p4 changes -l -m1 @3575421
          p4 changes -l -m1 @3575421

          Change 3575420 on 2021/02/09 by swarmadmin@swarm-2c02b25f-[snip] pending [snip CL description]
          (p4):stop:9

          I'm not entirely sure what 3575420 is – even now (created 15d ago) it seems to be a pending changelist with a shelved file, indeed related to a Swarm review that has been committed since. As far as I can tell, it's completely unrelated to 3575421.

           

          We just had another occurrence on another Jenkins pipeline, also related to a Swarm review:

          ... p4 changes -m20 //jenkins-[WORKSPACE-ID]___ -
          p4 changes -m20 //jenkins-[WORKSPACE-ID]-0/...@3582298,3582449

          Change 3582449 on 2021/02/24 by X@Y 'Merging }}{{[snip]}}{{'
          Change 3582298 on 2021/02/24 by X@Y 'Added }}{{[snip]}}{{'
          ... p4 changes -l -m1 @3582449 -
          p4 changes -l -m1 @3582449

          Change 3582448 on 2021/02/24 by swarmadmin@swarm-2c02b25f-[snip] pending 'Fix bug for [snip]'
          ... p4 user -o swarmadmin +
          ... p4 describe -s -S 3582448 -
          p4 describe -s -S 3582448

          ... p4 fixes -c3582448 -
          p4 fixes -c3582448

          ... done

          Similar to before; 3582448 is a pending/shelved CL related to a Swarm review (currently open), covering changes in a separate repo.

           

          It sounds like you may have an idea on what Swarm might be doing here; I can still attempt to fetch some Perforce server logs surrounding that time — only a bit tricky as the Jenkins logs are not dated.

          [Edit] Sorry about the edits, Jira's visual editor seems to do some wonky things

          Show
          r_asiebert Adrien Siebert added a comment - - edited Hey Karl, pulling back a bit, 3575420 is not even part of the build/sync being done; in fact it's not even related to the repo that is check out. Going back to the earlier sync log: (p4):cmd:... p4 changes -m20 //jenkins- [WORKSPACE-ID] ___ p4 changes -m20 //jenkins- [WORKSPACE-ID] -0/...@3574547, 3575421 Change 3575421 on 2021/02/09 by X@Y ... Change 3575321 on 2021/02/09 by X@Y ... Change 3574547 on 2021/02/08 by X@Y ... (p4):stop:8 (p4):cmd:... p4 changes -l -m1 @ 3575421 p4 changes -l -m1 @ 3575421 Change 3575420 on 2021/02/09 by swarmadmin@swarm-2c02b25f- [snip] pending [snip CL description] (p4):stop:9 I'm not entirely sure what 3575420 is – even now (created 15d ago) it seems to be a pending changelist with a shelved file, indeed related to a Swarm review that has been committed since. As far as I can tell, it's completely unrelated to 3575421.   We just had another occurrence on another Jenkins pipeline, also related to a Swarm review: ... p4 changes -m20 //jenkins- [WORKSPACE-ID] ___ - p4 changes -m20 //jenkins- [WORKSPACE-ID] -0/...@3582298, 3582449 Change 3582449 on 2021/02/24 by X@Y 'Merging }}{{ [snip] }}{{' Change 3582298 on 2021/02/24 by X@Y 'Added }}{{ [snip] }}{{' ... p4 changes -l -m1 @ 3582449 - p4 changes -l -m1 @ 3582449 Change 3582448 on 2021/02/24 by swarmadmin@swarm-2c02b25f- [snip] pending 'Fix bug for [snip] ' ... p4 user -o swarmadmin + ... p4 describe -s -S 3582448 - p4 describe -s -S 3582448 ... p4 fixes -c 3582448 - p4 fixes -c 3582448 ... done Similar to before; 3582448 is a pending/shelved CL related to a Swarm review (currently open), covering changes in a separate repo.   It sounds like you may have an idea on what Swarm might be doing here; I can still attempt to fetch some Perforce server logs surrounding that time — only a bit tricky as the Jenkins logs are not dated. [Edit] Sorry about the edits, Jira's visual editor seems to do some wonky things
          Hide
          p4karl Karl Wirth added a comment -

          Hi Adrien Siebert  - I'd like to get some more information so have sent an email directly to you. Also - I agree abot the wonky editor .

          Show
          p4karl Karl Wirth added a comment - Hi Adrien Siebert   - I'd like to get some more information so have sent an email directly to you. Also - I agree abot the wonky editor .

            People

            Assignee:
            p4karl Karl Wirth
            Reporter:
            r_asiebert Adrien Siebert
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated: