Bug 228544 - Review Request: keepalived - HA monitor built upon LVS, VRRP and service pollers
Review Request: keepalived - HA monitor built upon LVS, VRRP and service pollers
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dan Horák
Fedora Package Reviews List
:
: 499023 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-13 13:22 EST by Matthias Saou
Modified: 2009-06-03 11:09 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-27 12:55:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dan: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matthias Saou 2007-02-13 13:22:35 EST
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/keepalived/
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/keepalived/
Description:
The main goal of the keepalived project is to add a strong & robust keepalive
facility to the Linux Virtual Server project. This project is written in C with
multilayer TCP/IP stack checks. Keepalived implements a framework based on
three family checks : Layer3, Layer4 & Layer5/7. This framework gives the
daemon the ability to check the state of an LVS server pool. When one of the
servers of the LVS server pool is down, keepalived informs the linux kernel via
a setsockopt call to remove this server entry from the LVS topology. In
addition keepalived implements an independent VRRPv2 stack to handle director
failover. So in short keepalived is a userspace daemon for LVS cluster nodes
healthchecks and LVS directors failover.
Comment 1 Dan Horák 2007-02-13 14:35:25 EST
I have tried to rebuild it on FC6 system with 2 kernels and 2 kernel-devels and
it is confused. The include path is
-I/lib/modules/2.6.18-1.2869.fc62.6.19-1.2895.fc6/build/include. There is a
missing \n in the query format.
Comment 2 Matthias Saou 2007-02-13 14:47:31 EST
Thanks for the catch! Fixed in keepalived-1.1.13-4.fc6.
Comment 3 Dan Horák 2007-02-13 14:48:53 EST
Also the debug package is empty on FC6. The cause can be Makefile in the
keepalived subdir where the binary is linked together. There is no -g in the
$(CC) command.
Comment 4 Dan Horák 2007-02-13 14:54:17 EST
(In reply to comment #2)
> Thanks for the catch! Fixed in keepalived-1.1.13-4.fc6.

Double backslash required ;-) I have tried it ;-)
Comment 5 Matthias Saou 2007-02-13 14:55:49 EST
Oops, I noticed the \\n problem seconds after having send the new spec file...
I've overwritten keepalived-1.1.13-4.fc6 with a version where both the \\n and
the stripping problem are fixed.
Comment 6 Dan Horák 2007-02-13 15:57:13 EST
Full review is here:

OK	source files match upstream:
	    2545bd681580a97f9c5c9bbe6fe2f8a91988d0c5f063bba048148b52ccde2568 
keepalived-1.1.13.tar.gz
OK	package meets naming and versioning guidelines.
OK	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
OK	build root is correct.
OK	license field matches the actual license.
OK	license is open source-compatible. License text included in package.
OK	latest version is being packaged.
OK	BuildRequires are proper.
OK	compiler flags are appropriate.
OK	%clean is present.
OK	debuginfo package looks complete.
OK	rpmlint is silent.
BAD	final provides and requires don´t look sane:
		the scriptlet Requires are missing

Requires(post): /sbin/chkconfig
Requires(preun): /sbin/chkconfig
Requires(preun): /sbin/service

OK	%check is present and all tests pass:
OK	no shared libraries are added to the regular linker search paths.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
OK	scriptlets are present and are sane.
OK	code, not content.
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	no headers.
OK	no pkgconfig files.
OK	no libtool .la droppings.
OK	not a GUI app.

after you will add the scriptlet Requires, then the package is APPROVED
Comment 7 Dan Horák 2007-02-13 16:47:26 EST
And finally I have updated my rawhide mirror and here is the result from a mock
rebuild:

gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -I/lib/modules/2.6.20-1.2922.fc7/build/include
-I../include -I../../lib -Wall -Wunused -Wstrict-prototypes -D_KRNL_2_6_
-D_WITHOUT_LINKWATCH_ -D_WITH_LVS_ -D_HAVE_IPVS_SYNCD_  -c vrrp_netlink.c
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables  -I/lib/modules/2.6.20-1.2922.fc7/build/include
-I../include -I../../lib -Wall -Wunused -Wstrict-prototypes -D_KRNL_2_6_
-D_WITHOUT_LINKWATCH_ -D_WITH_LVS_ -D_HAVE_IPVS_SYNCD_  -c vrrp_arp.c
In file included from /usr/include/net/ethernet.h:26,
                 from ../include/vrrp_arp.h:29,
                 from vrrp_arp.c:29:
/usr/include/sys/types.h:62: error: conflicting types for 'dev_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:22: error: previous
declaration of 'dev_t' was here
/usr/include/sys/types.h:67: error: conflicting types for 'gid_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:54: error: previous
declaration of 'gid_t' was here
/usr/include/sys/types.h:72: error: conflicting types for 'mode_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:24: error: previous
declaration of 'mode_t' was here
/usr/include/sys/types.h:77: error: conflicting types for 'nlink_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:25: error: previous
declaration of 'nlink_t' was here
/usr/include/sys/types.h:82: error: conflicting types for 'uid_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:53: error: previous
declaration of 'uid_t' was here
In file included from /usr/include/sys/types.h:133,
                 from /usr/include/net/ethernet.h:26,
                 from ../include/vrrp_arp.h:29,
                 from vrrp_arp.c:29:
/usr/include/time.h:105: error: conflicting types for 'timer_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:31: error: previous
declaration of 'timer_t' was here
In file included from /usr/include/sys/types.h:220,
                 from /usr/include/net/ethernet.h:26,
                 from ../include/vrrp_arp.h:29,
                 from vrrp_arp.c:29:
/usr/include/sys/select.h:78: error: conflicting types for 'fd_set'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:21: error: previous
declaration of 'fd_set' was here
In file included from /usr/include/net/ethernet.h:26,
                 from ../include/vrrp_arp.h:29,
                 from vrrp_arp.c:29:
/usr/include/sys/types.h:235: error: conflicting types for 'blkcnt_t'
/lib/modules/2.6.20-1.2922.fc7/build/include/linux/types.h:152: error: previous
declaration of 'blkcnt_t' was here
vrrp_netlink.c: In function 'netlink_socket':
vrrp_netlink.c:89: warning: pointer targets in passing argument 3 of
'getsockname' differ in signedness
vrrp_arp.c: In function 'send_gratuitous_arp':
vrrp_arp.c:84: warning: pointer targets in initialization differ in signedness
make[2]: *** [vrrp_arp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/builddir/build/BUILD/keepalived-1.1.13/keepalived/vrrp'
make[1]: *** [all] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/keepalived-1.1.13/keepalived'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.83976 (%build)

This needs to be fixed too. So the approval is on hold.
Comment 8 David Woodhouse 2007-03-22 09:43:02 EDT
Including <linux/if_packet.h> _after_ the normal includes seems to fix the problem.
Comment 9 Matthias Saou 2007-03-22 11:30:25 EDT
(In reply to comment #8)
> Including <linux/if_packet.h> _after_ the normal includes seems to fix the
problem.

Indeed it does. Thanks for finding the "fix"!

New release :
http://ftp.es6.freshrpms.net/tmp/extras/keepalived/keepalived.spec
http://ftp.es6.freshrpms.net/tmp/extras/keepalived/keepalived-1.1.13-5.src.rpm
Comment 10 Dan Horák 2007-03-26 03:52:07 EDT
rpmlint gives a permission error:
E: keepalived non-readable
/usr/share/doc/keepalived-1.1.13/samples/sample.misccheck.smbcheck.sh 0600
-> this should be 0644
Comment 12 Dan Horák 2007-03-26 07:29:06 EDT
Looks fine, APPROVED
Comment 13 Matthias Saou 2007-03-26 07:33:47 EDT
New Package CVS Request
=======================
Package Name: keepalived
Short Description: HA monitor built upon LVS, VRRP and service pollers
Owners: matthias@rpmforge.net
Branches: devel FC-6 FC-5 EL-5 EL-4 (all current)
InitialCC: 
Comment 14 Jarod Wilson 2008-05-12 15:41:41 EDT
I see an EL5 branch was requested, but I don't see packages built... From what I
understand, there may be some kernel-level stuff required. Can anyone shed some
light on what we need to do to get this built for EL5?
Comment 15 Matthias Saou 2008-05-13 05:30:59 EDT
(In reply to comment #14)
> I see an EL5 branch was requested, but I don't see packages built... From what I
> understand, there may be some kernel-level stuff required. Can anyone shed some
> light on what we need to do to get this built for EL5?

Yes, it was kernel package related : The build requires both kernel and
kernel-devel in order to get the LVS header it requires, and these weren't
available in EPEL AFAIR.
Comment 16 Ruben Kerkhof 2009-06-03 06:34:16 EDT
*** Bug 499023 has been marked as a duplicate of this bug. ***
Comment 17 Ruben Kerkhof 2009-06-03 11:09:41 EDT
(in reply to comment #15)
keepalived builds fine on EL-4 and EL-5 (for me at least), but it fails to build in a EL-4 mockroot on a rawhide host. If you look at the mocks root.log you see that kernel-devel is installed, but somehow the rpm query in the spec doesn't pick it up.

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