Bug 1263941 - Review Request: tayga - Simple out-of-kernel stateless NAT64 daemon
Review Request: tayga - Simple out-of-kernel stateless NAT64 daemon
Status: ON_QA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
: Reopened
: 1028206 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-17 02:46 EDT by Ingvar Hagelund
Modified: 2016-02-01 04:56 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-01-15 10:22:03 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Ingvar Hagelund 2015-09-17 02:46:28 EDT
Spec URL: http://users.linpro.no/ingvar/tayga/tayga.spec
SRPM URL: http://users.linpro.no/ingvar/tayga/tayga-0.9.2-1.fc22.src.rpm
Description: Simple out-of-kernel stateless NAT64 daemon
Fedora Account System Username: ingvar

TAYGA is an out-of-kernel stateless NAT64 implementation for Linux that uses
the TUN driver to exchange IPv4 and IPv6 packets with the kernel. It is
intended to provide production-quality NAT64 service for networks where
dedicated NAT64 hardware would be overkill.
Comment 1 Ingvar Hagelund 2015-09-17 02:47:28 EDT
*** Bug 1028206 has been marked as a duplicate of this bug. ***
Comment 2 Ingvar Hagelund 2015-09-17 02:51:38 EDT
Additional info, or why NAT64?

With the support of a DNS64 server, tayga allows the translation of ipv4 addresses into an ipv6 prefix. This allows a network to be ipv6 only, while still maintaining contact with ipv4 networks.
Comment 3 Petr Pisar 2015-09-30 09:24:41 EDT
I suspect the spec file is missing various BuildRequires like coreutils, sed, make, gcc for executed commands.

It should build-require systemd for the %{_unitdir} macro. The %post and other scriptlets should run-require chkconfig or systemd respectively. See <https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd>.

The COPYING file should be packaged by %license macro instead of the %doc macro on Fedora.

I worry that that the LDFLAGS="%{optflags} -pie" ignores default LDFLAGS defined by rpmbuild.
Comment 4 Ingvar Hagelund 2015-09-30 19:40:37 EDT
(In reply to Petr Pisar from comment #3)

Thank you, Petr

Updated src.rpm and specfile here:

http://users.linpro.no/ingvar/tayga/tayga-0.9.2-2.fc22.src.rpm
http://users.linpro.no/ingvar/tayga/tayga.spec

> I suspect the spec file is missing various BuildRequires like coreutils,
> sed, make, gcc for executed commands.

I don't think this is necessary. There is an implicit minimum build system on all fedora build systems that includes coreutils, binutils, gcc, make, etc. The packages do build fine in mock and koji without adding these explicitly.

> It should build-require systemd for the %{_unitdir} macro. The %post and
> other scriptlets should run-require chkconfig or systemd respectively. See
> <https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd>.

Fixed
 
> The COPYING file should be packaged by %license macro instead of the %doc
> macro on Fedora.

Fixed

> I worry that that the LDFLAGS="%{optflags} -pie" ignores default LDFLAGS
> defined by rpmbuild.

Fixed


Ingvar
Comment 5 Upstream Release Monitoring 2015-09-30 19:46:38 EDT
ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for epel7 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11284416
Comment 6 Jason Tibbitts 2015-09-30 19:49:07 EDT
Just so you know, the packaging guidelines include no implicit minimum build system and while I doubt it will actually happen, it's quite possible for gcc to not be there.  Nobody is enforcing this currently, but if buildroot changes break your packages due to incompletely specified dependencies then you do get to keep all of the pieces.

Plus having the actual dependencies properly specified does make it easier for distro bootstrapping, though that is a minor thing for a leaf package such as this.
Comment 7 Ingvar Hagelund 2015-09-30 20:23:05 EDT
Okay, I've added explicit buildreqs on gcc, make, and coreutils. Updated source package and spec, though not bumped release.
Comment 8 Upstream Release Monitoring 2015-09-30 20:27:09 EDT
ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11284998
Comment 9 Upstream Release Monitoring 2015-09-30 20:32:26 EDT
ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11285099
Comment 10 Upstream Release Monitoring 2015-09-30 20:36:20 EDT
ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11285162
Comment 11 Upstream Release Monitoring 2015-09-30 20:39:05 EDT
ingvar's scratch build of tayga-0.9.2-2.fc22.src.rpm for dist-6E-epel completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11285185
Comment 12 Lubomir Rintel 2016-01-13 08:37:42 EST
* Named correctly
* Versioned correctly
* Packaging the latest version
* License good for fedora
* rpmlint mostly happy (see below)
* Builds fine in mock
* Dependencies sane
* Fileslist sane
* SPEC file clean and legible

0.) License tag not correct

> License:        GPLv2

It seems to be "GPLv2+" (there's an or later version clause).

1.) Please add comments that would describe the upstreaming status of the patches

> Patch0:         tayga-0.9.2_redhat_initscripts_and_systemd.patch
> Patch1:         tayga-0.9.2_cflags_override.patch

Have you send them upstream? Is there a mailing list or bug tracker reference?

2.) Why do you turn off PIE on ppc64?

> # PIE hardening seems to fail on ppc64

If this is really the case, please add a more descriptive comment (error
output).

3.) You're missing the %defattr tag

It's not needed for current RPM, but seems like you're targetting old RHEL
versions as well?

4.) You're installing a service with name of a templated service:

> ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service

This makes no sense. systemd would probably just ignore that. 'systemctl enable
tayga@example' would make the link in the proper place; just drop that line.

5.) rpmlint complains:

> tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING
> The Free Software Foundation address in this file seems to be outdated or
> misspelled.  Ask upstream to update the address, or if this is a license file,
> possibly the entire file with a new copy available from the FSF.

This is something the upstream would need to fix; you may want to let them
know. Obviously not a review blocker.
Comment 13 Upstream Release Monitoring 2016-01-14 07:32:23 EST
ingvar's scratch build of tayga-0.9.2-3.el7.src.rpm for epel7 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12546574
Comment 14 Ingvar Hagelund 2016-01-14 09:20:07 EST
Hello, Lubomir. Thanks for taking the review :-)

Updated src.rpm: https://ingvar.fedorapeople.org/tayga/tayga-0.9.2-3.fc23.src.rpm
Updated specfile: https://ingvar.fedorapeople.org/tayga/tayga.spec

(In reply to Lubomir Rintel from comment #12)
> 0.) License tag not correct
> 
> > License:        GPLv2
> 
> It seems to be "GPLv2+" (there's an or later version clause).

Fixed. I have also asked upstream to add this to README or some other more visible file. Atm, it's only visible in the manpage.
 
> 1.) Please add comments that would describe the upstreaming status of the
> patches
> 
> > Patch0:         tayga-0.9.2_redhat_initscripts_and_systemd.patch
> > Patch1:         tayga-0.9.2_cflags_override.patch

Done
 
> Have you send them upstream? Is there a mailing list or bug tracker
> reference?

Yes, they are sent upstream. I have not found any issue tracker nor mailing list for tayga. I have asked upstream for this as well, but not received any reply.

> 2.) Why do you turn off PIE on ppc64?
> 
> > # PIE hardening seems to fail on ppc64
> 
> If this is really the case, please add a more descriptive comment (error
> output).

After a bit of debugging, I found this not to be a problem with PIE, but a difference on Red Hat's ppc64 and x86_64 koji builders. The ppc64 one does not take "-z now" in LDFLAGS, though the x86_64 one does. Changing to "-znow" seems to work, so I have removed the workaround.


> 3.) You're missing the %defattr tag
> 
> It's not needed for current RPM, but seems like you're targetting old RHEL
> versions as well?

Fixed.

> 4.) You're installing a service with name of a templated service:
> 
> > ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service
> 
> This makes no sense. systemd would probably just ignore that.

No, it treats it as a service.

> 'systemctl enable tayga@example' would make the link in the proper place; 
> just drop that line.

Okay, dropped, but it would not be trivial for the non-seasoned systemd user to know how to enable the service. Should I add a README.RedHat with a note explaining this?

Also, Will the %systemd_preun %{name}@.service remove both the unit file and any generated symlinks?

> 5.) rpmlint complains:
> 
> > tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING
> > The Free Software Foundation address in this file seems to be outdated or
> > misspelled.  Ask upstream to update the address, or if this is a license file,
> > possibly the entire file with a new copy available from the FSF.
> 
> This is something the upstream would need to fix; you may want to let them
> know. Obviously not a review blocker.

I did notice upstream about this as well.

Ingvar
Comment 15 Upstream Release Monitoring 2016-01-14 09:51:23 EST
ingvar's scratch build of tayga-0.9.2-3.el5.src.rpm for dist-5E-epel completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12547542
Comment 16 Ingvar Hagelund 2016-01-14 18:40:25 EST
I've updated the spec with a README.redhat that describes how to set up multi instances of tayga, for both sysv and systemd, and updated the init scripts similarly. The package also now builds on epel5.
Comment 17 Lubomir Rintel 2016-01-15 04:31:36 EST
(In reply to Ingvar Hagelund from comment #14)
> Hello, Lubomir. Thanks for taking the review :-)
> 
> Updated src.rpm:
> https://ingvar.fedorapeople.org/tayga/tayga-0.9.2-3.fc23.src.rpm
> Updated specfile: https://ingvar.fedorapeople.org/tayga/tayga.spec
> 
> (In reply to Lubomir Rintel from comment #12)
> > 0.) License tag not correct
> > 
> > > License:        GPLv2
> > 
> > It seems to be "GPLv2+" (there's an or later version clause).
> 
> Fixed. I have also asked upstream to add this to README or some other more
> visible file. Atm, it's only visible in the manpage.

Well, it's visible in the comments in the source code, which is what actually matters.

> > 1.) Please add comments that would describe the upstreaming status of the
> > patches
> > 
> > > Patch0:         tayga-0.9.2_redhat_initscripts_and_systemd.patch
> > > Patch1:         tayga-0.9.2_cflags_override.patch
> 
> Done
>  
> > Have you send them upstream? Is there a mailing list or bug tracker
> > reference?
> 
> Yes, they are sent upstream. I have not found any issue tracker nor mailing
> list for tayga. I have asked upstream for this as well, but not received any
> reply.

Thanks.

> > 2.) Why do you turn off PIE on ppc64?
> > 
> > > # PIE hardening seems to fail on ppc64
> > 
> > If this is really the case, please add a more descriptive comment (error
> > output).
> 
> After a bit of debugging, I found this not to be a problem with PIE, but a
> difference on Red Hat's ppc64 and x86_64 koji builders. The ppc64 one does
> not take "-z now" in LDFLAGS, though the x86_64 one does. Changing to
> "-znow" seems to work, so I have removed the workaround.

Good job on this one, thanks.

> > 3.) You're missing the %defattr tag
> > 
> > It's not needed for current RPM, but seems like you're targetting old RHEL
> > versions as well?
> 
> Fixed.

> > 4.) You're installing a service with name of a templated service:
> > 
> > > ln -s %{_unitdir}/%{name}@.service %{buildroot}%{_unitdir}/%{name}@example.service
> > 
> > This makes no sense. systemd would probably just ignore that.
> 
> No, it treats it as a service.
> 
> > 'systemctl enable tayga@example' would make the link in the proper place; 
> > just drop that line.
> 
> Okay, dropped, but it would not be trivial for the non-seasoned systemd user
> to know how to enable the service.

Yes, but how does this help a non-seasoned user?

Ideally upstream would should distribute the service files along with the documentation on how to use them.

> Should I add a README.RedHat with a note
> explaining this?

Yes, I think that's okay.

> Also, Will the %systemd_preun %{name}@.service remove both the unit file and
> any generated symlinks?

Of course not. The packages shouldn't touch /etc. If the user installs file in /etc, he's responsible for dealing with it. That's the same regardless of whether the service is templated or not.

> > 5.) rpmlint complains:
> > 
> > > tayga.x86_64: E: incorrect-fsf-address /usr/share/licenses/tayga/COPYING
> > > The Free Software Foundation address in this file seems to be outdated or
> > > misspelled.  Ask upstream to update the address, or if this is a license file,
> > > possibly the entire file with a new copy available from the FSF.
> > 
> > This is something the upstream would need to fix; you may want to let them
> > know. Obviously not a review blocker.
> 
> I did notice upstream about this as well.

Thank you.

The package looks good to me now.

APPROVED
Comment 18 Ingvar Hagelund 2016-01-15 06:14:16 EST
(In reply to Lubomir Rintel from comment #17)
> (In reply to Ingvar Hagelund from comment #14)
> Ideally upstream would should distribute the service files along with the
> documentation on how to use them.
> 
> > Should I add a README.RedHat with a note
> > explaining this?
> 
> Yes, I think that's okay.

I included this the redhat initscript patch. I sent a note upstream on this as well.

> The package looks good to me now.
> APPROVED

Thank you very much for the review. I have requested new packages in the pkdb:

    user: ingvar request package: tayga on branch master
    user: ingvar request package: tayga on branch f23
    user: ingvar request package: tayga on branch f22
    user: ingvar request package: tayga on branch epel7
    user: ingvar request package: tayga on branch el6
    user: ingvar request package: tayga on branch el5


Ingvar
Comment 19 Gwyn Ciesla 2016-01-15 09:29:42 EST
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/tayga
Comment 20 Fedora Update System 2016-01-15 10:12:42 EST
tayga-0.9.2-3.el5 has been submitted as an update to Fedora EPEL 5. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fa11e5ba72
Comment 21 Fedora Update System 2016-01-15 10:12:42 EST
tayga-0.9.2-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-6782b896fa
Comment 22 Fedora Update System 2016-01-15 10:12:49 EST
tayga-0.9.2-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-75f83adc15
Comment 23 Fedora Update System 2016-01-15 10:12:51 EST
tayga-0.9.2-3.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-75f06b53c0
Comment 24 Fedora Update System 2016-01-15 10:12:55 EST
tayga-0.9.2-3.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-cf4194ca93
Comment 25 Ingvar Hagelund 2016-01-15 10:22:03 EST
Package built and requested for testing on all branches. Closing this as NEXTRELEASE.
Comment 26 Fedora Update System 2016-01-16 10:48:22 EST
tayga-0.9.2-3.el5 has been pushed to the Fedora EPEL 5 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-fa11e5ba72
Comment 27 Fedora Update System 2016-01-16 14:21:50 EST
tayga-0.9.2-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-6782b896fa
Comment 28 Fedora Update System 2016-01-16 14:25:12 EST
tayga-0.9.2-3.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-cf4194ca93
Comment 29 Fedora Update System 2016-01-16 15:49:34 EST
tayga-0.9.2-3.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-75f06b53c0
Comment 30 Fedora Update System 2016-01-17 09:24:00 EST
tayga-0.9.2-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-75f83adc15
Comment 31 Fedora Update System 2016-01-25 22:19:53 EST
tayga-0.9.2-3.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.
Comment 32 Fedora Update System 2016-01-26 13:27:34 EST
tayga-0.9.2-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
Comment 33 Fedora Update System 2016-01-31 02:21:46 EST
tayga-0.9.2-3.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.
Comment 34 Fedora Update System 2016-01-31 20:53:29 EST
tayga-0.9.2-3.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.
Comment 35 Fedora Update System 2016-02-01 04:56:39 EST
tayga-0.9.2-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

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