Skip to content

quadlet: expand %N specifier in the Pod key#29113

Open
larrasket wants to merge 1 commit into
podman-container-tools:mainfrom
larrasket:quadlet-pod-specifier-expansion
Open

quadlet: expand %N specifier in the Pod key#29113
larrasket wants to merge 1 commit into
podman-container-tools:mainfrom
larrasket:quadlet-pod-specifier-expansion

Conversation

@larrasket

@larrasket larrasket commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

The Pod key of a .container unit accepts systemd specifiers, handlePod looked the referenced pod up in the units map using the raw value, a "Pod=%N.pod" failed with "quadlet pod unit %N.pod does not exist" instead of resolving to the pod named after the container.

this expand %N to the container's service name before the lookup, matching the existing handling inGetContainerResourceName.

Fixes: #29108

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • Tests have been added/updated (or no tests are needed)
  • Documentation has been updated (or no documentation changes are needed)
  • All commits pass make validatepr (format/lint checks)
  • Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

Fixed the quadlet `Pod=` key not expanding the `%N` specifier (e.g. `Pod=%N.pod`).

@ygalblum

ygalblum commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

This fix works only if the .pod and .container have the same name. Otherwise, Quadlet will still not find the pod the container belongs to. Also, as a result, it will work only for one .container. I'm not sure what's the benefit of single container pods in Podman (I might be wrong here)

So while it does give some benefit, I'm not sure how much.

@larrasket

Copy link
Copy Markdown
Contributor Author

My aim was narrower than that case,%N is expanded for a container's own resource name i.e. it's inconsistent that the same specifier silently fails in Pod= with a misleading pod unit %N.pod does not exist. If you'd not resolve specifiers in Pod= at all (which would also fit the man-page note about specifiers that reference the generated service unit breaking resource linking) then replacing the confusing error with a clear message plus a doc line would be a reasonable alternative imo

@ygalblum

ygalblum commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Sorry, not sure I understand your comment. First, Quadlet fails processing the .container file and logs the failure, so WDYM by silently fails.
Second, my concern is that it tries to fix a very narrow case in which the names are the same while actual usage will have different names making this fix irrelevant.
Third, in order to test it, you should add e2e test cases in: https://github.com/podman-container-tools/podman/blob/d5213284f44643cc95cd2494300d402ce9b1bb62/test/e2e/quadlet_test.go . When testing, add tests that use different names, that set the container's systemd service name (via ServiceName). If it works in different combinations, I might be wrong.

@larrasket larrasket force-pushed the quadlet-pod-specifier-expansion branch from 54fc79c to 8c05e29 Compare July 3, 2026 21:58
@larrasket

Copy link
Copy Markdown
Contributor Author

the problem is that it reports quadlet pod unit %N.pod does not exist, which points at a literal unit name, rather than indicating specifiers aren't resolved here. And no I don't think it's a narrow-case, %N expands to the container's service name (same as systemd's %N), not its file name. So with ServiceName= set, the container and pod files can have different names (like web.container with ServiceName=frontend and Pod=%N.pod resolves frontend.pod)
I've added the e2e cases in test/e2e/quadlet_test.go.

@ygalblum ygalblum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please explain the usage of this specifier in the Quadlet documentation:

Comment thread pkg/systemd/quadlet/quadlet_test.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Quadlet tests are implemented in /test/e2e. Please remove this file

@larrasket larrasket force-pushed the quadlet-pod-specifier-expansion branch from 8c05e29 to ce3ad95 Compare July 6, 2026 15:37
@larrasket larrasket requested a review from ygalblum July 6, 2026 15:40

@ygalblum ygalblum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks

@ygalblum

ygalblum commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

@larrasket you have an error in the man page. Not sure what it is

The Pod key of a .container unit accepts systemd specifiers, but handlePod
looked the referenced pod up in the units map using the raw value, so
"Pod=%N.pod" failed with "quadlet pod unit %N.pod does not exist" instead
of resolving to the pod named after the container's service.

Expand %N to the container's service name before the lookup, matching the
existing handling in GetContainerResourceName. Since %N is the service
name, setting ServiceName= lets the container and pod unit files use
different names.

Add e2e cases for both combinations: a pod named after the container
file, and a pod named after a ServiceName that differs from the file
name.

Fixes: podman-container-tools#29108
Signed-off-by: Salih Muhammed <root@lr0.org>
@larrasket larrasket force-pushed the quadlet-pod-specifier-expansion branch from ce3ad95 to 1fa03a1 Compare July 6, 2026 18:36
@larrasket

Copy link
Copy Markdown
Contributor Author

the Pod= note contradicted the RESOURCE NAMING. I reworded it.

@larrasket larrasket requested a review from ygalblum July 12, 2026 18:33
@ygalblum

Copy link
Copy Markdown
Contributor

@larrasket the doc build is still failing

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quadlet Pod option does not expand systemd specifiers

2 participants