[Rdo-list] Fwd: Re: Octavia Packaging - spec review needed

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Hi all, this email was sent to the author of the openstack-octavia package, but now I think I should have shared it with broader community, since it highlights some issues in our new packages process. As you will see below, there are a lot of issues in the package that we happily allowed in Delorean. I think those could be easily spotted by anyone who would look into the contents of proposed repo. I suspect we just blindly allowed it in. This once more time highlights we need our own strict review process in Delorean, that would spot those issues before we get official Delorean packages built. Postponing it till Fedora review (if it's even perceived by authors of new packages, which I'm not really sure they do) is wrong. And after we get our own Delorean review process, we should consider the need of Fedora reviews too. ;) === Issues I identified in openstack-octavia package without even running it : - - amphora agent reads /etc/neutron/conf.d/octavia-api... And it logs into octavia-api.log??? And why is octavia-dist.conf located in /usr/share/neutron?.. - - Can we have log file to be api.log instead of octavia-api.log? It's located under /var/log/octavia/, so there is no big reason to repeat it again in the file name? - - octavia-dist.conf: do we really need most of those conf values? like verbose = True? notification_driver - does it even have reasonable value?.. etc. etc. We should walk thru each entry there and determine whether it's a must to have it, or whether it's a reasonable default. - - octavia-health-manager.service: it reads /usr/share/octavia/octavia-health-manager. I don't think it's even created, and it's probably not needed at all; - - in .spec file, I don't see where conf.d files are created. - - octavia-housekeeping.service: why does it refer to health manager??? - - for all .service files, have we checked that all declarative values we use are reasonable? NotifyAccess=all? KillMode=process? - - there is no need to have 'Epoch: 1' there since we haven't packaged anything with 201x.* versions for octavia before. - - "Summary: OpenStack LBaaSv2 Service" What's the difference from LBaaSv2 service plugin then? - - I think Group: is now not needed. - - what do you use "Requires: openstack-utils" for? Not sure it's needed. - - I assume requirements are all as per in upstream... - - I think the service for amphora agent should have octavia- in its name (same as its package). Ihar