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.

Sign up for GitHub

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

Jump to bottom

Support templates in quadlet #21068

Merged
merged 6 commits into from
Feb 2, 2024
Merged

Support templates in quadlet #21068

merged 6 commits into from
Feb 2, 2024

Conversation

alexlarsson
Copy link
Contributor

As discussed in #17744 we would like to support template systemd unit, and also have a good way to enable them.

This PR makes the service files work, and adds support for [Install] to handle enabling default instances as well as individual instances by using symlinks.

* support template units in quadlet

@packit-as-a-service Packit-as-a-Service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@alexlarsson
Copy link
Contributor Author

No tests added yet, I'm off on PTO, back after new year and will look at that then.

ygalblum
ygalblum reviewed Dec 21, 2023
View reviewed changes
Copy link
Contributor

@ygalblum ygalblum left a comment

Choose a reason for hiding this comment

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

I am not sure I understood who to use the templates. So, an example is probably needed in the man page.

docs/source/markdown/podman-systemd.unit.5.md Show resolved Hide resolved
@alexlarsson
Copy link
Contributor Author

An example is something like:

[email protected]:

[Unit]
Description=A template sleep container

[Container]
Image=quay.io/fedora/fedora
Exec=sleep %i

[Service]
# Restart service when sleep finishes
Restart=always

[Install]
WantedBy=multi-user.target
DefaultInstance=100

If this is installed, then on boot there should be a [email protected] running that sleeps 100 seconds. You can then do something like systemctl start [email protected] to start another instance. Or some other service can say [email protected].

In addition, if you do ln -s [email protected] [email protected] you will also have a 10 second sleep running at boot. And, if you want that particular instance to be running with another image, you can make [email protected]/10-image.conf:

[Container]
Image=quay.io/centos/centos

@rhatdan
Copy link
Member

rhatdan commented Dec 21, 2023

@ygalblum do you think you could drive this to completion, while we are sipping Egg Nog?

@github-actions GitHub Actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@alexlarsson alexlarsson force-pushed the quadlet-templates branch 2 times, most recently from 3998adf to 6100816 Compare January 23, 2024 16:09
@github-actions github-actions bot removed the stale-pr label Jan 24, 2024
ygalblum
ygalblum approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@ygalblum ygalblum left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2024
@alexlarsson
Copy link
Contributor Author

Unclear to me why the podman machine tests are failing.

@ygalblum
Copy link
Contributor

Unclear to me why the podman machine tests are failing.

I think they're failing for everyone. For example #21180

@TomSweeneyRedHat
Copy link
Member

@mheon are the sqlite tests happy now? Does @alexlarsson need to rebase?

@mheon
Copy link
Member

mheon commented Jan 25, 2024

HyperV is a flake. Mac is an expected failure.

@alexlarsson
quadlet: Don't put @ in container names for templated units
2df994b 
This is not supported by podman, so we make "foo@bar" into "foo_bar".

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
systemd.parser: Add GetTemplateParts()
7e1942e 
This helper splits out a templated filename into the base template and
the instance name. This will be used later.

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
quadlet: Support [Install] for templated units
bb6dec4 
For a base template like `[email protected]` the WantedBy and RequiredBy
keys does nothing. However, if a DefaultInstance= key is specified
that is used by default.

However, even if the DefaultInstance= is not given, the Install
section is still useful, because you can instantiate the generic
template by making a symlink for it, and that symlink will then
pick up the instance id.

So, for example, this [email protected] will not enable anything
on boot.
```
[Container]
Image=foo
Exec=sleep 100

[Install]
WantedBy=other.container
```

But if you have a symlink '[email protected]` -> `[email protected]'
then the `foo@instance` service will be marked as wanted by `other`.

In addition, even if the main template doesn't have an Install
section, you can instantiate it with a symlink like above, and then
enabling it using a dropin file like
[email protected]/install.conf containing:
```
[Install]
WantedBy=other.container
```

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
quadlet: When loading dropin files for foo@instance, also load those …
01dccba 
…for foo@.

This is how systemd works for templates, and it allows us lots of
flexibilities.

Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
quadlet: Add documentation about template use to manpage
dc94a10 
Signed-off-by: Alexander Larsson <[email protected]>
@alexlarsson
quadlet: Add tests for templates
cd5982e 
Signed-off-by: Alexander Larsson <[email protected]>
vrothberg
vrothberg approved these changes Feb 2, 2024
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm

Really great feature!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Feb 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexlarsson, 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:

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

@openshift-merge-bot openshift-merge-bot bot merged commit 5e64d4f into containers:main Feb 2, 2024
91 of 92 checks passed
@Luap99 Luap99 mentioned this pull request Apr 4, 2024
Closed
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label May 3, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@eriksjolund eriksjolund eriksjolund left review comments

@vrothberg vrothberg vrothberg approved these changes

@ygalblum ygalblum ygalblum approved these changes

Assignees

@vrothberg vrothberg

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@alexlarsson @rhatdan @ygalblum @TomSweeneyRedHat @mheon @eriksjolund @vrothberg