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.

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

[quadlet] add back support for directory symlinks in the podman unit search paths #20504

Closed
xduugu opened this issue Oct 26, 2023 · 5 comments · Fixed by #20688
Closed

[quadlet] add back support for directory symlinks in the podman unit search paths #20504

xduugu opened this issue Oct 26, 2023 · 5 comments · Fixed by #20688
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. quadlet

Comments

@xduugu
Copy link
Contributor

xduugu commented Oct 26, 2023

Issue Description

I had symlinked ~/.config/containers/systemd/ to the directory ~/containers/quadlet which worked fine in podman version v4.6.2 and before. My setup broke with podman v4.7.0, because of commit 413552e that added support for unit files in subdirectories.

podman/cmd/quadlet/main.go

Lines 148 to 156 in c3fae01

func appendSubPaths(dirs []string, path string, isUserFlag bool, filterPtr func(string, bool) bool) []string {
err := filepath.WalkDir(path, func(_path string, info os.DirEntry, err error) error {
if info == nil || info.IsDir() {
if filterPtr == nil || filterPtr(_path, isUserFlag) {
dirs = append(dirs, _path)
}
}
return err
})

filepath.WalkDir explicitly does not support directory symlinks:

WalkDir does not follow symbolic links.

Directory symlinks inside of a directory ~/.config/containers/systemd/ also don't work because of this (and probably the info.isDir() check).

Steps to reproduce the issue

Steps to reproduce the issue

  1. create a symlink from ~/.config/containers/systemd/ to a directory that contains quadlet unit files
  2. run quadlet

Describe the results you received

Quadlet does not find the unit files anymore.

Describe the results you expected

Quadlet finds the unit files.

podman info output

host:
  arch: amd64
  buildahVersion: 1.33.0-dev
  cgroupControllers:
  - cpu
  - io
  - memory
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.1.7-3.fc39.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.7, commit: '
  cpuUtilization:
    idlePercent: 97.66
    systemPercent: 0.83
    userPercent: 1.5
  cpus: 16
  databaseBackend: boltdb
  distribution:
    distribution: fedora
    variant: workstation
    version: "39"
  eventLogger: journald
  freeLocks: 2048
  hostname: fedora
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 65536
  kernel: 6.5.6-300.fc39.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 17862057984
  memTotal: 32817713152
  networkBackend: netavark
  networkBackendInfo:
    backend: netavark
    dns:
      package: aardvark-dns-1.7.0-2.fc39.x86_64
      path: /usr/libexec/podman/aardvark-dns
      version: aardvark-dns 1.7.0
    package: netavark-1.7.0-2.fc39.x86_64
    path: /usr/libexec/podman/netavark
    version: netavark 1.7.0
  ociRuntime:
    name: crun
    package: crun-1.9-1.fc39.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.9
      commit: a538ac4ea1ff319bcfe2bf81cb5c6f687e2dc9d3
      rundir: /run/user/1000/crun
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +LIBKRUN +WASM:wasmedge +YAJL
  os: linux
  pasta:
    executable: /usr/bin/pasta
    package: passt-0^20230908.g05627dc-1.fc39.x86_64
    version: |
      pasta 0^20230908.g05627dc-1.fc39.x86_64
      Copyright Red Hat
      GNU General Public License, version 2 or later
        <https://www.gnu.org/licenses/old-licenses/gpl-2.0.html>
      This is free software: you are free to change and redistribute it.
      There is NO WARRANTY, to the extent permitted by law.
  remoteSocket:
    exists: false
    path: /run/user/1000/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: false
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.2.1-1.fc39.x86_64
    version: |-
      slirp4netns version 1.2.1
      commit: 09e31e92fa3d2a1d3ca261adaeb012c8d75a8194
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.3
  swapFree: 8589930496
  swapTotal: 8589930496
  uptime: 1h 13m 17.00s (Approximately 0.04 days)
  variant: ""
plugins:
  authorization: null
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  - ipvlan
  volume:
  - local
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
  - quay.io
store:
  configFile: /home/xduugu/.config/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /home/xduugu/.local/share/containers/storage
  graphRootAllocated: 998380109824
  graphRootUsed: 335406395392
  graphStatus:
    Backing Filesystem: btrfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Supports shifting: "false"
    Supports volatile: "true"
    Using metacopy: "false"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 4
  runRoot: /run/user/1000/containers
  transientStore: false
  volumePath: /home/xduugu/.local/share/containers/storage/volumes
version:
  APIVersion: 4.8.0-dev
  Built: 1698350044
  BuiltTime: Thu Oct 26 21:54:04 2023
  GitCommit: e5101b28deff158feb8cd69a25c082a441c07c9a
  GoVersion: go1.21.3
  Os: linux
  OsArch: linux/amd64
  Version: 4.8.0-dev

Podman in a container

No

Privileged Or Rootless

None

Upstream Latest Release

Yes

Additional environment details

Additional environment details

Additional information

Instead of adding back support for directory symlinks, it would also help if additional quadlet unit directories could be set via configuration file.

@xduugu xduugu added the kind/bug Categorizes issue or PR as related to a bug. label Oct 26, 2023
@ygalblum
Copy link
Collaborator

ygalblum commented Nov 8, 2023

Hi @xduugu thanks for bringing this up.

The problem is that once Quadlet supports traversing a directory tree, following symlinks may lead to loops (see the reason why it is not supported in WalkDir: golang/go#4759).

I do like the idea of adding directories via the configuration file. @rhatdan @vrothberg WDYT?

@vrothberg
Copy link
Member

We can't really use containers.conf as it would binary-bloat Quadlet. And I do not like introducing "yet another" configuration file just for Quadlet.

I think we should restore the previous behavior as I consider a regression. Couldn't resolve the root directories before recursing/walking it? In this case, we could first resolve ~/.config/containers/systemd/ which resolves to ~/containers/quadlet and then walk ~/containers/quadlet for sub-directories.

If this works and we agree to move forward, the PR fixing it could also make the behavior explicit in the docs.

@ygalblum
Copy link
Collaborator

ygalblum commented Nov 8, 2023

If the symlink resolution is only for the root directory, then I guess we can handle it and it will not cause any loops (since it is done only once). Is this the only case we want to support?

For this case, I think we can use: https://pkg.go.dev/io/fs#WalkDir

@xduugu
Copy link
Contributor Author

xduugu commented Nov 13, 2023

At least for my use case, root directory symlink support would be sufficient. However, I would somehow prefer smylinks at the first level below the root directory, but that is of course a little bit more complicated to implement.

The QUADLET_UNIT_DIRS environment variable, which is intended for debugging purpose, would actually fulfill my need, but I don't know how to set the environment variable when quadlet is not executed from command line. Of course, it would be better, if it would not replace the unit search path, only adds additional directories to search for unit files.

@xduugu
Copy link
Contributor Author

xduugu commented Nov 15, 2023

Thanks a lot for the PR, @ygalblum!

@github-actions 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 Feb 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. quadlet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants