Skip to content

Wait for pod template on resume to avoid leaking agent pods after restart#2851

Open
RanjitCS wants to merge 8 commits into
jenkinsci:masterfrom
RanjitCS:JENKINS-67390
Open

Wait for pod template on resume to avoid leaking agent pods after restart#2851
RanjitCS wants to merge 8 commits into
jenkinsci:masterfrom
RanjitCS:JENKINS-67390

Conversation

@RanjitCS

@RanjitCS RanjitCS commented Jul 10, 2026

Copy link
Copy Markdown

If the controller restarts while a dynamic pod template agent is being provisioned, KubernetesLauncher.launch() fails on resume because the template isnt re-registered yet, leaving the pod orphaned

Testing done

  • Tested practically in an jenkins cluster
  • Added integration test

Submitter checklist

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

@RanjitCS RanjitCS marked this pull request as ready for review July 10, 2026 17:31
@RanjitCS RanjitCS requested a review from a team as a code owner July 10, 2026 17:31
@jglick jglick added the bug Bug Fixes label Jul 10, 2026
@jglick jglick changed the title [JENKINS-67390][ISSUE-2512] Wait for pod template on resume to avoid leaking agent pods after restart Wait for pod template on resume to avoid leaking agent pods after restart Jul 10, 2026

@jglick jglick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is solving some bug. But it does not seem to have anything to do with what was reported in #2512.

private static final Logger LOGGER = Logger.getLogger(KubernetesLauncher.class.getName());

private static final long TEMPLATE_RESOLUTION_TIMEOUT_SECONDS =
SystemProperties.getLong(KubernetesLauncher.class.getName() + ".templateResolutionTimeoutSeconds", 60L);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use getDuration please, and remove the time unit from the property name, and adjust docs to clarify that you pass a unit expression.

@RanjitCS RanjitCS Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

f4ffb03, i had to bump the baseline for getDuration

* re-registers the dynamic template asynchronously, and this launcher may run first. Rather than failing
* immediately (which would leak the pod), poll for the template for a short while. Falls back to
* {@link KubernetesSlave#getTemplate()} (which throws a descriptive exception) if it never becomes available.
* See <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a>.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* See <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a>.

Anyone interested can git blame.

@RanjitCS RanjitCS Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

* See <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a>.
*/
@NonNull
private static PodTemplate waitForTemplate(KubernetesSlave node) throws InterruptedException {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this not be waiting only for dynamic templates? A global template would already be available, or should be, so if it is not then we may as well fail immediately.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Comment on lines +354 to +355
LOGGER.log(
FINE,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
LOGGER.log(
FINE,
LOGGER.fine(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

* launch and restores it shortly after. Without the fix the pod is orphaned and with it the build resumes on the
* same pod and the pod is cleaned up.
*/
@Issue("JENKINS-67390")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Jira is dead. Refer to GH Issues

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

}

/**
* Reproduces <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a> if, on resume,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* Reproduces <a href="https://github.com/jenkinsci/kubernetes-plugin/issues/2512">...</a> if, on resume,
* Reproduces a problem in which, on resume,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Comment on lines +456 to +458
cloud.removeDynamicTemplate(template);
// Restore it shortly after, mimicking PodTemplateStepExecution#onResume re-registering it late.
Timer.get().schedule(() -> cloud.addDynamicTemplate(template), 5, TimeUnit.SECONDS);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not very convincing. If there is no way to reliably reproduce the actual problem, I would rather not have a test at all.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

as its a very narrow window, the initial test which i had was more faithful but was flaky without the fix (i.e 4/10 failures without the fix, fully passes with the fix), i will remove the test itself then if this is not good enough

containerTemplate(name: 'busybox', image: 'busybox', command: 'cat', ttyEnabled: true)
]) {
node('orphan-race') {
container('busybox') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the container step relevant in this context?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed in f4ffb03

@@ -0,0 +1,15 @@
podTemplate(label: 'orphan-race', yaml: '''

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do not use label, long deprecated

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed in f4ffb03

@jglick

jglick commented Jul 10, 2026

Copy link
Copy Markdown
Member

Tested practically in an jenkins cluster

Details?

@RanjitCS

RanjitCS commented Jul 13, 2026

Copy link
Copy Markdown
Author

I think this is solving some bug. But it does not seem to have anything to do with what was reported in #2512.

thinking it through, as i found the same symptom (suspended agent, pod never cleaned up) while trying to reproduce, i assumed it was the same fix, but i was wrong, i will continue to focus on #2512, but i believe this is also a bug that can happen

edit: i have spent some time trying to reproduce #2512, but i have been unsuccessfull, i believe #1137 has solved the "agent stays suspended, pod reconnects but Jenkins does nothing" problem. this pr however is still valid

@RanjitCS

Copy link
Copy Markdown
Author

Tested practically in an jenkins cluster

Details?

  1. create a Pipeline with an inline podTemplate {} (a dynamic template) whose pod stays unschedulable i used nodeSelector: disktype=special. This keeps it pending so the agent never connects before the restart, forcing launch() to run again on resume and hit the race
  2. run it and wait for pod creation
  3. restart the controller while the pod is still pending
  4. on resume, make the pod schedulable, kubectl label node <node> disktype=special
  5. notice that the pod is orphaned

again, this is extremely timing dependent but can be reproduced consistently without any fake pause injection at least 5/10 tries i did

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants