Skip to content

[ISSUE-2830] Let yaml override inherited containerTemplate fields - #2855

Open
RajeshRajendiran wants to merge 4 commits into
jenkinsci:masterfrom
RajeshRajendiran:fix/issue-2830-yaml-inherit-precedence
Open

[ISSUE-2830] Let yaml override inherited containerTemplate fields#2855
RajeshRajendiran wants to merge 4 commits into
jenkinsci:masterfrom
RajeshRajendiran:fix/issue-2830-yaml-inherit-precedence

Conversation

@RajeshRajendiran

Copy link
Copy Markdown

Summary

Fixes #2830.

When a pod template uses inheritFrom, a container defined in the parent's
containerTemplate was always preferred over a same-named container defined
in the child's own yaml block — even though the child's yaml is the more
specific, local override. This happened regardless of yamlMergeStrategy.

containerTemplate-based overrides worked correctly, because they're merged
by container name earlier, during the PodTemplate-level combine(). yaml
overrides were merged later at the Pod level, where the already-merged
containerTemplate-derived container list unconditionally won over yaml
for any matching container name — so the two override mechanisms behaved
inconsistently for what should be the same use case.

Fix

  • PodTemplate now tracks which container names were declared directly on
    that template (as opposed to only present via inheritance).
  • During the final pod merge in PodTemplateBuilder, containers that are
    purely inherited (not redeclared locally) now let yaml take precedence,
    matching how containerTemplate overrides already behaved.
  • Containers declared locally via containerTemplate are unaffected and
    keep taking priority over yaml, exactly as before.

This is intentionally scoped to the inheritFrom case only. Precedence
between containerTemplate and yaml within a single (non-inherited)
template is unchanged.

Test plan

  • Updated PodTemplateBuilderTest#testInheritsFromWithYaml, which
    previously pinned the old (documented as "counter intuitive")
    behavior, to assert the corrected precedence.
  • Added PodTemplateBuilderTest#yamlOverridesInheritedContainerTemplate,
    reproducing the exact scenario from container in declarative pipeline yaml ignored while containerTemplate works #2830 and asserting that
    containerTemplate and yaml overrides now behave identically.
  • Full existing test suite passes with no regressions.

When a pod template uses inheritFrom, a container that only exists
because it was inherited from a parent's containerTemplate was always
preferred over a same-named container defined in the child's own yaml
block, regardless of yamlMergeStrategy. containerTemplate overrides
worked correctly since they're merged by name earlier, in the
PodTemplate-level combine, so the two mechanisms behaved
inconsistently for the same use case.

Track which container names are declared directly on a template
(not inherited) and, only for containers that are purely inherited,
let yaml take precedence over the inherited containerTemplate fields
during the final pod merge. Containers declared locally via
containerTemplate keep taking priority over yaml, unchanged.
@RajeshRajendiran
RajeshRajendiran requested a review from a team as a code owner July 19, 2026 19:01
Text block opener must stay on the same line as setYaml(, per the
project's spotless style (google-java-format).
@jglick jglick added the bug Bug Fixes label Jul 22, 2026
Comment on lines -568 to +571
PodTemplateUtilsTest.assertQuantity("1", jnlp.getResources().getLimits().get("cpu"));
assertEquals("jenkins-jnlp-override", jnlp.getImage());
PodTemplateUtilsTest.assertQuantity("2", jnlp.getResources().getLimits().get("cpu"));

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.

So, changing tested behavior dating to #372 (eight years ago) and potentially breaking working setups.

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.

Fair concern — this does flip behavior that's been pinned since #372. To be precise about the blast radius: the change only affects containers that are purely inherited from a parent's containerTemplate with no local
override at all.
If a template redeclares the container via its own containerTemplate (the common override pattern), that still wins over yaml, unchanged.
So the only setups affected are ones using inheritFrom + a yaml block to override a container that already exists in the parent template — which per #2830 currently silently does nothing, so I'd guess most of those configs are unintentionally inert today rather than intentionally relying on the old precedence. But I don't have visibility into how common that pattern is in the wild.

Would you be more comfortable with this landing as a straight fix, or would you rather see it gated behind an opt-in flag (e.g. on PodTemplate) so existing behavior is preserved by default and users opt into the corrected merge order? Happy to go either way, just don't want to guess wrong on compat expectations here.

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.

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.

container in declarative pipeline yaml ignored while containerTemplate works

2 participants