Quadlet - Add support for .pod units #20762

ygalblum · 2023-11-23T16:42:53Z

Add support for .pod unit files with only PodmanArgs, GlobalArgs, ContainersConfModule and PodName
Add support for linking .container units with .pod ones
Add e2e and system tests
Add to man page

Does this PR introduce a user-facing change?

Yes

Quadlet - Add support for .pod unit files

Resolves: #17687

ygalblum · 2023-11-23T16:44:54Z

@vrothberg PTAL.
This change required some additional mechanism in order to allow for the cyclic dependency between .pod and .container files. For this reason, I deliberately did not add support for any specific keys other than PodName to allow adding this feature more quickly and to allow for a simpler review.

ygalblum · 2023-11-23T16:47:57Z

@edsantiago can you please take a look at the test I added. My main problem is with the sleep I added. The problem is that since the services are linked with "Want" and "Before", I need to wait for the container service to transition from Activating to Active. But, AFAIK there's no way to do a retry on the assert.

vrothberg

Thanks for putting this together, @ygalblum ! Quite impressive how elegantly pods can be supported in Quadlet. Just some nits and a comment on the dependencies.

The units from generate systemd set dependencies between the units which I do not see here:

Wants=container-...
Before=container-...

and

BindsTo=pod-...
After=pod-...

docs/source/markdown/podman-systemd.unit.5.md

test/e2e/quadlet/name.pod

test/e2e/quadlet/podmanargs.pod

test/system/252-quadlet.bats

ygalblum · 2023-11-26T08:57:40Z

@vrothberg Regarding your comment about the dependencies.

The unit for the .pod file is linked to the corresponding units of the .container files here:
https://github.com/containers/podman/pull/20762/files#diff-a6aef00ed2763cf69a94f6c50af8ab3fd48532d01b2f1bf527e5545e984a1021R1282

The unit for the .container file is linked to the corresponding unit of the .pod file here:
https://github.com/containers/podman/pull/20762/files#diff-a6aef00ed2763cf69a94f6c50af8ab3fd48532d01b2f1bf527e5545e984a1021R1811

For example,
test.pod

[Pod]
PodName=test

first.container

[Container]
Image=registry.fedoraproject.org/fedora:38
Exec=sh -c "sleep inf"
Pod=test.pod

second.container

[Container]
Image=registry.fedoraproject.org/fedora:38
Exec=sh -c "sleep inf"
Pod=test.pod

Produce the following:

---first.service---
[X-Container]
Image=registry.fedoraproject.org/fedora:38
Exec=sh -c "sleep inf"
Pod=test.pod

[Unit]
SourcePath=/tmp/quadlet/first.container
RequiresMountsFor=%t/containers
BindsTo=test-pod.service
After=test-pod.service

[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
KillMode=mixed
ExecStop=/usr/bin/podman rm -v -f -i --cidfile=%t/%N.cid
ExecStopPost=-/usr/bin/podman rm -v -f -i --cidfile=%t/%N.cid
Delegate=yes
Type=notify
NotifyAccess=all
SyslogIdentifier=%N
ExecStart=/usr/bin/podman run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm --cgroups=split --sdnotify=conmon -d --pod-id-file %t/test-pod.pod-id registry.fedoraproject.org/fedora:38 sh -c "sleep inf"

---second.service---
[X-Container]
Image=registry.fedoraproject.org/fedora:38
Exec=sh -c "sleep inf"
Pod=test.pod

[Unit]
SourcePath=/tmp/quadlet/second.container
RequiresMountsFor=%t/containers
BindsTo=test-pod.service
After=test-pod.service

[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
KillMode=mixed
ExecStop=/usr/bin/podman rm -v -f -i --cidfile=%t/%N.cid
ExecStopPost=-/usr/bin/podman rm -v -f -i --cidfile=%t/%N.cid
Delegate=yes
Type=notify
NotifyAccess=all
SyslogIdentifier=%N
ExecStart=/usr/bin/podman run --name=systemd-%N --cidfile=%t/%N.cid --replace --rm --cgroups=split --sdnotify=conmon -d --pod-id-file %t/test-pod.pod-id registry.fedoraproject.org/fedora:38 sh -c "sleep inf"

---test-pod.service---
[X-Pod]
PodName=test

[Unit]
SourcePath=/tmp/quadlet/test.pod
RequiresMountsFor=%t/containers
Wants=first.service
Before=first.service
Wants=second.service
Before=second.service

[Service]
SyslogIdentifier=%N
ExecStart=/usr/bin/podman pod start --pod-id-file=%t/%N.pod-id
ExecStop=/usr/bin/podman pod stop --pod-id-file=%t/%N.pod-id --ignore --time=10
ExecStopPost=/usr/bin/podman pod rm --pod-id-file=%t/%N.pod-id --ignore --force
ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile=%t/%N.pid --pod-id-file=%t/%N.pod-id --exit-policy=stop --replace --name=test
Environment=PODMAN_SYSTEMD_UNIT=%n
Type=forking
Restart=on-failure
PIDFile=%t/%N.pid

docs/source/markdown/podman-systemd.unit.5.md

vrothberg · 2023-11-27T08:37:46Z

@vrothberg Regarding your comment about the dependencies.

The unit for the .pod file is linked to the corresponding units of the .container files here: https://github.com/containers/podman/pull/20762/files#diff-a6aef00ed2763cf69a94f6c50af8ab3fd48532d01b2f1bf527e5545e984a1021R1282

The unit for the .container file is linked to the corresponding unit of the .pod file here: https://github.com/containers/podman/pull/20762/files#diff-a6aef00ed2763cf69a94f6c50af8ab3fd48532d01b2f1bf527e5545e984a1021R1811

Thanks a lot, @ygalblum!

I should have paid more attention to the docs. I left another comment on that subject. The rest LGTM. Great work!

test/system/helpers.bash

vrothberg

LGTM

Just spotted two missing dots.
@rhatdan @edsantiago PTAL

docs/source/markdown/podman-systemd.unit.5.md

packit-as-a-service · 2023-11-28T11:43:15Z

Cockpit tests failed for commit 48a348a9d0fe849356f510f4b3b8c0ae84e6de62. @martinpitt, @jelly, @mvollmer please check.

edsantiago

LGTM, with one documentation request and one test-log-cleanup request. And could you please squash your commits? Thank you!

edsantiago · 2023-11-28T12:15:02Z

docs/source/markdown/podman-systemd.unit.5.md

@@ -72,7 +70,8 @@ Quadlet requires the use of cgroup v2, use `podman info --format {{.Host.Cgroups
 ### Service Type

 By default, the `Type` field of the `Service` section of the Quadlet file does not need to be set.
-Quadlet will set it to `notify` for `.container` and `.kube` files and to `oneshot` for `.volume`, `.network` and `.image` files.
+Quadlet will set it to `notify` for `.container` and `.kube` files,
+`forking` for `.pod` and `oneshot` for `.volume`, `.network` and `.image` files.


This takes a few retries for my brain to parse. I think 'files' and a comma would help:

... for .pod files, and ...

edsantiago · 2023-11-28T12:23:38Z

test/system/252-quadlet.bats

+
+    # Container should not exist
+    run_podman 1 container exists ${container_name}
+}


Please add run_podman rmi $(pause_image) to avoid nasty red warnings.

Add support for .pod unit files with only PodmanArgs, GlobalArgs, ContainersConfModule and PodName Add support for linking .container units with .pod ones Add e2e and system tests Add to man page Signed-off-by: Ygal Blum <[email protected]>

ygalblum · 2023-11-28T12:32:43Z

@edsantiago all done. Thanks

vrothberg

/lgtm
/hold

openshift-ci · 2023-11-28T12:33:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, vrothberg, ygalblum

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [edsantiago,vrothberg,ygalblum]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

edsantiago · 2023-11-28T14:39:30Z

/hold cancel

Thanks @ygalblum

gdmn · 2024-01-08T12:04:20Z

Should it be merged into 4.8 branch? I have 4.8.3 on Archlinux and pod support seems not to be present: quadlet-generator[50966]: converting "miniflux-db.container": unsupported key 'Pod' in group 'Container' in /home/..../.config/containers/systemd/miniflux-db.container

ygalblum · 2024-01-08T13:04:40Z

@gdmn Sorry, but the support for .pod files missed the cutoff date for V4.8 and new features are not merged into patch releases. It will be part of the next version

openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 23, 2023

ygalblum force-pushed the quadlet-pod branch from 1512813 to f170fba Compare November 23, 2023 16:51

vrothberg reviewed Nov 24, 2023

View reviewed changes

ygalblum force-pushed the quadlet-pod branch from f170fba to a293e86 Compare November 26, 2023 09:57

vrothberg reviewed Nov 27, 2023

View reviewed changes

docs/source/markdown/podman-systemd.unit.5.md Outdated Show resolved Hide resolved

vrothberg reviewed Nov 27, 2023

View reviewed changes

docs/source/markdown/podman-systemd.unit.5.md Outdated Show resolved Hide resolved

ygalblum force-pushed the quadlet-pod branch from a293e86 to 3c91be4 Compare November 27, 2023 12:58

edsantiago suggested changes Nov 27, 2023

View reviewed changes

test/system/helpers.bash Show resolved Hide resolved

test/system/helpers.bash Outdated Show resolved Hide resolved

test/system/helpers.bash Outdated Show resolved Hide resolved

ygalblum force-pushed the quadlet-pod branch from 3c91be4 to de01dfb Compare November 28, 2023 06:08

vrothberg reviewed Nov 28, 2023

View reviewed changes

docs/source/markdown/podman-systemd.unit.5.md Outdated Show resolved Hide resolved

docs/source/markdown/podman-systemd.unit.5.md Outdated Show resolved Hide resolved

edsantiago approved these changes Nov 28, 2023

View reviewed changes

edsantiago reviewed Nov 28, 2023

View reviewed changes

Quadlet - Add support for .pod units

6b2f481

Add support for .pod unit files with only PodmanArgs, GlobalArgs, ContainersConfModule and PodName Add support for linking .container units with .pod ones Add e2e and system tests Add to man page Signed-off-by: Ygal Blum <[email protected]>

ygalblum force-pushed the quadlet-pod branch from 48a348a to 6b2f481 Compare November 28, 2023 12:32

vrothberg approved these changes Nov 28, 2023

View reviewed changes

openshift-ci bot assigned vrothberg Nov 28, 2023

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Nov 28, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 28, 2023

openshift-merge-bot bot merged commit 545daed into containers:main Nov 28, 2023
93 checks passed

ygalblum deleted the quadlet-pod branch November 28, 2023 15:12

Luap99 mentioned this pull request Nov 30, 2023

Quadlet: Add support for --pod #20522

Closed

drewlsvern mentioned this pull request Nov 30, 2023

Quadlet: support for [Pod] Network #20851

Closed

jeancf mentioned this pull request Jan 26, 2024

unsupported key 'Pod' in group 'Container' with podman 4.9 #21371

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Apr 8, 2024

github-actions bot locked as resolved and limited conversation to collaborators Apr 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quadlet - Add support for .pod units #20762

Quadlet - Add support for .pod units #20762

ygalblum commented Nov 23, 2023

ygalblum commented Nov 23, 2023

ygalblum commented Nov 23, 2023

vrothberg left a comment

ygalblum commented Nov 26, 2023

vrothberg commented Nov 27, 2023

vrothberg left a comment

packit-as-a-service bot commented Nov 28, 2023

edsantiago left a comment

edsantiago Nov 28, 2023

ygalblum Nov 28, 2023

edsantiago Nov 28, 2023

ygalblum Nov 28, 2023

ygalblum commented Nov 28, 2023

vrothberg left a comment

openshift-ci bot commented Nov 28, 2023

edsantiago commented Nov 28, 2023

gdmn commented Jan 8, 2024

ygalblum commented Jan 8, 2024

Quadlet - Add support for .pod units #20762

Quadlet - Add support for .pod units #20762

Conversation

ygalblum commented Nov 23, 2023

Does this PR introduce a user-facing change?

ygalblum commented Nov 23, 2023

ygalblum commented Nov 23, 2023

vrothberg left a comment

Choose a reason for hiding this comment

ygalblum commented Nov 26, 2023

vrothberg commented Nov 27, 2023

vrothberg left a comment

Choose a reason for hiding this comment

packit-as-a-service bot commented Nov 28, 2023

edsantiago left a comment

Choose a reason for hiding this comment

edsantiago Nov 28, 2023

Choose a reason for hiding this comment

ygalblum Nov 28, 2023

Choose a reason for hiding this comment

edsantiago Nov 28, 2023

Choose a reason for hiding this comment

ygalblum Nov 28, 2023

Choose a reason for hiding this comment

ygalblum commented Nov 28, 2023

vrothberg left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 28, 2023

edsantiago commented Nov 28, 2023

gdmn commented Jan 8, 2024

ygalblum commented Jan 8, 2024