-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MODULES-5649 Do not install mod_fastcgi on el7 #1701
Conversation
| @@ -106,7 +106,11 @@ | |||
| default => 'mod_authz_ldap', | |||
| }, | |||
| 'authnz_pam' => 'mod_authnz_pam', | |||
| 'fastcgi' => 'mod_fastcgi', | |||
| 'fastcgi' => $::apache::version::distrelease ? { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this produce undefined results on other distros like Debian?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look here https://github.com/tphoney/puppetlabs-apache/blob/c891950899a35a4af79fb23ce123f29d6da3639c/manifests/params.pp#L62-L110 It is for centos only @ekohl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, my bad. It's not an early morning but it does feel like it ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its raining and miserable here, it isnt good. Thanks for looking over the PR. Much appreciated.
manifests/mod/fastcgi.pp
Outdated
| @@ -1,6 +1,8 @@ | |||
| class apache::mod::fastcgi { | |||
| include ::apache | |||
|
|
|||
| if ($::osfamily == 'Redhat' and $::operatingsystemrelease > '6') { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use versioncmp?
manifests/mod/fastcgi.pp
Outdated
| @@ -1,6 +1,8 @@ | |||
| class apache::mod::fastcgi { | |||
| include ::apache | |||
|
|
|||
| if ($::osfamily == 'Redhat' and $::operatingsystemrelease > '6') { | |||
| fail("mod_fastcgi is no longer supported on el7 and above.") | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do wonder if other distros will deprecate it too and if there should be a more generalized check but I think this could be fine for now and refactored later.
c891950
to
6d1c59e
Compare
6d1c59e
to
6c1ed75
Compare
MODULES-5649 Do not install mod_fastcgi on el7
MODULES-5649 Do not install mod_fastcgi on el7
No description provided.