Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an experimental 'DX-Next' mode, adding systemd services and Quadlet containers for Incus, Libvirt, Docker, and Containerd, along with a new Brewfile and a 'just' recipe for environment setup. Feedback highlights several critical issues in the systemd unit files and the setup script, including quoting errors in 'ExecStart' commands that prevent proper variable expansion, missing 'systemctl daemon-reload' calls after copying configuration files, and incorrect logic when generating 'sysusers.d' entries for multiple user groups.
| After=network.target local-fs.target dbus.service | ||
|
|
||
| [Service] | ||
| ExecStart=/bin/bash -c "env PATH="$PATH:/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin" /home/linuxbrew/.linuxbrew/bin/containerd" |
There was a problem hiding this comment.
The ExecStart command contains a quoting error where the double quotes around the PATH value prematurely terminate the outer string, which will lead to a systemd configuration error. Additionally, using exec is recommended so that the containerd process replaces the shell, allowing systemd to manage the daemon's lifecycle and signals correctly.
ExecStart=/bin/bash -c 'PATH="/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:$PATH" exec /home/linuxbrew/.linuxbrew/bin/containerd'
| Documentation=https://docs.docker.com/ | ||
|
|
||
| [Service] | ||
| ExecStart=/bin/bash -c "env PATH='$PATH:/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin' /home/linuxbrew/.linuxbrew/bin/dockerd" |
There was a problem hiding this comment.
The use of single quotes around $PATH prevents the shell from expanding the variable, and since env does not perform shell expansion, the literal string $PATH will be prepended to the environment variable. This will likely cause dockerd to fail when looking for system binaries. Using exec is also recommended for proper process management.
ExecStart=/bin/bash -c 'PATH="/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:$PATH" exec /home/linuxbrew/.linuxbrew/bin/dockerd'
| TasksMax=infinity | ||
|
|
||
| [Install] | ||
| WantedBy=default.target |
There was a problem hiding this comment.
|
This is awesome! Can we add an echo to the justfile to remind the user where to report bugs? Something like: "Help shape the future of DX, " so people know they can drop feedback here? Just like when it gets turned on they can see the link. |
|
Oh it's at the bottom never mind. :) Looks good to me, we can rev fast with this one. |
|
The new ujust can now be tested easily:
Then |
This comment was marked as outdated.
This comment was marked as outdated.
|
testing activity on this feature is happening here: https://discord.com/channels/1345470678408626206/1495876837165760622. |
|
Ok let's slide it in before F44 and call it alpha? A DX transition will take a long time so we can just run with both for a while. Thanks for working on this! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I now consider this feature-complete, everything I would want from a DX mode is implemented. It's now really modular, by default "Groups, Docker, Virt and DX-Tools" are selected, these alone are enough for what the majority need from the DX mode. For other users, Cockpit and Incus are also available, and ready to use in one click. New dx modules could be added later on; for exemple an AI module to mimic GDX Current dx modules status:
There might be some UX changes to do down the line, but this is good enough for a first alpha. I'm done, all that is left is a lot of testing! |
Relates to ublue-os/bluefin#3879
Finally, a first "working" version of DX without the need for a separate image!
Beware!, consider this preview of a pre-alpha, not everything is tested or approved by any maintainer!
This is only my proposition :
Where would everything move?
Other notable changes
Instead of using whatever weird workarround we are using for dx-groups, it would now be managed by systemd-sysusers.
Wall of shame
All other apps are ported!
Check dx-next.Brewfile!
[x] cockpit -> quadlet
[x] Incus -> quadlet
[ ] iotop
[ ] bcc
[ ] bpftrace
[ ] nicstat
[ ] osbuild-selinux
[ ] podman-machine
[ ] tiptop
New dx apps
Tracker/todo
libvirt-quadlet
[x] TPM support
[x] Some issues with /dev/kvm
[x] VirtGL
[ ] The OCI qemu image needs to be moved into the org (owned by my user for now)
incus-quadlet
[x] Add a simple incus setup
[x] The socket doesnt have the right permission on boot
[x] Fix kvm/dri
docker
[x] Needs to be moved to the main tap
ydotool
[x] Enable bottles
[ ] Fix linker
cockpit
[x] Add a simple cockpit setup