Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RemovedNodeListener.cancelOwnerExecution can be noisy #387

Merged

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 31, 2024

Regression in #372 caught by PCT on kubernetes:

java.lang.AssertionError: 
routine build should not issue warnings
Expected: an empty iterable
     but: ["org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution$RemovedNodeListener.cancelOwnerExecution: null"]
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.csanchez.jenkins.plugins.kubernetes.pipeline.KubernetesPipelineTest.runInPod(KubernetesPipelineTest.java:245)

after

   5.274 [run In Pod #1] [Pipeline] // node
   5.274 [run In Pod #1] [Pipeline] }
   5.274 [run In Pod #1] [Pipeline] // podTemplate
   5.274 [run In Pod #1] [Pipeline] semaphore
   5.274 [run In Pod #1] [Pipeline] End of Pipeline
   5.297 [run In Pod #1] Finished: SUCCESS
   5.300 [id=3097]	FINE	o.c.j.p.k.KubernetesCloud#connect: Connected to Kubernetes kubernetes URL https://127.0.0.1:…/ namespace kubernetes-plugin-test
   5.301 [id=3097]	INFO	o.c.j.p.k.KubernetesTestUtil#deletePods: Waiting for pods to terminate
   5.303 [id=1158]	INFO	o.c.j.p.k.KubernetesSlave#deleteSlavePod: Terminated Kubernetes instance for agent kubernetes-plugin-test/runinpod-…-…
   5.303 [id=1158]	INFO	o.c.j.p.k.KubernetesSlave#_terminate: Disconnected computer runinpod-…-…
   5.306 [id=1158]	WARNING	o.j.p.w.s.s.ExecutorStepExecution$RemovedNodeListener#cancelOwnerExecution
java.io.IOException: cannot find current thread
	at org.jenkinsci.plugins.workflow.cps.CpsStepContext.doGet(CpsStepContext.java:295)
	at org.jenkinsci.plugins.workflow.support.DefaultStepContext.get(DefaultStepContext.java:75)
	at org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution$RemovedNodeListener.cancelOwnerExecution(ExecutorStepExecution.java:379)
	at org.jenkinsci.plugins.workflow.support.steps.ExecutorStepExecution$RemovedNodeListener.onDeleted(ExecutorStepExecution.java:354)
	at jenkins.model.NodeListener.lambda$fireOnDeleted$2(NodeListener.java:97)
	at jenkins.util.Listeners.lambda$notify$0(Listeners.java:59)
	at jenkins.util.Listeners.notify(Listeners.java:70)
	at jenkins.model.NodeListener.fireOnDeleted(NodeListener.java:97)
	at jenkins.model.Nodes.removeNode(Nodes.java:297)
	at jenkins.model.Jenkins.removeNode(Jenkins.java:2257)
	at hudson.slaves.AbstractCloudSlave.terminate(AbstractCloudSlave.java:91)
	at org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy.lambda$done$5(OnceRetentionStrategy.java:142)
	at hudson.model.Queue._withLock(Queue.java:1410)
	at hudson.model.Queue.withLock(Queue.java:1284)
	at org.jenkinsci.plugins.durabletask.executors.OnceRetentionStrategy.lambda$done$6(OnceRetentionStrategy.java:137)
	at …

Previously this had happened after some delay, giving the Executor a chance to detach from the Computer (I suppose). Now cancelOwnerExecution is run immediately, and sometimes the Executor is still there yet the StepContext is invalid since the node block has already completed. The condition is harmless (there is no longer any body to cancel), so this stack trace was just noise, which the test case rightly flagged.

@jglick jglick added the bug label Jul 31, 2024
@jglick jglick requested a review from a team as a code owner July 31, 2024 13:17
@@ -389,11 +389,7 @@ private static void cancelOwnerExecution(Node node, CauseOfInterruption... cause
return;
}
listener.getLogger().println("Agent " + node.getNodeName() + " was deleted; cancelling node body");
if (Util.isOverridden(BodyExecution.class, body.getClass(), "cancel", Throwable.class)) {
body.cancel(new FlowInterruptedException(Result.ABORTED, false, causes));
} else { // TODO remove once https://github.com/jenkinsci/workflow-cps-plugin/pull/570 is widely deployed
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jenkinsci/workflow-cps-plugin#570 was released over two years ago now, so this is likely safe enough.

@jglick jglick merged commit 2fd76fb into jenkinsci:master Jul 31, 2024
16 checks passed
@jglick jglick deleted the RemovedNodeListener.cancelOwnerExecution branch July 31, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants