Add support for `podman quadlet` #26330

flouthoc · 2025-06-09T21:45:31Z

Does this PR introduce a user-facing change?

Allow users to manage quadlets using a new podman quadlet command

openshift-ci · 2025-06-09T21:45:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flouthoc
Once this PR has been reviewed and has the lgtm label, please assign mheon for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

packit-as-a-service · 2025-06-09T22:20:37Z

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

Luap99 · 2025-06-11T11:21:51Z

bin/podman-remote    size= 52036192  delta= 37592
bin/podmansh         size=        6  delta=     0
bin/podman-testing   size= 56619600  delta=  4480
bin/quadlet          size=  3693018  delta= 68612

Why is the quadlet binary bloated by this? the quadlet binary size really needs to be small and avoid any unnecessary imports.

Luap99 · 2025-06-11T11:23:32Z

pkg/systemd/quadlet/unitdirs.go

+var (
+	Errorf func(string, ...interface{}) = logrus.Errorf
+	Debugf func(string, ...interface{}) = logrus.Debugf
+)


we should not use logrus in quadlet, that is causing the bloat i guess

ygalblum

Some comments/questions

cmd/podman/quadlet/list.go

ygalblum · 2025-06-12T18:10:30Z

cmd/podman/quadlet/remove.go

+	if err != nil {
+		return err
+	}
+
+	return nil


You can just return err

Suggested change

if err != nil {

return err

}

return nil

return err

cmd/quadlet/main.go

ygalblum · 2025-06-12T18:17:54Z

pkg/domain/infra/tunnel/quadlet.go

+	"github.com/containers/podman/v5/pkg/domain/entities"
+)
+
+var errNotImplemented = errors.New("not implemented for the remote Podman client")


Is it just for this PR, or is it not intended to be implemented for remote?

Not as of now, as discussed with @mheon , our priority is to get basic feature in, in future we can explore implementation of API for this. But overall I am still working on this PR by the time I undraft this PR i hope to resolve all your comments, thanks for the feedback.

I think we'll put it in Q3 prioritization to finish the commands by adding missing things (multi-file, better integration with bootc LBIs, etc)

pkg/libartifact/store/store.go

ygalblum · 2025-06-12T18:41:41Z

pkg/domain/infra/abi/quadlet.go

+	installDir := systemdquadlet.UnitDirAdmin
+	if rootless.IsRootless() {
+		// Install just for the user in question.
+		quadletRootlessDirs := systemdquadlet.GetUnitDirs(true)


I'm not sure systemdquadlet.GetUnitDirs acts the way you expect it to. The list of directories is sorted to the order in which the directories were added. So, IIUC the code here it will break some assumptions you have here:

/etc/containers/systemd/users might be the only one that fits your check. But, this directory is for root to put quadlets for all users. Yes, usually the write check will fail, but nothing guaranties it

Because you don't break from the loop when foundAdminDir = true, if there are directories under /etc/containers/systemd/users/$UID e.g. /etc/containers/systemd/users/aaa and /etc/containers/systemd/users/bbb, the Quadlets will be installed in /etc/containers/systemd/users/bbb. It might even go further in if of you have /etc/containers/systemd/users/bbb/z

Not sure this is all

This is fixed.

pkg/libartifact/store/store.go

axel7083 · 2025-06-13T07:37:20Z

cmd/podman/quadlet/install.go

+			}
+			return nil
+		},
+		Example: `podman quadlet install /path/to/myquadlet.container


question: what would be the recommend way of installing a kube quadlet? since the YAML would not be copied with this command?

Actually, this is an even bigger questions. Many .container units come with a configuration file stored locally and mounted into the container. For example, see: https://github.com/containers/appstore/tree/main/quadlet/minio-prometheus. prometheus.yml must be copied along with prometheus.container.

Furthermore, if you look at the examples, you can see that in many cases, more than one file is needed (units and others). Can the install command support installing a directory?

Furthermore, if you look at the examples, you can see that in many cases, more than one file is needed (units and others). Can the install command support installing a directory?

Yes it supports multiplefile but I think we need to start consuming entire directory as well.

Yes it supports multiplefile but I think we need to start consuming entire directory as well.

Yes, having to list all the files does not make sense for the user. Plus, as I wrote not all of them will be Quadlet files

If we copy resources, we may want to ensure they don't already exists, and have a --force flag, to allow overwriting existing files

I have added patch to install from directory in new commit.

If we copy resources, we may want to ensure they don't already exists, and have a --force flag, to allow overwriting existing files

Will add --force in a follow up PR once this gets merged in order to keep review simpler and not to added too much to matt's original patch.

flouthoc · 2025-06-19T19:09:22Z

This is ready for review @mheon @Luap99

ygalblum

One general issue that keep repeating in my comments is about non-Quadlet files. Not supporting them creates an issue

ygalblum · 2025-06-19T19:56:43Z

pkg/domain/infra/abi/quadlet.go

+			quadletExtension := getFileExtension(r)
+			if quadletExtension == "" {
+				quadletExtension = ".container"


Why? What if this is configuration file that also need to be copied?

ygalblum · 2025-06-19T20:00:10Z

pkg/domain/infra/abi/quadlet.go

+							continue
+						}
+						filePath := filepath.Join(toInstall, file.Name())
+						if !systemdquadlet.IsExtSupported(filePath) {


Won't this skip non-Qualet files? E.g. configuration files that are mounted into the container?

Added support for non-quadlet files.

ygalblum · 2025-06-19T20:00:58Z

pkg/domain/infra/abi/quadlet.go

+						continue
+					}
+					for _, file := range files {
+						if file.IsDir() {


Why skip subdirectories? Why are we not just copying the directory. I think we are overcomplicating this

ygalblum · 2025-06-19T20:05:40Z

pkg/domain/infra/abi/quadlet.go

+	// really has to parse everything), but we should be able to output in a machine-readable format
+	// (JSON, maybe?) so we can easily associated error to quadlet here, and return better
+	// results.
+	var validateErr error


Not sure why we are doing this.
But if we are, first, I think the user should be aware of it and also maybe have a way to skip the check. Second, doesn't it make more sense to prepare everything in a temporary directory, run the dry-run on it and only if the dry-run passes copy to the real target?

Does that work if the quadlet has dependencies on already-installed quadlets?

I have removed this check for now, already too much in this PR.

ygalblum · 2025-06-19T20:13:45Z

pkg/domain/infra/abi/quadlet.go

+			continue
+		}
+
+		report.Status = fmt.Sprintf("%s/%s", unitStatus.ActiveState, unitStatus.SubState)


Move this line to an else section of the previous if. This way you can remove the continue and combine the duplicated two lines

ygalblum · 2025-06-19T20:17:32Z

pkg/domain/infra/abi/quadlet.go

+	return string(contents), nil
+}
+
+func (ic *ContainerEngine) QuadletRemove(ctx context.Context, quadlets []string, options entities.QuadletRemoveOptions) (*entities.QuadletRemoveReport, error) {


Not sure I understand how the user can remove non-Quadlet files

Added this feature and also added test.

ygalblum · 2025-06-19T20:22:24Z

pkg/domain/infra/abi/quadlet.go

+	}
+
+	// Reload systemd, if necessary/requested.
+	if needReload {


First, needReload is not set according to QuadletRemoveOptions so if the user requests it it won't run. Second, the doc does not mention the reload will happen regardless to what they asked.

ygalblum · 2025-06-19T20:24:44Z

test/system/252-quadlet.bats

@@ -1865,4 +1865,164 @@ EOF

    run_podman rmi -i $image_tag
 }
+


Install and remove tests should also include non-Quadlet files that are mounted into the container.

Done, new tests below have these.

ygalblum · 2025-06-19T20:25:28Z

test/system/252-quadlet.bats

@@ -1865,4 +1865,164 @@ EOF

    run_podman rmi -i $image_tag
 }
+
+@test "quadlet verb - install, list, rm" {


Shouldn't these tests be in a separate file? quadlet and podman quadlet are not the same

I thought quadlet and quadlet verb are togather part of similar big feature, since helpers functions are being shared therefore I added them in same file. These are only few new tests and I have added quadlet verb as suffix to differentiate them. If these extend to more test I will move them to a new file.

I rather they be in a separate file. It's relevant when running the tests locally. I don't see a reason to run the quadlet verb tests when making changes to quadlet

flouthoc · 2025-06-20T16:08:13Z

One general issue that keep repeating in my comments is about non-Quadlet files. Not supporting them creates an issue

We don't support non-quadlet files in this current PR, however if we want to support non-quadlet files, we must come up with a way so podman can associate non-quadlet files to a quadlet file so whenever a remove is done we can remove non-quadlet files along with quadlet files.

I propose a method like this podman quadlet install myquadlet.container:/associated/non-quadlets/ifany,/second/non-quadlet-path , and podman can maintain a list of all non quadlet files in a hidden file called .myquadlet.container.assets to store list of non-quadlet files associated to a quadlet and can remove those if needed.

@ygalblum @Luap99 @mheon WDYT ?

mheon · 2025-06-20T16:13:27Z

Using a hidden file SGTM, but that user interface seems pretty bad. Should we restrict podman quadlet install to a single Quadlet at a time, the first argument is the quadlet, all subsequent arguments are extra files to install with the quadlet?

flouthoc · 2025-06-20T16:14:42Z

Using a hidden file SGTM, but that user interface seems pretty bad. Should we restrict podman quadlet install to a single Quadlet at a time, the first argument is the quadlet, all subsequent arguments are extra files to install with the quadlet?

That sounds good to me and much cleaner. What do others think @ygalblum @Luap99 ?

ygalblum · 2025-06-20T16:25:41Z

Installing a single Quadlet file will generate errors in daemon reload if other units are missing. For example, if you install the .container file that depends on a .network file that wasn't yet installed, the processing of the former will fail. Then you start mandating not only multiple calls, but also that the call order.
Maybe a simpler approach is to support either single units or directories.

flouthoc · 2025-06-20T16:39:08Z

Installing a single Quadlet file will generate errors in daemon reload if other units are missing. For example, if you install the .container file that depends on a .network file that wasn't yet installed, the processing of the former will fail. Then you start mandating not only multiple calls, but also that the call order. Maybe a simpler approach is to support either single units or directories.

Problem with directory is, if a directory contains two quadlet files and contains different non-quadlet files, then how will podman know which non-quadlet file belongs which quadlet file ?

Or in case of a directory we can just associate all non-quadlet file to one quadlet file in directory and assume users to be aware that mutiple quadlet files should be orchestrated or removed together.

ygalblum · 2025-06-20T16:47:13Z

Or in case of a directory we can just associate all non-quadlet file to one quadlet file in directory and assume users to be aware that mutiple quadlet files should be orchestrated or removed together.

Maybe when it's a directory, the identifier shouldn't be at a Quadlet unit level, but an application or deployment (find better name) level.

flouthoc · 2025-06-20T17:03:25Z

Or in case of a directory we can just associate all non-quadlet file to one quadlet file in directory and assume users to be aware that mutiple quadlet files should be orchestrated or removed together.

Maybe when it's a directory, the identifier shouldn't be at a Quadlet unit level, but an application or deployment (find better name) level.

I think quadlet list should still show all quadlets independentaly but with another column if they belong to common app and deleting a quadlet from app, should remove entire app ?

So it both should remove app

quadlet remove unit --> if belongs to app, removes entire app
quadlet remove app --> also removes app.

This adds `podman quadlet list`, `podman quadlet install`, `podman quadlet rm` and `podman quadlet print`. Signed-off-by: Matt Heon <[email protected]> Co-authored-by: flouthoc <[email protected]> Signed-off-by: flouthoc <[email protected]>

flouthoc · 2025-06-23T21:35:43Z

@mheon @Luap99 @ygalblum This is ready for review.

ygalblum

I still need to review the abi. But, I wanted to put in these comments

ygalblum · 2025-06-25T19:49:26Z

cmd/podman/quadlet/list.go

+	var err error
+	switch {
+	case cmd.Flag("format").Changed:
+		rpt, err = rpt.Parse(report.OriginUser, format)
+	default:
+		rpt, err = rpt.Parse(report.OriginPodman, format)
+	}


Suggested change

var err error

switch {

case cmd.Flag("format").Changed:

rpt, err = rpt.Parse(report.OriginUser, format)

default:

rpt, err = rpt.Parse(report.OriginPodman, format)

}

origin := report.OriginPodman

if cmd.Flag("format").Changed {

origin = report.OriginUser

}

rpt, err := rpt.Parse(origin, format)

ygalblum · 2025-06-25T19:52:23Z

docs/source/markdown/podman-quadlet-rm.1.md

+Remove one or more installed Quadlets from the current user. Following command also takes application name
+as input and removes all the Quadlets which belongs to that specific application.
+
+Note: If quadlet belongs to an application then removing that specific quadlet will remove entire application.


It's not clear from reading this and the install page what defines an application

ygalblum · 2025-06-25T19:55:22Z

test/system/252-quadlet.bats

@@ -1865,4 +1865,164 @@ EOF

    run_podman rmi -i $image_tag
 }
+
+@test "quadlet verb - install, list, rm" {


I rather they be in a separate file. It's relevant when running the tests locally. I don't see a reason to run the quadlet verb tests when making changes to quadlet

ygalblum · 2025-06-25T19:57:04Z

test/system/252-quadlet.bats

+
+    # Test quadlet install
+    run_podman quadlet install $quadlet_file
+    assert $status -eq 0 "quadlet install should succeed"


AFAIK run_podman already asserts the value of status. If first parameter is numeric, it will treat it as the expected value and start the command from the second parameter. Otherwise, it will expect 0.
This is relevant throughout this test

ygalblum · 2025-06-25T20:04:49Z

test/system/252-quadlet.bats

+    assert "$output" =~ "Mount=type=bind,source=./test.txt,destination=/test.txt:z" "print should show volume mount configuration"
+
+    # Test quadlet rm
+    run_podman quadlet rm mount-test.container


Is this call expected to also remove the "$install_dir/test.txt"? If yes, it should be verified. If not, aren't we leaving leftovers?

ygalblum · 2025-06-25T20:08:05Z

test/system/252-quadlet.bats

+LogDriver=passthrough
+EOF
+    # Clean all quadlets
+    run_podman quadlet rm --all -f


If I run this locally, it will remove all my Quadlet files.

ygalblum · 2025-06-25T20:09:24Z

test/system/252-quadlet.bats

+# bats test_tags=distro-integration
+@test "quadlet verb - install multiple quadlets from directory and remove by application" {
+    # Create a temporary directory for multiple quadlet files
+    local app_name="test-app-$(random_string)"


Use safe_name instead of random_string (throughout)

ygalblum · 2025-06-25T20:10:23Z

test/system/252-quadlet.bats

+# bats test_tags=distro-integration
+@test "quadlet verb - install multiple files from directory" {
+    # Create a directory for multiple quadlet files
+    local quadlet_dir=$PODMAN_TMPDIR/quadlet-multi


Always use safe_name to allow parallelism

mheon · 2025-06-25T21:05:18Z

docs/source/markdown/podman-quadlet-install.1.md

+
+## DESCRIPTION
+
+Install a Quadlet file or an application (which may include multiple Quadlet files) for the current user. You can specify Quadlet files as local files or web URLs.


Need more details on how applications work here - it's not very clear

mheon · 2025-06-25T21:05:41Z

docs/source/markdown/podman-quadlet-list.1.md

+
+| Filter     | Description                                                                                      |
+|------------|--------------------------------------------------------------------------------------------------|
+| name       | Filter by quadlet name.                                                                          |


Can you add a TODO somewhere to add an Application filter to this?

mheon · 2025-06-25T21:07:17Z

pkg/domain/infra/abi/quadlet.go

+
+// AppendStringToFile appends the given text to the specified file.
+// If the file does not exist, it will be created with 0644 permissions.
+func appendStringToFile(filePath, text string) error {


Feels like this should be in pkg/util?

mheon · 2025-06-25T21:08:30Z

pkg/domain/infra/abi/quadlet.go

+	return err
+}
+
+// deleteFilesFromAsset reads .<name>.asset, deletes listed files, then deletes the asset file


Since this deletes everything it feels like this should be named deleteAsset instead

mheon · 2025-06-25T21:12:10Z

docs/source/markdown/podman-quadlet.1.md

+| install | [podman-quadlet-install(1)](podman-quadlet-install.1.md)   | Install a quadlet file or quadlet application                |
+| list    | [podman-quadlet-list(1)](podman-quadlet-list.1.md)         | List installed quadlets                                      |
+| print   | [podman-quadlet-print(1)](podman-quadlet-print.1.md)       | Display the contents of a quadlet                            |
+| rm      | [podman-quadlet-rm(1)](podman-quadlet-rm.1.md)             | Removes an installed quadlet                                 |


I feel like at some point we need a quadlet inspect that prints more details. Particularly about the app the quadlet is part of. But that is followon work.

mheon · 2025-06-25T21:14:24Z

pkg/domain/infra/abi/quadlet.go

+		quadlet := quadlets[i]
+		quadletPath, err := getQuadletByName(quadlet)
+		if options.All {
+			quadletPath = quadlet


This doesn't seem right. Why are we even doing getQuadletByName when options.All is set?

openshift-ci bot added the release-note label Jun 9, 2025

flouthoc marked this pull request as draft June 9, 2025 21:45

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 9, 2025

flouthoc added the bloat_approved Approve a PR in which binary file size grows by over 50k label Jun 10, 2025

flouthoc force-pushed the quadlet-work branch from d83912a to b9a7564 Compare June 10, 2025 19:40

Luap99 reviewed Jun 11, 2025

View reviewed changes

flouthoc force-pushed the quadlet-work branch 6 times, most recently from 9cee402 to c7c1da9 Compare June 12, 2025 18:12

ygalblum reviewed Jun 12, 2025

View reviewed changes

pkg/libartifact/store/store.go Outdated Show resolved Hide resolved

flouthoc force-pushed the quadlet-work branch from c7c1da9 to 2e1b214 Compare June 12, 2025 18:29

ygalblum reviewed Jun 12, 2025

View reviewed changes

flouthoc added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jun 12, 2025

flouthoc marked this pull request as ready for review June 12, 2025 20:30

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2025

flouthoc marked this pull request as draft June 12, 2025 20:30

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2025

flouthoc force-pushed the quadlet-work branch 4 times, most recently from a2b08f8 to ae01986 Compare June 13, 2025 00:21

axel7083 reviewed Jun 13, 2025

View reviewed changes

flouthoc requested a review from mheon June 19, 2025 19:08

ygalblum reviewed Jun 19, 2025

View reviewed changes

flouthoc force-pushed the quadlet-work branch 9 times, most recently from e4d2fb5 to 690c3ae Compare June 23, 2025 20:42

flouthoc requested a review from ygalblum June 23, 2025 21:19

flouthoc force-pushed the quadlet-work branch from 690c3ae to d5eb9ee Compare June 23, 2025 21:35

ygalblum reviewed Jun 25, 2025

View reviewed changes

mheon reviewed Jun 25, 2025

View reviewed changes


		## DESCRIPTION

		Install a Quadlet file or an application (which may include multiple Quadlet files) for the current user. You can specify Quadlet files as local files or web URLs.

Add support for podman quadlet #26330

Are you sure you want to change the base?

Add support for podman quadlet #26330

Conversation

flouthoc commented Jun 9, 2025

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci bot commented Jun 9, 2025

Uh oh!

packit-as-a-service bot commented Jun 9, 2025

Uh oh!

Luap99 commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ygalblum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flouthoc commented Jun 19, 2025

Uh oh!

ygalblum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Add support for `podman quadlet` #26330

Add support for `podman quadlet` #26330

flouthoc Jun 23, 2025 •

edited

Loading

flouthoc commented Jun 20, 2025 •

edited

Loading

flouthoc commented Jun 20, 2025 •

edited

Loading