search

containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
23.69k stars 2.41k forks source link

Quadlet support symlinked subdirectory #23755

Closed pypingou closed 1 month ago

pypingou commented 2 months ago

Feature request description

Currently quadlet supports having, for example: /etc/containers/systemd be a symlink. It supports having files in that directory be symlinks (regardless of whether the directory itself is a symlink). It supports having subfolder in that directory (regardless of whether it is a symlink).

But quadlet does not support having a subfolder be a symlink.

For my use-case, I want to be able to atomically switch a from a set of quadlet files to another. The simplest way to do that is using symlinks. I can get it to work using the top level directory, but that means no one else but me can use that directory as otherwise I will override everything that's in it.

So in order to play nice with other apps/folks, I'd like to be able to group all my quadlets in say: /etc/containers/systemd/apps and have that be a symlink to another folder which I can update when needed to atomically move from a set of quadlet files to another.

Suggest potential solution

After discussing with @ygalblum it looks like the issue is that filePath.WalkDir doesn't follow symlink. This got fixed for the top-level folder. Could we use the same approach for the first level of subfolders?

Have you considered any alternatives?

A clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context or screenshots about the feature request here.

ygalblum commented 2 months ago

As @pypingou wrote, we discussed this earlier. I understand the requirement and see the benefit in supporting this request. Having said that, I know that this one of those slippery slops things. So, I wanted to clear it with others before implementing it. @Luap99 @rhatdan @alexlarsson @vrothberg @umohnani8 anyone else WDYT?

Luap99 commented 2 months ago

I see no problem with following symlinks, however at least for the root generator the following rule must be followed:

Non-essential file systems like /var/ and /home/ are mounted after generators have run

https://www.freedesktop.org/software/systemd/man/latest/systemd.generator.html

which means the symlink cannot point to volumes that are mounted later. Although I guess this is not an issue for rootless users.

ygalblum commented 2 months ago

Yes, this is true for all symlinks (and we've seen this before).

But, my question is this. Currently, the code uses filepath.WalkDir which does not follow symlinks in order to avoid infinite loops. From this discussion: https://github.com/golang/go/issues/49580 I see that this is quite the pitfall. So, I would like to limit the support for symlinks only one lever further, but then still use filepath.WalkDir. This is where my slippery slop starts (I can see people asking why we stopped at this level). WDYT?

Luap99 commented 2 months ago

Yeah restricting one level seems a bit odd for users. I would prefer to do it for all.

avoid infinite loops

That is certainly a good point, given the generator blocks the systemd boot process AFAIK that could be painful to deal with. I think we can avoid loops by storing each path in a map as key and check if we have seen the path before then skip it.

alexlarsson commented 2 months ago

Another possibility is to read a config file somewhere and use that to add additional quadlet sources. Then that could be used to point to a toplevel symlink to solve @pypingou s problem.

ygalblum commented 2 months ago

Thanks @alexlarsson I like this idea. Will it be OK to add a section for Quadlet in containers.conf?

pypingou commented 2 months ago

On Tue, Aug 27, 2024 at 12:09:48AM -0700, Alexander Larsson wrote:

Another possibility is to read a config file somewhere and use that to add additional quadlet sources. Then that could be used to point to a toplevel symlink to solve @.*** s problem.

Would it run from a single place then or a list of places? Say /etc/containers/systemd and /etc/myquadlets or just the later? (I'd argue for a list/both as otherwise it's kinda like the situation we have today)

ygalblum commented 2 months ago

@pypingou I would go for a list of paths that are in addition to the default ones. Then each path can actually be a symlink

pypingou commented 2 months ago

On Tue, Aug 27, 2024 at 07:18:27PM -0700, Ygal Blum wrote:

@pypingou I would go for a list of paths that are in addition to the default ones. Then each path can actually be a symlink

Perfect for me, thanks!

Luap99 commented 2 months ago

Will it be OK to add a section for Quadlet in containers.conf?

Containers.conf is not a good place because it will bloat the quadlet binary size massively due to of all the transitive imports there. My understanding is that this is not wanted for quadlet.

But do we really want a config file just to specific extra paths? If we allow symlinks this seems easier to use than having to configure another config file. But I also realize a config file could be used for more options in the future but so far I haven't seen a need for it.

alexlarsson commented 2 months ago

Yeah, I don't think we want to use the full podman config parser in the generator. What if we add a /etc/containers/systemd.d directory that can contain symlinks to directories.

Then we don't even need to parse any config file.

ygalblum commented 2 months ago

@alexlarsson do you mean that we'll have just one directory for which Quadlet will support symlinks on the next level? I don't mind that, just trying to limit the complaints and questions we'll get in the future. So, we need to be specific and document it.