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
Copr build: https://copr.fedorainfracloud.org/coprs/build/6963272 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260492-incus/fedora-rawhide-x86_64/06963272-incus/builder-live.log.gz 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.
[fedora-review-service-build]
Created attachment 2011019 [details] The .spec file difference from Copr build 6963272 to 6967304
Copr build: https://copr.fedorainfracloud.org/coprs/build/6967304 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260492-incus/fedora-rawhide-x86_64/06967304-incus/builder-live.log.gz 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.
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
Copr build: https://copr.fedorainfracloud.org/coprs/build/7092290 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2260492-incus/fedora-rawhide-x86_64/07092290-incus/fedora-review/review.txt 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.
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.
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-permissions/18520/4 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-of-runincus-for-runtime-data-20 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.
> 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-permissions/18520/4 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-of-runincus-for-runtime-data-20 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.
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
previous reviewer seems to have accepted there are no remaining concerns but never set this as approved, so I've approved to unblock
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.
The Pagure repository was created at https://src.fedoraproject.org/rpms/incus
is it possible to add incus-ui? https://gist.github.com/vaxvhbe/ce679df15fc521c8aca1ff9ddf537201
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"