[JENKINS-64800] Recursively check previous builds for changes during polling

[stuartrowe]

Fix for https://issues.jenkins.io/browse/JENKINS-64800.

My team recently ran into this issue because of problems with authentication against our company's LDAP servers.

In our case a run would fail at the p4 checkout stage because of the authentication issues. Subsequent polling tasks for the job would fail to find new changes because the last run didn't have an associated TagAction.

As suggested in one of the comments on JENKINS-64800, our fix iterates through earlier builds in an attempt to find a baseline for polling.

Testing done

There aren't currently any unit tests related to polling. Instead my team created a manual test with the following steps:

1. Temporarily swap a valid credential with an invalid one
2. Run a job using that credential and note that it fails during checkout
3. Restore the valid credential
4. Submit a change to trigger polling and confirm that polling is successful

We confirmed that this test failed with p4-plugin 1.14.1 and succeeded with the fix.

Submitter checklist

Preview Give feedback

1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
2. Ensure that the pull request title represents the desired changelog entry
3. Please describe what you did
4. Link to relevant issues in GitHub or Jira
5. Link to relevant pull requests, esp. upstream and downstream changes
6. Ensure you have provided tests - that demonstrates feature works or fixes the issue

[stuartrowe]

@p4paul could you please review this when you have a chance?

[stuartrowe]

Close + Reopen to re-trigger CI.

[stuartrowe]

@p4paul @skumar7322 are either of you available to review this?

[skumar7322]

Thanks, @stuartrowe for initiating this pull request. This change will throw the NPE when lastRun is null. Because of this, there are test failures in PollingTest, PerforceSCMSourceTest suit.
Please execute these test suites for your proposed changes. Also given our planned workload, we intend to address JENKINS-64800 in our upcoming releases.

[stuartrowe]

@skumar7322 thanks for pointing that out, I had overlooked it somehow. Should be addressed now 🤞.

[stuartrowe]

@skumar7322 there seems to be an issue with org.jenkinsci.plugins.p4.SimpleTestServer that is affecting other PRs too.

[stuartrowe]

@skumar7322 is there any update on the next planned release and a fix for JENKINS-64800? If not, can this PR please be revisited?

[skumar7322]

Hi @stuartrowe this change looks good to me. I will run the tests on this and merge it once all is good.

[skumar7322]

Hi @stuartrowe, The Following test cases failed while doing the merge. Could you please run these test on your system.
[ERROR] Failures:
[ERROR] PollingTest.testPipelineFailedPolling:815 Poll, but no build expected:<2> but was:<3>
[ERROR] PollingTest.testPollingBuildInProgress:699 Poll and trigger Build #2 expected:<2> but was:<3>
[ERROR] PollingTest.testPollingLatestChangeFilter:747 Poll and trigger Build #2 expected:<2> but was:<3>
[ERROR] Errors:
[ERROR] PollingTest.testShouldTriggerPipelineJobIfChanges » Execute Process exited with an error: 255 (Exit value: 255)

[skumar7322]

Following are logs shared in https://issues.jenkins.io/browse/JENKINS-64800
Started on Feb 2, 2021, 9:54:00 AM
Executor number at runtime: 0
P4: Polling on: master with:Mainline-EC2AMAZ-AAAAAAA
P4: Polling: No changes in previous build.
P4: Polling error; no previous change.
Done. Took 5 ms
No changes

List lastRefs = TagAction.getLastChange(lastRun, listener, syncID);
lastRefs is null. Analysing TagAction.getLastChange method it looks build found changes from the last build as there is no log message "No previous build found...".

In last this method will compare the syncID of the workspace with the syncID in the syncID of previous build changes if these match then only the change will be valid. But somehow this syncID is different from the last build hence this method returns null.

Now the question is why the syncID is not same.

They are using dynamic syncID as following:
workspace: staticSpec(
charset: 'none',
name: "Mainline-${env.COMPUTERNAME}",
syncID: "Mainline-${env.COMPUTERNAME}-${env.JOB_NAME}",
pinHost: false
)
Change in any of these variable ${env.COMPUTERNAME}-${env.JOB_NAME} in between two builds is causing this issue. Let me what is your view on this.

[stuartrowe]

Hi @skumar7322 I also see this failure on master when run locally:

[ERROR] PollingTest.testPipelineFailedPolling:815 Poll, but no build expected:<2> but was:<3>

and these errors:

[ERROR]   PollingTest.testMultiBranchFailedPolling » Execute Process exited with an error: -1 (Exit value: -1)
[ERROR]   PollingTest.testPatternListCaseSensitive » IOExceptionList E:\github\stuartrowe\p4-plugin\target\tmp-PollingTest-p4root
[ERROR]   PollingTest.testPollingMask » IOExceptionList E:\github\stuartrowe\p4-plugin\target\tmp-PollingTest-p4root
[ERROR]   PollingTest.testShouldNotTriggerPipelineIfNoChanges » Execute Process exited with an error: -1 (Exit value: -1)
[ERROR]   PollingTest.testShouldTriggerPipelineJobIfChanges » IOExceptionList E:\github\stuartrowe\p4-plugin\target\tmp-PollingTest-p4root

I will start by looking into these 2 additional failures:

[ERROR] PollingTest.testPollingBuildInProgress:699 Poll and trigger Build https://github.com/jenkinsci/p4-plugin/pull/2 expected:<2> but was:<3>
[ERROR] PollingTest.testPollingLatestChangeFilter:747 Poll and trigger Build https://github.com/jenkinsci/p4-plugin/pull/2 expected:<2> but was:<3>