Delete DownstreamJobListener and instead create NodeDownstreamBuildAction dynamically during graph processing based on DownstreamBuildAction

[dwnusbaum]

DownstreamJobListener here has the same bug as jenkinsci/pipeline-build-step-plugin#127 for completed builds, which was fixed by jenkinsci/pipeline-build-step-plugin#128. Prior to jenkinsci/workflow-cps-plugin#807, this only worked as long as you were using the MAX_SURVIVABILITY durability level, otherwise it did nothing when the build was already complete. jenkinsci/workflow-cps-plugin#826 then made it so a warning was logged instead of the save being silently ignored in the problematic case, which can lead to log spam.

Covered by existing tests in GraphBuilderTest and PipelineNodeTest as far as I can tell (at least if I change this PR to just never create NodeDownstreamBuildAction, tests in those classes start to fail).

You can test this by running a Pipeline like build job: 'downstream', wait: false where the downstream job is simply sleep 10 or similar, observe that the upstream build completes almost immediately, wait 10 seconds for the downstream job to complete, and then open Blue Ocean and see that the downstream build link is present in the API response. Whether it shows up properly in the UI seems to be inconsistent. See discussion in JENKINS-38339 and JENKINS-60995, which both predate the issue I am trying to fix.

Submitter checklist

• Link to JIRA ticket in description, if appropriate.
• Change is code complete and matches issue description
• Appropriate unit or acceptance tests or explanation to why this change has no tests
• Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

• Run the changes and verified the change matches the issue description
• Reviewed the code
• Verified that the appropriate tests have been written or valid explanation given

[jglick]

Can we not just delete (or deprecate) NodeDownstreamBuildAction and change whatever code is calling is to look up org.jenkinsci.plugins.workflow.support.steps.build.DownstreamBuildAction on demand?

[jglick]

blueocean-plugin/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineNodeImpl.java

Lines 191 to 192 in a910c53

	// Actions from any child nodes
	actions.addAll(node.getPipelineActions(NodeDownstreamBuildAction.class));

+

blueocean-plugin/blueocean-dashboard/src/main/js/components/karaoke/components/Pipeline.jsx

Lines 312 to 314 in a910c53

	downstreamRuns = this.pager.currentNode.actions
	.filter(action => action._class === 'io.jenkins.blueocean.listeners.NodeDownstreamBuildAction')
	.map(action => ({ runDescription: action.description, runLink: action.link.href }));

IIUC

[dwnusbaum]

Can we not just delete (or deprecate) NodeDownstreamBuildAction and change whatever code is calling is to look up org.jenkinsci.plugins.workflow.support.steps.build.DownstreamBuildAction on demand?

Yes I considered this kind of approach, but I think it would only move the performance issue to graph loading time (a bit better at least since it wouldn't affect non-Blue Ocean use cases). I can switch to it though.

[dwnusbaum]

Oh, and I'm not sure if you can just change the code that looks for the action in the special Blue Ocean FlowNodeWrapper class in PipelineNodeImpl. I think you need to change the code that sets the actions when the graph is being processed so that the action propagates up to the node that is visible in Blue Ocean correctly.

[dwnusbaum]

See b7ef607 for an approach that moves the logic to graph processing time.

Delaying build loading until a user actually clicks on one of the relevant links does not seem possible without UX changes, since we expect the description of the run to be available in the UI.

[jglick]

it wouldn't affect non-Blue Ocean use cases

Which is pretty important I think, since B.O. might be installed yet rarely used.

[jglick]

OK AFAICT

[jglick]

Blue Ocean doesn't build for me

If you do not need to test the GUI interactively, you can just use https://github.com/eirslett/frontend-maven-plugin?tab=readme-ov-file#skipping-execution (which will be a lot faster too).

[wkoot]

Can we not just delete (or deprecate) NodeDownstreamBuildAction and change whatever code is calling is to look up org.jenkinsci.plugins.workflow.support.steps.build.DownstreamBuildAction on demand?

Would this by any chance also address https://issues.jenkins.io/browse/JENKINS-38339 ?

[dwnusbaum]

Would this by any chance also address https://issues.jenkins.io/browse/JENKINS-38339 ?

I am not sure, but given that JENKINS-38339 and JENKINS-60995 are both many years old, whereas the issue I am trying to fix was caused by jenkinsci/workflow-cps-plugin#826 earlier this year, I suspect that this PR will not fix those issues. This comment seems like a decent hypothesis as to the underlying problem. There is a chance it could help, but someone would need to investigate the bug reports and find a way to reproduce the reported issues to know either way.

If you have a way to reproduce the bugs described in those tickets consistently, then by all means please try out this PR and let us know if it helps, and post your reproduction steps in Jira.

[wkoot]

If you have a way to reproduce the bugs described in those tickets consistently, then by all means please try out this PR and let us know if it helps, and post your reproduction steps in Jira.

Sadly, it's an on-and-off issue which affects about a quarter of builds. We'll just have to drop BlueOcean and move to Pipeline Graph View, thanks for your quick reply and clarity