Handle uninstantiated template quadlets

[simonbrauner]

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?

`podman quadlet list` and `podman quadlet rm` works with uninstantiated templates

[simonbrauner]

The issue proposes to disregard template files, so this would be a quick fix.

diff --git a/pkg/domain/infra/abi/quadlet.go b/pkg/domain/infra/abi/quadlet.go
index 34652ebbc4..c8a4dc24ed 100644
--- a/pkg/domain/infra/abi/quadlet.go
+++ b/pkg/domain/infra/abi/quadlet.go
@@ -708,6 +708,12 @@ func (ic *ContainerEngine) QuadletList(ctx context.Context, options entities.Qua
                        reports = append(reports, &report)
                        continue
                }
+
+               if strings.Contains(serviceName, "@.") {
+                       // Template file, skipping
+                       continue
+               }
+
                partialReports[serviceName] = report
                allServiceNames = append(allServiceNames, serviceName)
        }

I will try to use a different way to query systemd, so that templates could also be included in the listing.

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[packit-as-a-service]

tmt tests failed for commit 36d7f3c. @lsm5, @psss, @thrix please check.

[danishprakash]

There can still be running instances of the templates removed, right? I wonder if we should distinguish between the removal of concrete services and templated ones. Because while concrete services are stopped before they are removed, it's not the same for template services, and iiuc the user can't really tell the difference while running podman quadlet rm <concrete_svc> <template_svc>, unless I'm missing something.

[simonbrauner]

Yes, that is right. I documented it that way

When the argument is uninstantiated template quadlet, this command removes the
template quadlet file (e.g. `templateName@.container`) and the generated systemd
template unit (e.g. `templateName@.service`, unless **--reload-systemd** is set to `false`).
Instances of the systemd template unit (e.g. `templateName@instanceName.service`)
may persist, and can be removed with **systemctl(1)**.

It seemed reasonable to do it this way for the minimal fix of the issue, but there could be a follow up which elaborates on this.

On one hand, perhaps it would be nice to help users to manage instances, but there are plenty of ways how instances can be created:

• from a file which has instance name already - podman quadlet install templateName@instanceName.container
• by creating a symlink to template name in ~/.config/containers/systemd/
• DefaultInstance
• systemctl --user start
• Systemd dependency (Wants key for example)

so it could be messy to cover all the cases.

On the other hand, instances of templates could be even considered out of scope, as quadlets are about generating services, and what the services are used for is responsibility of systemd. So from this point of view - my fix is consistent, it removes the .container file and the .service file generated from it for both concrete units and templates.

[simonbrauner]

This changed. For templates, rm without --force fails if there are running instances. With --force the instances are stopped.

[packit-as-a-service]

tmt tests failed for commit c3ecb25. @lsm5, @psss, @thrix please check.

[simonbrauner]

/packit rebuild-failed

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[simonbrauner]

/packit rebuild-failed

[packit-as-a-service]

tmt tests failed for commit c3ecb25. @lsm5, @psss, @thrix please check.

[psss]

tmt tests failed for commit c3ecb25. @lsm5, @psss, @thrix please check.

Should be fixed once rebased on the latest main.

[simonbrauner]

Removing all instances when template is removed could be tricky as sleep@1.service is an instance of template:

sleep@1.service - A templated sleepy container
   Loaded: loaded (/home/sbrauner/.config/containers/systemd/sleep@.container; generated)

but sleep@2.service is just a service which supports the %i substitution but otherwise works just as regular concrete service and is not related to sleep@.container at all:

sleep@2.service - A templated sleepy container
   Loaded: loaded (/home/sbrauner/.config/containers/systemd/sleep@2.container; generated)

https://0pointer.de/blog/projects/instances.html

I'll try to solve it by looking at the SourcePath property in systemd.

[packit-as-a-service]

tmt tests failed for commit b25bb53. @lsm5, @psss, @thrix please check.

[simonbrauner]

/packit rebuild-failed

[Honny1]

Just non-blocking comments.

[packit-as-a-service]

tmt tests failed for commit e36f1d3. @lsm5, @psss, @thrix please check.

[TomSweeneyRedHat]

LGTM
some test unhappiness at the moment though

[Honny1]

LGTM

[TomSweeneyRedHat]

@simonbrauner any thoughts on the test issues?

[simonbrauner]

The labels machine, governance and CI are there from a rebase mistake. The original failures were fixed by rebase, there is only one failure now, which does not seem quadlet-related.
@TomSweeneyRedHat @Honny1