Bug 683229 - rackup config provided by upstream distribution is Debian-specific
Summary: rackup config provided by upstream distribution is Debian-specific
Keywords:
Status: CLOSED INSUFFICIENT_DATA
Alias: None
Product: Fedora EPEL
Classification: Fedora
Component: puppet
Version: el5
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Jeroen van Meeuwen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-03-08 21:19 UTC by Steve Huff
Modified: 2014-02-08 16:04 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-02-08 16:04:32 UTC
Type: ---


Attachments (Terms of Use)
patch rackup config and puppet manifest for RHEL (3.98 KB, application/octet-stream)
2011-03-08 21:19 UTC, Steve Huff
no flags Details
patch rackup config and puppet manifest for RHEL (4.37 KB, patch)
2011-03-14 13:18 UTC, Steve Huff
no flags Details | Diff

Description Steve Huff 2011-03-08 21:19:15 UTC
Created attachment 483024 [details]
patch rackup config and puppet manifest for RHEL

Description of problem:

The puppet distribution since 0.25.0 has included a Rack config file (config.ru) to facilitate running puppetmasterd under mod_passenger or other Rack-compatible servers.  However, the provided puppet manifest (/usr/share/puppet/ext/rack/manifest.pp) and Apache config (/usr/share/puppet/ext/rack/files/apache2.conf) are designed for Debian systems, and thus following the documented install procedure on a RHEL system generates a number of errors.

Version-Release number of selected component (if applicable):
0.25.5-1.el5

How reproducible:
Install EPEL's puppet package, follow procedure documented in /usr/share/puppet/ext/rack/README

Steps to Reproduce:
1. configure EPEL repository
2. configure Passenger repo (http://passenger.stealthymonkeys.com/)
3. yum install puppet puppet-server mod_passenger
4. service puppetmaster start; service puppetmaster stop
4. puppet --verbose --modulepath /usr/share/puppet/ext/ /usr/share/puppet/ext/rack/manifest.pp
  
Actual results:
Puppet errors; unsatisfied package and service dependencies, missing files

Expected results:
Puppet configures mod_passenger and Apache appropriately.

Additional info:
I have attached a patch which modifies the Apache config and the manifest appropriately; I've tested this patch on CentOS 5.5 x86_64, with the mod_passenger-3.0.4-1.el5 package from the Passenger repo linked above.

I suspect that this issue will also be present on el6; however, since the Passenger repo doesn't yet provide packages for el6, I haven't tested that platform yet.

Comment 1 Steve Huff 2011-03-14 13:18:00 UTC
Created attachment 484176 [details]
patch rackup config and puppet manifest for RHEL

the previous patch lacked some crucial SSL configuration

Comment 2 Todd Zullinger 2011-03-14 15:00:29 UTC
Since we don't ship passenger (and can't AFAIK, since they bundle a number of libraries), I'm not sure it's worth the effort to maintain a patch against an example file.  If anything, making the changes in a way that is suitable for shipping upstream would be seem best.  I don't use passenger, so I'd have no way to test the patch or keep it updated in the future.  If any of the other maintainers use passenger and want to maintain such a patch, hopefully they'll speak up here.

Also, the upstream documentation has a instruction for using passenger at:

http://projects.puppetlabs.com/projects/1/wiki/Using_Passenger

I have no idea if those are more complete than what is in the tarball or not.  It does specifically deal with RHEL 5 and Debian differences though.  Working with upstream to update the included documentation in the tarball would be excellent, and would benefit everyone.

Comment 3 Steve Huff 2011-03-14 16:26:22 UTC
(In reply to comment #2)
> Since we don't ship passenger (and can't AFAIK, since they bundle a number of
> libraries), I'm not sure it's worth the effort to maintain a patch against an
> example file.  If anything, making the changes in a way that is suitable for
> shipping upstream would be seem best.  I don't use passenger, so I'd have no
> way to test the patch or keep it updated in the future.  If any of the other
> maintainers use passenger and want to maintain such a patch, hopefully they'll
> speak up here.

Fair enough.  While EPEL indeed doesn't ship Passenger, the upstream project
appears to have given their blessing to the following repo, which does ship
Passenger packages for EPEL's target platforms (and, indeed leverage existing
EPEL packages):

http://blog.phusion.nl/2011/01/04/phusion-passenger-native-packages-for-redhatfedoracentos/

Also, the rackup configuration is not restricted to Passenger, but rather
should work on any Rack-compatible webserver if I understand correctly, so
even if we take Passenger out of the equation, the issue still exists.

> Also, the upstream documentation has a instruction for using passenger at:
> 
> http://projects.puppetlabs.com/projects/1/wiki/Using_Passenger
> 
> I have no idea if those are more complete than what is in the tarball or not. 
> It does specifically deal with RHEL 5 and Debian differences though.  Working
> with upstream to update the included documentation in the tarball would be
> excellent, and would benefit everyone.

I spent a while reviewing this documentation prior to submitting this report;
in fact, what impelled me to submit this report is the fact that the first
line in those instructions directs the reader to the following README from the
puppet source tree:

http://github.com/puppetlabs/puppet/tree/master/ext/rack

Unfortunately, following those instructions on an EL system with the EPEL
puppet package results in a raft of errors, since the manifest shipped from
upstream is designed for Debian systems.  It's not unreasonably difficult to
figure out how to set everything up by hand, but I can't help but think that
it would be preferable for the EPEL package not to contain a config that is
broken out of the box.

I agree with you wholeheartedly that the optimum solution would be to merge
these changes upstream; a good solution would be not to change the
documentation, but rather to change the provided installation script, so that
one can safely follow the existing instructions on an EL system and get the
desired result.  I propose the following:

1) I'll develop a patch that creates a more general solution, and submit it
upstream.

2) In the interim, I'd be happy to maintain my patch against the EPEL puppet
package.  I have already started the process of applying to become an EPEL
maintainer; in fact, my first package submission
(https://bugzilla.redhat.com/show_bug.cgi?id=676007) is waiting for a
sponsorship.  Would you be willing to be my sponsor?  I have a few years of
experience maintaining RPMs, both in my professional life and over at
RPMforge.

thanks,  
-Steve

p.s. I'm in #epel right now if you'd like to discuss this further in real
time.

Comment 4 Todd Zullinger 2011-03-21 17:54:05 UTC
Steve, it looks like your patch takes puppet-2.6 into account.  Can you confirm that it works for 2.6.6-1 which is in Fedora and EPEL testing repositories?  (Also, why is the 'provider => yum' needed.  It should be the default package provider on Fedora/RHEL/CentOS already.)

Charles, thanks for the bodhi report.  Can you describe what sort of changes were required to config.ru for the 2.6 update?  If the changes are not intentional, upstream may fix them.  At the least, we can add details to the update notice.

Comment 5 Steve Huff 2011-03-21 19:09:05 UTC
(In reply to comment #4)
> Steve, it looks like your patch takes puppet-2.6 into account.  Can you
> confirm that it works for 2.6.6-1 which is in Fedora and EPEL testing
> repositories?  (Also, why is the 'provider => yum' needed.  It should be the
> default package provider on Fedora/RHEL/CentOS already.)

I can confirm this; I've tested it both with the EPEL puppet-2.6.6 and the RPMforge puppet-2.6.6 packages.  There's no change in any of the rack/passenger-related stuff between Puppet 2.6.5 and 2.6.6 iirc.

The 'provider => yum' looks superfluous to me; I don't remember why I added it, but it shouldn't be necessary.

-Steve

Comment 6 Todd Zullinger 2011-04-04 21:35:12 UTC
Thanks Steve.  Perhaps once 2.6.6 is pushed we can roll these patches into 2.6.7 (or .8 if that comes along soon) and start talking to upstream about merging this in some form that works for various distros.

Charles, Can you describe what sort of changes were required to config.ru for the 2.6 update?  If the changes are not intentional, upstream may fix them.  At the least, we can add details to the update notice.  Since I don't use passenger, I'd need some help.

Comment 7 Michael Stahnke 2011-05-17 04:24:22 UTC
Adding myself to this bug.  I think this can be arranged in the contrib directory or something similar.  I don't think anybody will mind an EL specific config and carrying one for Debian as well. 

Please submit upstream if you haven't already.  If you have, please add upstream issue number to this report. (I work for Puppet Labs)

Comment 8 Orion Poplawski 2011-08-23 17:31:01 UTC
The apache config should probably also have:

PassengerTempDir /var/run/passenger

Or perhaps that should go into some other apache config file?

Comment 9 Steve Huff 2011-08-23 18:08:08 UTC
(In reply to comment #8)
> The apache config should probably also have:
> 
> PassengerTempDir /var/run/passenger
> 
> Or perhaps that should go into some other apache config file?

Orion,  

I'd argue that a config like that is not Puppet-specific, and so should
go in /etc/httpd/conf.d/passenger.conf, which is provided by the
mod_passenger package.  This doesn't come from EPEL, but rather from the
stealthymonkeys repo (as referenced in comment #3).

I've found Erik (the maintainer) to be friendly and responsive.  Send
him a pull request :)

https://github.com/erikogan/passenger/tree/master/rpm

-steve

Comment 10 Steve Huff 2011-08-23 18:12:40 UTC
General status on this issue:

My initial patch ripped out a lot of the Debian-specific configuration
and replaced it with RHEL-specific configuration.  Before being able to
submit it upstream, I need to write and test a somewhat more
sophisticated version that will use the $operatingsystem fact
appropriately in order to be able to work on multiple distributions.

I'll get to it when I can find the time, and then submit it upstream,
notifying this ticket (and Michael in particular) once I've done so.

Thanks for your patience!

-Steve


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