Bug 655582 - Review Request: mod_cluster - Apache HTTPD based load balancer
Summary: Review Request: mod_cluster - Apache HTTPD based load balancer
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrick Monnerat
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-11-21 20:11 UTC by Marek Goldmann
Modified: 2011-03-27 16:22 UTC (History)
3 users (show)

Fixed In Version: mod_cluster-1.1.1-2.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-17 03:24:35 UTC
Type: ---
patrick: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)
Avoid most of the compilation warnings (11.08 KB, patch)
2011-03-11 13:48 UTC, Patrick Monnerat
no flags Details | Diff

Description Marek Goldmann 2010-11-21 20:11:26 UTC
Spec URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster.spec
SRPM URL: http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster-1.1.0.Final-1.fc14.src.rpm
Description:

mod_cluster is an httpd-based load balancer. Like mod_jk and mod_proxy, mod_cluster uses a communication channel to forward requests from httpd to one of a set of application server nodes. Unlike mod_jk and mod_proxy, mod_cluster leverages an additional connection between the application server nodes and httpd. The application server nodes use this connection to transmit server-side load balance factors and lifecycle events back to httpd via a custom set of HTTP methods, affectionately called the Mod-Cluster Management Protocol (MCMP). This additional feedback channel allows mod_cluster to offer a level of intelligence and granularity not found in other load balancing solutions.

Scratch build for rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=2614547

Comment 1 Patrick Monnerat 2011-03-04 12:02:49 UTC
rpmlint output:

*** mod_cluster.spec:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

*** mod_cluster-1.1.0.Final-1.fc14.src.rpm:
mod_cluster.src: W: name-repeated-in-summary C Mod_cluster
mod_cluster.src: W: spelling-error %description -l en_US httpd -> HTTP
mod_cluster.src: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.src: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.src: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 5 warnings.

*** mod_cluster-1.1.0.Final-1.fc14.x86_64.rpm:
mod_cluster.x86_64: W: name-repeated-in-summary C Mod_cluster
mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

*** mod_cluster-debuginfo-1.1.0.Final-1.fc14.x86_64.rpm:
mod_cluster-debuginfo.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Note: URL is accepted when accessed from Firefox.

Comment 2 Patrick Monnerat 2011-03-04 14:48:57 UTC
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2884588

Some "first sight" remarks:

_ No alpha allowed in version for "final" releases.
  You should use version "1.1.0" and adapt the source URL accordingly.
_ Summary should be reworked, at least in compliance with rpmlint output.
_ Last upstream version is now 1.1.1
_ There are some bundled software in the source tarball (httpd, apr, ssl, etc):
  although you explicitly avoid using them, I would advise to remove the
  corresponding subdirectories before configure:
find srclib -mindepth 1 -maxdepth 1 ! -name mod_cluster -print0|xargs -0 -r rm -rf
  this will cause compilation failure whenever a bundled directory is used.
_ You do not use RPM_OPT_FLAGS as a base for the compilation flags.
_ There are compilation warnings that do indicate potential problems
  (format descriptor/value type mismatch). Consider writing a patch for those.
_ You should include the license file srclib/mod_cluster/lgpl.txt as %doc.

Comment 3 Marek Goldmann 2011-03-05 07:12:18 UTC
Thank you for this review, I'll prepare a new src.rpm including your comments shortly!

Comment 4 Marek Goldmann 2011-03-10 13:34:52 UTC
Spec URL:
http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster.spec
SRPM URL:
http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster-1.1.1-1.fc14.src.rpm

I fixed described issues. Re warnings - I'm not a C programmer, will contact mod_cluster developers to fix it in the future versions if that's ok.

rpmlint output:

$ rpmlint /var/lib/mock/fedora-14-x86_64/result/mod_cluster-1.1.1-1.fc14.x86_64.rpm 
mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

$ rpmlint /var/lib/mock/fedora-14-x86_64/result/mod_cluster-1.1.1-1.fc14.src.rpm 
mod_cluster.src: W: spelling-error %description -l en_US httpd -> HTTP
mod_cluster.src: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.src: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.src: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2900512

Thanks!

Comment 5 Patrick Monnerat 2011-03-11 12:06:29 UTC
rpmlint output:

*** mod_cluster.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

*** mod_cluster-1.1.1-1.fc14.src.rpm
mod_cluster.src: W: spelling-error %description -l en_US httpd -> HTTP
mod_cluster.src: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.src: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.src: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

*** mod_cluster-1.1.1-1.fc14.x86_64.rpm
mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

*** mod_cluster-debuginfo-1.1.1-1.fc14.x86_64.rpm
mod_cluster-debuginfo.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


+=OK, -=Not OK, X=Not applicable, ?=Not verifiable

MUST Items:
+ rpmlint output OK (see above)
+ named and versioned according to the Package Naming Guidelines
+ spec file name matches base package name
- dist tag is present and OK
+ complies with all the legal guidelines:
+  License: xyz valid, matches actual license
+  No known patent problems
+  No emulator, no firmware, no binary-only or prebuilt components
+  COPYING packaged as %doc
+ spec file is legible and written in american english
+ source matches upstream:
MD5: 3d4fec37ecfd1e5ec33cd147525cfe5e
SHA1: 2e42674ac8a5e43fba1b78a04b0087d9772e3afc
SHA256: 368a1db30abd0d7c4910e677f7eed4a5c0b1ca2c661cfb76ec6dee820b6c0058
+ latest version is being packaged
+ build root is correct
+ builds on at least one primary architecture
X known non-working architectures are listed in ExcludeArch (BZ #)
+ no missing BuildRequires (builds in mock)
X complies with translation/locale guidelines
X ldconfig calls in %post and %postun for packages containing shared libraries
+ no shared libraries are added to the regular linker search paths
+ no duplicated system libraries
+ package not relocatable
+ owns the directories and files it creates
+ doesn't own and directory it shouldn't
+ no duplicate files in %files
+ permissions correct, defattr used correctly
+ macros used consistently
+ no non-code content
X large documentation files are in a -doc subpackage
+ no %doc files required at runtime
X header files are in a -devel subpackage
X static libraries are in a -static subpackage
X suffixed library files have a matching .so file in the -devel subpackage
X pkgconfig files are in a -devel subpackage
X -devel package requires the base package using a fully versioned dependency:
  Requires: %{name} = %{version}-%{release}"
+ no .la files
X .desktop file present
X desktop-file-install is used in %install and the .desktop file passes
  validation
+ all filenames are valid UTF-8
+ complies with the FHS
- proper changelog, tags, BuildRoot, BuildRequires, Summary, Description
+ no macros in Summary and Description
+ no non-UTF-8 characters
- all relevant documentation included as %doc
+ compiler flags are appropriate (RPM_OPT_FLAGS are used)
+ %clean is present
+ no bundled software
+ debuginfo package is valid
+ no rpaths
+ complies to %config guidelines
X complies with init script guidelines
- no timestamp-clobbering file commands
- _smp_mflags used
X complies to web application guideline
X %check is present and all test pass
+ final provides and requires are sane
X no conflicts (installs properly)

SHOULD Items:
+ license already included upstream
X translations for description and summary are provided by upstream
+ package builds in mock
? submitter reports having successfully tested the package functionality
X scriptlets are sane
X subpackages other than -devel should require the base package using a
  fully versioned dependency
+ no file dependencies
X package contains man pages


Comments:
_ Use %global instead of %define
_ Release: 1%{dist} should be 1%{?dist}
_ You must add your name before the e-mail address in the changelog records:
  http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
_ Since there is a docs directory in the source, there is probably something
  to package as %doc. It seems to be a docbook, thus some processing is
  needed to generate ready-to-read files for the doc package (i.e.:
  html).
_ Use "cp -a" to preserve files timestamp.
_ Use "make %{?_smp_mflags}" or explain why you don't.
  http://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make


About the printf format warnings problem: in any case you should report it upstream. In the meantime, and since the review policy does not explicitly forbid a reviewer to do so, I can try to produce such a patch for you. Your opinion ?

Comment 6 Marek Goldmann 2011-03-11 12:19:18 UTC
Yes, please any help is appreciated. I mentioned these errors to developers yesterday, in a mail.

I'll fix my mistakes in the meantime, waiting for your patch. Thank you!

Comment 7 Patrick Monnerat 2011-03-11 13:48:19 UTC
Created attachment 483718 [details]
Avoid most of the compilation warnings

I compiled successfully with this patch on F14 i386 and x86_64.
I still have one warning after applying this patch: I did not fix it because the fix might cause problems when compiling for M$W :-(
Please note the following:
1) I did NOT test the resulting binaries (no infrastructure to do it here). I'm really expecting you can and would do it.
2) I'm sure about fixes dealing with output formats.
3) I'm only pretty confident about fixes for undefined variables: the way I defined them seems the most probable, but I'm only 95% sure. Anyway it cannot be worse than not defined them at all!
4) Other fixes are suppressions of unused code and variables: those should be 100% sure too.

Comment 8 Marek Goldmann 2011-03-11 17:02:05 UTC
Patrick,

I fixed almost everything you found and applied your patch, works great. I have tested it locally, mod_cluster loads perfectly and mod_cluster-manager shows up without issues. I would like to test it more by the community in real-world scenarios (updates-testing).

I've notified upstream developers about your patch.

I haven't added the documentation because it requires maven3 to build. I failed building it with maven2.

Thank you for your work!

$ rpmlint mod_cluster.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint ./mod_cluster-1.1.1-2.fc14.x86_64.rpm 
mod_cluster.x86_64: W: spelling-error %description -l en_US jk -> j, k, ja
mod_cluster.x86_64: W: spelling-error %description -l en_US lifecycle -> life cycle, life-cycle, lifestyle
mod_cluster.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

$ rpmlint ./mod_cluster-debuginfo-1.1.1-2.fc14.x86_64.rpm 
mod_cluster-debuginfo.x86_64: W: invalid-url URL: http://jboss.org/mod_cluster HTTP Error 403: Forbidden
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Spec URL:
http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster.spec
SRPM URL:
http://goldmann.fedorapeople.org/package_review/mod_cluster/mod_cluster-1.1.1-2.fc14.src.rpm

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2904647

Comment 9 Patrick Monnerat 2011-03-11 17:31:24 UTC
> I would like to test it more by the community in real-world scenarios (updates-testing).

Sure, I would too!

> I haven't added the documentation because it requires maven3 to build. I failed
building it with maven2.

That's a very good reason !
Perhaps some comments telling it in the %build and %files section, as well as a commented dummy BuildRequires wouldn't be too much...

APPROVED

Comment 10 Marek Goldmann 2011-03-11 17:51:58 UTC
Thank you for the review!

I'll add the comments to final package.

New Package SCM Request
=======================
Package Name: mod_cluster
Short Description: Apache HTTPD based load balancer
Owners: goldmann
Branches: f13 f14 f15 el6

Comment 11 Jason Tibbitts 2011-03-11 19:29:27 UTC
Git done (by process-git-requests).

Comment 12 Fedora Update System 2011-03-12 09:28:22 UTC
mod_cluster-1.1.1-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.el6

Comment 13 Fedora Update System 2011-03-12 09:29:06 UTC
mod_cluster-1.1.1-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.fc14

Comment 14 Fedora Update System 2011-03-12 09:29:47 UTC
mod_cluster-1.1.1-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.fc13

Comment 15 Fedora Update System 2011-03-12 09:38:26 UTC
mod_cluster-1.1.1-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/mod_cluster-1.1.1-2.fc15

Comment 16 Marek Goldmann 2011-03-12 09:39:23 UTC
Thank you for review and git process. Packages submitted to updates-testing.

Comment 17 Fedora Update System 2011-03-12 20:58:01 UTC
mod_cluster-1.1.1-2.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 18 Fedora Update System 2011-03-17 03:24:29 UTC
mod_cluster-1.1.1-2.fc15 has been pushed to the Fedora 15 stable repository.

Comment 19 Fedora Update System 2011-03-20 21:23:45 UTC
mod_cluster-1.1.1-2.fc14 has been pushed to the Fedora 14 stable repository.

Comment 20 Fedora Update System 2011-03-20 21:27:45 UTC
mod_cluster-1.1.1-2.fc13 has been pushed to the Fedora 13 stable repository.

Comment 21 Fedora Update System 2011-03-27 16:22:46 UTC
mod_cluster-1.1.1-2.el6 has been pushed to the Fedora EPEL 6 stable repository.


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