[Bug 2260492] New: Review Request: incus - Powerful system container and virtual machine manager
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Bug ID: 2260492 Summary: Review Request: incus - Powerful system container and virtual machine manager Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: ngompa13@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://ngompa.fedorapeople.org/for-review/incus.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/incus-0.5-1.fc39.src.rpm
Description: Container hypervisor based on LXC Incus offers a REST API to remotely manage containers over the network, using an image based work-flow and with support for live migration.
Fedora Account System Username: ngompa
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Whiteboard| |NotReady Doc Type|--- |If docs needed, set a value
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6963272 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Neil Hanlon neil@shrug.pw changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? Assignee|nobody@fedoraproject.org |neil@shrug.pw CC| |neil@shrug.pw
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #2 from Neal Gompa ngompa13@gmail.com --- [fedora-review-service-build]
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #3 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2011019 --> https://bugzilla.redhat.com/attachment.cgi?id=2011019&action=edit The .spec file difference from Copr build 6963272 to 6967304
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #4 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6967304 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #5 from Neal Gompa ngompa13@gmail.com --- New version.
Spec URL: https://ngompa.fedorapeople.org/for-review/incus.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/incus-0.6-1.fc39.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Whiteboard|NotReady |
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #6 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7092290 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Xavier Bachelot xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |xavier@bachelot.org Status|NEW |ASSIGNED
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #7 from Neil Hanlon neil@shrug.pw --- Skipping the formality of entire review template as there are no glaring errors or problems. That said, a few notes and comments for posterity, mostly/exclusively from rpmlint.
Expected: these are set purposefully.
``` incus.x86_64: E: non-standard-dir-perm /var/cache/incus 700 incus.x86_64: E: non-standard-dir-perm /var/lib/incus 711 incus.x86_64: E: non-standard-dir-perm /var/log/incus 700 ```
These seem possibly problematic, but I admit I am not sure if the selinux one is expected, as above.
``` incus-selinux.noarch: E: non-readable /var/lib/selinux/targeted/active/modules/200/incus 0 incus.x86_64: W: log-files-without-logrotate ['/var/log/incus'] incus.x86_64: W: post-without-tmpfile-creation /usr/lib/tmpfiles.d/incus.conf ```
Expected -- comes from macros and are standard:
``` incus-selinux.noarch: W: dangerous-command-in-%pre cp incus-selinux.noarch: W: dangerous-command-in-%postun rm incus-selinux.noarch: W: dangerous-command-in-%posttrans rm incus-selinux.noarch: W: dangerous-command-in-%post rm ```
The rest of the review looks great. Aside from these couple items/questions, this is good to go, from my perspective.
If you want to throw some answers back at me and/or address issues if you think they are in need of fixing, I'm happy to throw an Approved stamp on this.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #8 from Reto Gantenbein reto.gantenbein@linuxmonk.ch --- Regarding the non-default permissions: That's enforced by the upstream source code anyway so I thought I make it transparent this way. See: https://discuss.linuxcontainers.org/t/incus-log-file-and-directory-permissio...
Regarding incus-selinux error: I think the few SELinux file context definitions should eventually be moved to container-selinux similar to how it's done for Podman or Docker. To have these as a sub-package was convenient on COPR as long as I wasn't sure if this could land in Fedora.
Regarding log-files-without-logrotate: In the current default configuration I actually disabled the creation of a daemon log and only depend on journald as a log target. The problem is that the /var/log/incus directory is still "misused" for all kind of per-container runtime configuration and logs that are fully controlled by the Incus daemon. That's why there is no log file that must be managed by logrotate. These runtime state files will be moved to /run in the next few releases so that it might be possible to drop /var/log/incus altogether in the future. See: https://discuss.linuxcontainers.org/t/incus-0-5-has-been-released/18814#use-...
In case you're wondering why I reply instead of Neal Gompa is because he heavily based the submitted spec file on mine that I'm maintaining for the time being on COPR (https://github.com/ganto/copr-lxc4/tree/master/incus). Obviously I'm looking forward to co-maintain this package in Fedora too.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #9 from Neil Hanlon neil@shrug.pw ---
Regarding the non-default permissions: That's enforced by the upstream source code anyway so I thought I make it transparent this way. See: https://discuss.linuxcontainers.org/t/incus-log-file-and-directory-permissio...
Ack.
Regarding incus-selinux error: I think the few SELinux file context definitions should eventually be moved to container-selinux similar to how it's done for Podman or Docker. To have these as a sub-package was convenient on COPR as long as I wasn't sure if this could land in Fedora.
Right--definitely something that can be amended over time to be "correct".
Regarding log-files-without-logrotate: In the current default configuration I actually disabled the creation of a daemon log and only depend on journald as a log target. The problem is that the /var/log/incus directory is still "misused" for all kind of per-container runtime configuration and logs that are fully controlled by the Incus daemon. That's why there is no log file that must be managed by logrotate. These runtime state files will be moved to /run in the next few releases so that it might be possible to drop /var/log/incus altogether in the future. See: https://discuss.linuxcontainers.org/t/incus-0-5-has-been-released/18814#use-...
Acknowledged, that makes sense. I think good to leave for now, then.
In case you're wondering why I reply instead of Neal Gompa is because he heavily based the submitted spec file on mine that I'm maintaining for the time being on COPR (https://github.com/ganto/copr-lxc4/tree/master/incus). Obviously I'm looking forward to co-maintain this package in Fedora too.
Right, that makes sense :) I knew the spec was familiar! Very glad to see you here.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #10 from Neal Gompa ngompa13@gmail.com --- New version.
Spec URL: https://ngompa.fedorapeople.org/for-review/incus.spec SRPM URL: https://ngompa.fedorapeople.org/for-review/incus-0.7-1.fc40.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Michel Lind michel@michel-slm.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|neil@shrug.pw |michel@michel-slm.name Flags|fedora-review? | CC| |michel@michel-slm.name
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Michel Lind michel@michel-slm.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+
--- Comment #11 from Michel Lind michel@michel-slm.name --- previous reviewer seems to have accepted there are no remaining concerns but never set this as approved, so I've approved to unblock
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #12 from Neil Hanlon neil@shrug.pw --- Thanks Michel. I had this in my queue several times and kept getting hit with login issues with FAS :|
This is indeed fine and ready to move forward. Apologies for the delay.
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST
--- Comment #13 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/incus
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
Kamil Cynk k.d.cynk@protonmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |k.d.cynk@protonmail.com
--- Comment #14 from Kamil Cynk k.d.cynk@protonmail.com --- is it possible to add incus-ui? https://gist.github.com/vaxvhbe/ce679df15fc521c8aca1ff9ddf537201
https://bugzilla.redhat.com/show_bug.cgi?id=2260492
--- Comment #15 from Kamil Cynk k.d.cynk@protonmail.com --- also on fedora kinoite incus require another package to work rpm-ostree install qemu-system-x86
"Instance type not operational QEMU command not available: exec: "qemu-system-x86_64": executable file not found in $PATH"
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com