RDO tickets are now tracked in Jira https://issues.redhat.com/projects/RDO/issues/
Bug 1400715 - New package: networking-fujitsu - neutron ML2 plugin
Summary: New package: networking-fujitsu - neutron ML2 plugin
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RDO
Classification: Community
Component: Package Review
Version: trunk
Hardware: x86_64
OS: Linux
unspecified
high
Target Milestone: ---
: trunk
Assignee: Alfredo Moralejo
QA Contact: hguemar
URL:
Whiteboard:
Depends On:
Blocks: RDO-OCATA
TreeView+ depends on / blocked
 
Reported: 2016-12-01 22:42 UTC by Koki Sanagi
Modified: 2016-12-23 15:26 UTC (History)
13 users (show)

Fixed In Version: python-networking-fujitsu-3.0.0-0.20161220120152.5a1da9f.el7
Clone Of: 1392578
Environment:
Last Closed: 2016-12-23 15:26:47 UTC
Embargoed:
amoralej: rdo-review+


Attachments (Terms of Use)
spec file (2.00 KB, text/x-rpm-spec)
2016-12-01 22:42 UTC, Koki Sanagi
no flags Details
srpm (49.49 KB, application/x-rpm)
2016-12-01 22:43 UTC, Koki Sanagi
no flags Details
spec file v2 (2.33 KB, text/x-matlab)
2016-12-05 02:43 UTC, Koki Sanagi
no flags Details
srpm v2 (50.23 KB, application/x-rpm)
2016-12-05 02:44 UTC, Koki Sanagi
no flags Details
spec file v3 (2.20 KB, text/x-matlab)
2016-12-05 15:54 UTC, Koki Sanagi
no flags Details
srpm v3 (50.01 KB, application/x-rpm)
2016-12-05 15:54 UTC, Koki Sanagi
no flags Details
spec file v4 (2.20 KB, text/x-matlab)
2016-12-05 18:35 UTC, Koki Sanagi
no flags Details
srpm v4 (50.01 KB, application/x-rpm)
2016-12-05 18:36 UTC, Koki Sanagi
no flags Details
spec file v5 (2.20 KB, text/x-matlab)
2016-12-06 07:16 UTC, Koki Sanagi
no flags Details
srpm v5 (50.01 KB, application/x-rpm)
2016-12-06 07:16 UTC, Koki Sanagi
no flags Details
spec file v6 (2.50 KB, text/x-matlab)
2016-12-07 21:05 UTC, Koki Sanagi
no flags Details
srpm v6 (50.09 KB, application/x-rpm)
2016-12-07 21:06 UTC, Koki Sanagi
no flags Details
spec file v7 (2.50 KB, text/x-matlab)
2016-12-13 17:22 UTC, Koki Sanagi
no flags Details
srpm v7 (50.09 KB, application/x-rpm)
2016-12-13 17:22 UTC, Koki Sanagi
no flags Details

Description Koki Sanagi 2016-12-01 22:42:35 UTC
Created attachment 1227054 [details]
spec file

networking-fujitsu is a neutron ML2 plugin for C-Fabic switch made by FUJITSU.
The Japanese explanation of C-Fabric is following.(No English version)
       
http://jp.fujitsu.com/platform/server/primergy/technical/handbook/c-fabric/
       
The main conept of C-Fabric switch is that multiple C-Fabric switches can
behave as one big virtual switch with High-Availability and High-Bandwidth.
       
With this plugin, neutron can control the C-Fabric switch directly and
form virtual network with C-Fabric.

Source code is following.
  https://github.com/openstack/networking-fujitsu
         
The PyPI is here.
  https://pypi.python.org/pypi/networking-fujitsu

rdoinfo review:
  https://review.rdoproject.org/r/#/c/3874/

Comment 1 Koki Sanagi 2016-12-01 22:43:32 UTC
Created attachment 1227056 [details]
srpm

Comment 2 Koki Sanagi 2016-12-01 22:44:10 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1227054
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1227056
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

Comment 3 Alfredo Moralejo 2016-12-02 11:52:38 UTC
Thanks for contributing this package to RDO.

Some comments:

- Don't need to define "%define upstream_version 2.0.0" in line 1. Instead, use 2.0.0 for Version and Release 1%{?dist} in the review. We'll change it to XXX when importing it to review.rdoproject.org for dlrn builds.

- About requirements:
  - Unit test requirements defined in test-requirements.txt should be added only as BuildRequires, not as Requires.
  - In Requires using explicit required versions help to ensure we provide minimal required versions, specially for the rpm update case. In this case, it's the first version of the package, but i think it's a good practice to define them (only in Requires, not in BuildRequires)

- In %prep section we recomend using autosetup -S git and removing *requirements.txt files to ensure dependencies are not installed by pip accidentally during the build.

- For build and install tasks, for consistency we recomend using existing macros %py2_build and %py2_install.

- It's highly recommended to run unit tests in the %check section.

- For the spec in the review you should add a comment in the %changelog section. It'll be removed for dlrn builds.

Comment 4 Koki Sanagi 2016-12-05 02:43:24 UTC
Created attachment 1227961 [details]
spec file v2

Comment 5 Koki Sanagi 2016-12-05 02:44:16 UTC
Created attachment 1227962 [details]
srpm v2

Comment 6 Koki Sanagi 2016-12-05 02:45:21 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1227961
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1227962
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

Comment 7 Koki Sanagi 2016-12-05 02:47:06 UTC
Hi Alfredo,

Thanks for your review.
I reflected them into v2. Could you review it again ?

Koki

Comment 8 Alfredo Moralejo 2016-12-05 11:56:17 UTC
Some more comments:

- Don't add Epoch version. Epoch use is not recommended unless it's fully required in very specific scenarios.

- python-rpm-macros and python2-rpm-macros must not be added as BuildRequires. We take care of injecting the required macros in build environment using rdo-rpm-macros. It's not required to be managed in the spec file.

- I think python-mock and python-mimeparse are not required as Requires but as BuildRequires (i'd say python-mimeparse is not even needed as BuildRequires)

- Project documentation is being built but not added to the package. You need to add following line in the %files section:

%doc html
 
- Empty sections post, preun and postun can be removed.

Comment 9 Koki Sanagi 2016-12-05 15:54:11 UTC
Created attachment 1228096 [details]
spec file v3

Comment 10 Koki Sanagi 2016-12-05 15:54:40 UTC
Created attachment 1228097 [details]
srpm v3

Comment 11 Koki Sanagi 2016-12-05 15:56:02 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1228096
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1228097
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

Comment 12 Koki Sanagi 2016-12-05 15:56:57 UTC
Hi Alfredo,

Thanks for your review.
I included them into v3. Could you review it again ?

Koki

Comment 13 Alfredo Moralejo 2016-12-05 18:08:57 UTC
fedora-review output:

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package contains BR: python2-devel or python3-devel


===== MUST items =====

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Apache (v2.0)", "*No copyright* Apache", "Unknown or
     generated", "*No copyright* Apache (v2.0)". 31 files have unknown
     license. Detailed output of licensecheck in /tmp/fujitsu/python-
     networking-fujitsu/licensecheck.txt
[ ]: Package requires other packages for directories it uses.
     Note: No known owner of /etc/neutron/plugins,
     /etc/neutron/plugins/ml2, /etc/neutron
[ ]: Package must own all directories that it creates.
     Note: Directories without known owners: /etc/neutron/plugins/ml2,
     /etc/neutron, /etc/neutron/plugins
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 296960 bytes in 39 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[ ]: Python eggs must not download any dependencies during the build
     process.
[ ]: A package which is used by another package via an egg interface should
     provide egg info.
[ ]: Package meets the Packaging Guidelines::Python
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: Package should compile and build into binary rpms on all supported
     architectures.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: python-networking-fujitsu-2.0.0-1.el7.centos.noarch.rpm
          python-networking-fujitsu-2.0.0-1.el7.centos.src.rpm
python-networking-fujitsu.noarch: W: incoherent-version-in-changelog 2.0.0-1 ['2.0.0-1.el7.centos', '2.0.0-1.centos']
python-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini neutron
python-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini 640
python-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini neutron
python-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini 640
2 packages and 0 specfiles checked; 2 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
python-networking-fujitsu.noarch: E: explicit-lib-dependency python-httplib2
python-networking-fujitsu.noarch: W: incoherent-version-in-changelog 2.0.0-1 ['2.0.0-1.el7.centos', '2.0.0-1.centos']
python-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini neutron
python-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini 0640L
python-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini neutron
python-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini 0640L
1 packages and 0 specfiles checked; 3 errors, 3 warnings.



Requires
--------
python-networking-fujitsu (rpmlib, GLIBC filtered):
    config(python-networking-fujitsu)
    openstack-neutron
    python(abi)
    python-babel
    python-eventlet
    python-httplib2
    python-oslo-config
    python-oslo-i18n
    python-oslo-log
    python-oslo-utils
    python-pbr
    python-six



Provides
--------
python-networking-fujitsu:
    config(python-networking-fujitsu)
    python-networking-fujitsu



Source checksums
----------------
https://tarballs.openstack.org/networking-fujitsu/networking-fujitsu-2.0.0.tar.gz :
  CHECKSUM(SHA256) this package     : 6ce162dcb87402f6b7a9e10406364c71f996f9abd3e02538697bd675dfa2991d
  CHECKSUM(SHA256) upstream package : 6ce162dcb87402f6b7a9e10406364c71f996f9abd3e02538697bd675dfa2991d


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -m dlrn-master -rn python-networking-fujitsu-2.0.0-1.el7.centos.src.rpm
Buildroot used: dlrn-centos-ocata-x86_64-1
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 14 Alfredo Moralejo 2016-12-05 18:09:46 UTC
licensecheck:

*No copyright* Apache
---------------------
networking-fujitsu-2.0.0/PKG-INFO
networking-fujitsu-2.0.0/README.rst
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/PKG-INFO
networking-fujitsu-2.0.0/setup.cfg

*No copyright* Apache (v2.0)
----------------------------
networking-fujitsu-2.0.0/LICENSE
networking-fujitsu-2.0.0/doc/source/conf.py
networking-fujitsu-2.0.0/networking_fujitsu/__init__.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/test_networking_fujitsu.py

Apache (v2.0)
-------------
networking-fujitsu-2.0.0/networking_fujitsu/i18n.py
networking-fujitsu-2.0.0/networking_fujitsu/ml2/drivers/fujitsu/cfab/cfabdriver.py
networking-fujitsu-2.0.0/networking_fujitsu/ml2/drivers/fujitsu/cfab/mechanism_fujitsu.py
networking-fujitsu-2.0.0/networking_fujitsu/ml2/drivers/fujitsu/common/utils.py
networking-fujitsu-2.0.0/networking_fujitsu/ml2/drivers/fujitsu/ism/ism_base.py
networking-fujitsu-2.0.0/networking_fujitsu/ml2/drivers/fujitsu/ism/mech_fujitsu_ism.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/base.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/unit/ml2/drivers/fujitsu/cfab/test_fujitsu_cfabdriver.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/unit/ml2/drivers/fujitsu/cfab/test_fujitsu_mechanism_driver.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/unit/ml2/drivers/fujitsu/common/test_utils.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/unit/ml2/drivers/fujitsu/ism/ism_common.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/unit/ml2/drivers/fujitsu/ism/test_ism_base.py
networking-fujitsu-2.0.0/networking_fujitsu/tests/unit/ml2/drivers/fujitsu/ism/test_mech_fujitsu_ism.py
networking-fujitsu-2.0.0/networking_fujitsu/version.py
networking-fujitsu-2.0.0/tools/subunit-trace.py

Unknown or generated
--------------------
networking-fujitsu-2.0.0/.coveragerc
networking-fujitsu-2.0.0/.mailmap
networking-fujitsu-2.0.0/.testr.conf
networking-fujitsu-2.0.0/AUTHORS
networking-fujitsu-2.0.0/CONTRIBUTING.rst
networking-fujitsu-2.0.0/ChangeLog
networking-fujitsu-2.0.0/HACKING.rst
networking-fujitsu-2.0.0/MANIFEST.in
networking-fujitsu-2.0.0/babel.cfg
networking-fujitsu-2.0.0/doc/source/contributing.rst
networking-fujitsu-2.0.0/doc/source/index.rst
networking-fujitsu-2.0.0/doc/source/installation.rst
networking-fujitsu-2.0.0/doc/source/readme.rst
networking-fujitsu-2.0.0/doc/source/usage.rst
networking-fujitsu-2.0.0/etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini
networking-fujitsu-2.0.0/etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/SOURCES.txt
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/dependency_links.txt
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/entry_points.txt
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/not-zip-safe
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/pbr.json
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/requires.txt
networking-fujitsu-2.0.0/networking_fujitsu.egg-info/top_level.txt
networking-fujitsu-2.0.0/openstack-common.conf
networking-fujitsu-2.0.0/releasenotes/notes/bug-1571949-725c62c33fda637c.yaml
networking-fujitsu-2.0.0/releasenotes/source/conf.py
networking-fujitsu-2.0.0/releasenotes/source/index.rst
networking-fujitsu-2.0.0/releasenotes/source/liberty.rst
networking-fujitsu-2.0.0/setup.py
networking-fujitsu-2.0.0/tools/pretty_tox.sh
networking-fujitsu-2.0.0/tox.ini

Comment 15 Alfredo Moralejo 2016-12-05 18:16:03 UTC
Last issue:

- BuidRequires should be on python2-devel instead of python-devel


About rpmlint errors:

- python-networking-fujitsu.noarch: E: explicit-lib-dependency python-httplib2 : we manage python dependencies explicitely in RDO

- Configuration files permissions:
python-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini 0640L
python-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini 0640L

configuration files owner is root:neutron with mode 0640 which seems correct as package depends on openstack-neutron that takes care of neutron user and group creation.

For the record, this spec is not providing subpackages for tests and docs. Given the size of the package and the fact that tests will not be required by other packages, i think it's ok to keep tests in main package.

Comment 16 Koki Sanagi 2016-12-05 18:35:55 UTC
Created attachment 1228130 [details]
spec file v4

Comment 17 Koki Sanagi 2016-12-05 18:36:19 UTC
Created attachment 1228131 [details]
srpm v4

Comment 18 Koki Sanagi 2016-12-05 18:37:40 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1228130
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1228131
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

Comment 19 Koki Sanagi 2016-12-05 18:40:35 UTC
Hi Alfredo,

Thanks for your work.
This is the diff between v3 and v4.

---
 python-networking-fujitsu.spec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python-networking-fujitsu.spec b/python-networking-fujitsu.spec
index 8ad5bfb..d101997 100644
--- a/python-networking-fujitsu.spec
+++ b/python-networking-fujitsu.spec
@@ -23,7 +23,7 @@ BuildRequires:  python-testscenarios
 BuildRequires:  python-testtools
 BuildRequires:  python-reno
 BuildRequires:  python-testresources
-BuildRequires:  python-devel
+BuildRequires:  python2-devel
 BuildRequires:  python-setuptools
 BuildRequires:  python-oslo-i18n
 BuildRequires:  python-oslo-utils

Koki

Comment 20 Koki Sanagi 2016-12-06 07:16:03 UTC
Created attachment 1228297 [details]
spec file v5

Comment 21 Koki Sanagi 2016-12-06 07:16:36 UTC
Created attachment 1228298 [details]
srpm v5

Comment 22 Koki Sanagi 2016-12-06 07:19:17 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1228297
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1228298
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

I just lowered the version of python-oslo-utils

--- a/python-networking-fujitsu.spec
+++ b/python-networking-fujitsu.spec
@@ -32,7 +32,7 @@ BuildRequires:  python-neutron-tests
 BuildRequires:  python-mock
 
 Requires:       python-oslo-i18n >= 2.1.0
-Requires:       python-oslo-utils >= 3.18.0
+Requires:       python-oslo-utils >= 3.16.0
 Requires:       python-oslo-log >= 3.11.0
 Requires:       python-pbr >= 1.8
 Requires:       python-babel >= 2.3.4

Koki

Comment 23 Haïkel Guémar 2016-12-07 14:57:09 UTC
We need formal reviews report, this is hard requirement from legal team.

Comment 24 Koki Sanagi 2016-12-07 16:41:11 UTC
Hi, Haïkel,

What is the formal reviews report ?
Is that something we Fujitsu need to provide ? Or something Red Hat needs to
work on ?

Best Regards,
Koki Sanagi

Comment 25 Alfredo Moralejo 2016-12-07 17:34:56 UTC
Hi,

I'm preparing the formal review report. While doing it, i found a couple of new issues:

- According to fedora guidelines for python, rpm containing the module should be a subpackage with name python2-networking-fujitsu. Note that the package should provide python-networking-fujitsu using the python_provide macro.

- About the requirements, instead of "Requires: openstack-neutron" it should depend on openstack-neutron-common and openstack-neutron-ml2.

Comment 26 Koki Sanagi 2016-12-07 21:05:52 UTC
Created attachment 1229219 [details]
spec file v6

Comment 27 Koki Sanagi 2016-12-07 21:06:26 UTC
Created attachment 1229220 [details]
srpm v6

Comment 28 Koki Sanagi 2016-12-07 21:13:00 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1229219
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1229220
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

>I'm preparing the formal review report.

Thanks.

>- According to fedora guidelines for python, rpm containing the module should >be a subpackage with name python2-networking-fujitsu. Note that the package >should provide python-networking-fujitsu using the python_provide macro.
>
>- About the requirements, instead of "Requires: openstack-neutron" it should >depend on openstack-neutron-common and openstack-neutron-ml2.

I included them into v6.

Koki

Comment 29 Koki Sanagi 2016-12-08 22:23:25 UTC
Hi,

I put the v6 spec file into rdo gerrit.

https://review.rdoproject.org/r/#/c/4039/

When it is OK, please merge it.
If there is another issue let me know.

Koki

Comment 30 Alfredo Moralejo 2016-12-12 15:35:00 UTC
Note that "Requires" must be moved under python2-networking-fujitsu package instead of the main one.

Comment 31 Koki Sanagi 2016-12-13 17:22:01 UTC
Created attachment 1231285 [details]
spec file v7

Comment 32 Koki Sanagi 2016-12-13 17:22:39 UTC
Created attachment 1231286 [details]
srpm v7

Comment 33 Koki Sanagi 2016-12-13 17:24:51 UTC
Spec URL: https://bugzilla.redhat.com/attachment.cgi?id=1231285
SRPM URL: https://bugzilla.redhat.com/attachment.cgi?id=1231286
Description: neutron ML2 plugin for FUJITSU C-Fabric switch

>Note that "Requires" must be moved under python2-networking-fujitsu package instead of the main one.

I included it into v7.

Koki

Comment 34 Alfredo Moralejo 2016-12-13 18:55:42 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Apache (v2.0)", "*No copyright* Apache", "Unknown or
     generated", "*No copyright* Apache (v2.0)". 31 files have unknown
     license. Detailed output of licensecheck in /tmp/fujitsu1/python-
     networking-fujitsu/licensecheck.txt
[x]: Package requires other packages for directories it uses.
     Note: No known owner of /etc/neutron/plugins,
     /etc/neutron/plugins/ml2, /etc/neutron
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /etc/neutron/plugins/ml2,
     /etc/neutron, /etc/neutron/plugins
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 296960 bytes in 39 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: python2-networking-fujitsu-2.0.0-1.el7.centos.noarch.rpm
          python-networking-fujitsu-2.0.0-1.el7.centos.src.rpm
python2-networking-fujitsu.noarch: W: summary-not-capitalized C neutron ML2 plugin for Fujitsu switch
python2-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini neutron
python2-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini 640
python2-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini neutron
python2-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini 640
2 packages and 0 specfiles checked; 2 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
python2-networking-fujitsu.noarch: E: explicit-lib-dependency python-httplib2

Comment: this is a false negative error.  rpm doesn't handle explicit lib dependencies for python packages.

python2-networking-fujitsu.noarch: W: summary-not-capitalized C neutron ML2 plugin for Fujitsu switch
python2-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini neutron
python2-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_ism.ini 0640L

Comment: permissions are correct root:neutron 0640. Only neutron user needs to access the config file and it has required permissions.

python2-networking-fujitsu.noarch: W: non-standard-gid /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini neutron
python2-networking-fujitsu.noarch: E: non-readable /etc/neutron/plugins/ml2/ml2_conf_fujitsu_cfab.ini 0640L

Comment: permissions are correct root:neutron 0640. Only neutron user needs to access the config file and it has required permissions.

1 packages and 0 specfiles checked; 3 errors, 3 warnings.



Requires
--------
python2-networking-fujitsu (rpmlib, GLIBC filtered):
    config(python2-networking-fujitsu)
    openstack-neutron-common
    openstack-neutron-ml2
    python(abi)
    python-babel
    python-eventlet
    python-httplib2
    python-oslo-config
    python-oslo-i18n
    python-oslo-log
    python-oslo-utils
    python-pbr
    python-six



Provides
--------
python2-networking-fujitsu:
    config(python2-networking-fujitsu)
    python-networking-fujitsu
    python2-networking-fujitsu



Source checksums
----------------
https://tarballs.openstack.org/networking-fujitsu/networking-fujitsu-2.0.0.tar.gz :
  CHECKSUM(SHA256) this package     : 6ce162dcb87402f6b7a9e10406364c71f996f9abd3e02538697bd675dfa2991d
  CHECKSUM(SHA256) upstream package : 6ce162dcb87402f6b7a9e10406364c71f996f9abd3e02538697bd675dfa2991d


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -m dlrn-master -rn python-networking-fujitsu-2.0.0-1.fc25.src.rpm
Buildroot used: dlrn-centos-ocata-x86_64-1
Active plugins: Python, Generic, Shell-api
Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6


Package is approved.

Comment 35 Koki Sanagi 2016-12-14 20:44:21 UTC
Hi,

Thanks for including spec file into repository.
Now I will remove under-review tag and put ocata and newton tag. 
But I got a question. What is ocata-uc ?
Is this something for undercloud ?

Koki

Comment 36 Alfredo Moralejo 2016-12-14 22:16:23 UTC
(In reply to Koki Sanagi from comment #35)
> Hi,
> 
> Thanks for including spec file into repository.
> Now I will remove under-review tag and put ocata and newton tag. 
> But I got a question. What is ocata-uc ?
> Is this something for undercloud ?

In RDO Trunk we keep two delorean builders for master (currently ocata), one follwoing upper-constraints.txt, ocata-uc for libraries and clients and one following master for oo packages, ocata. You have more details in https://www.rdoproject.org/blog/2016/11/chasing-the-trunk-but-not-too-fast/

> 
> Koki

Comment 37 Koki Sanagi 2016-12-15 03:09:03 UTC
Thanks the explanation, I got it.

I submitted the patch to handle the tags.

https://review.rdoproject.org/r/#/c/4087/

Koki


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