Bug 1242886 - Review Request: openstack-ironic-inspector - Hardware introspection service for OpenStack Ironic
Summary: Review Request: openstack-ironic-inspector - Hardware introspection service f...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chandan Kumar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: RDO-LIBERTY-REVIEWS
TreeView+ depends on / blocked
 
Reported: 2015-07-14 10:47 UTC by Dmitry Tantsur
Modified: 2015-09-11 17:16 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-09-11 17:16:14 UTC
Type: ---
Embargoed:
chkumar246: fedora-review?


Attachments (Terms of Use)

Description Dmitry Tantsur 2015-07-14 10:47:51 UTC
Spec URL: https://dl.dropboxusercontent.com/u/1730743/RPM/openstack-ironic-inspector.spec
SRPM URL: https://dl.dropboxusercontent.com/u/1730743/RPM/openstack-ironic-inspector-2.0.1-1.fc22.src.rpm

Description: Hardware introspection service for OpenStack Ironic
This is essentially openstack-ironic-discoverd package renamed upstream (in backward incompatible way).

Fedora Account System Username: divius
Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=10351884

Comment 1 bigswitch 2015-08-26 21:05:59 UTC
Hi Dmitry,

I see there's 'oslo.log' as requires on pypi, but its missing in the requires section in the spec file. Is that intentional (some other oslo package includes it)? Otherwise it should be added.

Everything else looks good.

Comment 2 Chandan Kumar 2015-08-31 04:52:11 UTC
Hello Dmitry,

Thanks for submitting for Package Review,

Below is my inline comments.
[1.] Since new version of openstack-ironic-inspector i.e. 2.1.0 is available,

Please update the spec file with latest version.

[2.] "Group:      System Environment/Base" is not required, 
Since this tag is optional, Please remove it, https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag

[3.] Please check the requires with requirements.txt and please update it

[4.] Under prep section,
Please rm -rf {test-,}requirements.txt update this to rm -rf {plugin-, test-,}requirements.txt

[5.] under %files -n python-ironic-inspector section,
please update %{python_sitelib}/ironic_inspector* to %{python2_sitelib}/ironic_inspector*
and also include %{python2_sitelib}/ironic_inspector-%{version}-py?.?.egg-info

[6.] Please run rpmlint on srpm, rpms and spec file and make a scratch koji build.

Thanks,

Chandan Kumar

Comment 3 bigswitch 2015-09-01 05:32:35 UTC
One more thing. My understanding is that documentation is recommended to be put to a subpackage.

Comment 4 John Trowbridge 2015-09-10 19:02:53 UTC
Dmitry,

I think we also need a line like the following to provide an upgrade path:

Provides:openstack-ironic-discoverd = 2.1.0-1

https://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

Comment 5 Haïkel Guémar 2015-09-10 20:12:25 UTC
## blockers 

This needs to be fixed and carefully tested!


* John is right, it needs provides for older package name 
Provides: openstack-ironic-discover = %{version}-%{release} # needs to be adapted for delorean though.
Obsoletes: openstack-ironic-discoverd < 1.1.0-3

* I haven't had the time to check but there was a python-ironic-discoverd subpackage. 
a) if it has been splitted, the new package needs to provides/obsoletes it
b) if it's been merged back in main package, main packages needs to provides/obsoletes it.

From Koji last builds
http://koji.fedoraproject.org/koji/buildinfo?buildID=655063
http://koji.fedoraproject.org/koji/rpminfo?rpmID=6438695
http://koji.fedoraproject.org/koji/rpminfo?rpmID=6438696


To check provides/obsoletes in a rpm package
rpm -qp --provides <rpm>
rpm -qp --provides <rpm>

Some very simple sanity checks:
* install older package then rpm -Uvh new package and see if it fails.
* install new packages and then use yum/dnf to install packages requiring them, see if it fails.


## Minor changes

* changelog was not updated
* %description should be before %prep
* to reduce diff with delorean packages
=> %{python2_sitelib}/ironic_inspector-*.egg-info
https://www.rdoproject.org/packaging/rdo-packaging.html#_differences_between_master_and_rawhide_packaging

Comment 6 Dmitry Tantsur 2015-09-11 07:46:04 UTC
Note that Provides: python-ironic-discoverd would be a lie: inspector Python modules are not compatible with discoverd.

Will fix, thanks for reviews!

Comment 7 John Trowbridge 2015-09-11 17:16:14 UTC
I updated the spec and srpm:

https://trown.fedorapeople.org/openstack-ironic-inspector.spec
https://trown.fedorapeople.org/openstack-ironic-inspector-2.1.0-1.fc24.src.rpm

and successfully built on koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=11050634

However, F24+ will no longer include openstack-* services. These will only be in delorean and Centos.[1]

I have started the process to import this to delorean, and Centos packaging will be cloned from there, so we can close this as WONTFIX.


[1] https://trello.com/c/wzdl1IlZ/52-openstack-in-fedora


Note You need to log in before you can comment on or make changes to this bug.