Skip to content
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

Support for mod_php on Ubuntu 22.04 #2278

Closed

Conversation

xchotard-talentsoft
Copy link

Summary

Fixes #2259

Changes

  • Added PHP version for Ubuntu 22.04 (and 16.04) to the $php_version mapping in params.pp.
  • In mod/php.pp added conditions for Red Hat family that has a specific behavior with PHP 8.x modules name convention.

Tests

Tested with the following Puppet code:

class { 'apache':
  mpm_module => 'prefork',
}
class { 'apache::mod::php': }

To validate that PHP works:

# Don't do this in production servers!
echo "<?php phpinfo(); ?>" > /var/www/html/phpinfo.php

Then open http://<sandbox_ip>/phpinfo.php

Works on:

  • Ubuntu 16.04
  • Ubuntu 18.04
  • Ubuntu 20.04
  • Ubuntu 22.04
  • CentOS 7
  • CentOS 8
  • Debian 11

I could not test SLES because it's not free. I tried openSuse Leap 15.4 but the module does not seems to work for other reasons.

@xchotard-talentsoft xchotard-talentsoft requested a review from a team as a code owner August 5, 2022 08:36
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@puppet-community-rangefinder
Copy link

apache::mod::php is a class

Breaking changes to this file WILL impact these 49 modules (exact match):
Breaking changes to this file MAY impact these 36 modules (near match):

apache::params is a class

Breaking changes to this file WILL impact these 7 modules (exact match):
Breaking changes to this file MAY impact these 4 modules (near match):

This module is declared in 174 of 579 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@ekohl
Copy link
Collaborator

ekohl commented Aug 5, 2022

Relevant previous effort: #2163

Tested with the following Puppet code:

class { 'apache':
  mpm_module => 'prefork',
}
class { 'apache::mod::php': }

To validate that PHP works:

# Don't do this in production servers!
echo "<?php phpinfo(); ?>" > /var/www/html/phpinfo.php

Then open http://<sandbox_ip>/phpinfo.php

This would be a great acceptance test. In fact, there already is https://github.com/puppetlabs/puppetlabs-apache/blob/main/spec/acceptance/mod_php_spec.rb but that doesn't run anything like curl to see if it actually renders.

@david22swan
Copy link
Member

@xchotard-talentsoft If you could add Ubuntu 22.04 to the metadata that should activate the tests against it to double-check this change.
From my own pr added Certification for the OS it look's like yours should fix all outstanding issues.

@xchotard-talentsoft
Copy link
Author

@xchotard-talentsoft If you could add Ubuntu 22.04 to the metadata that should activate the tests against it to double-check this change. From my own pr added Certification for the OS it look's like yours should fix all outstanding issues.

Done, but I haven't tested any of the other apache modules.

@@ -349,7 +349,9 @@
$php_version = $facts['os']['release']['major'] ? {
'9' => '7.0', # Debian Stretch
'10' => '7.3', # Debian Buster
'16.04' => '7.0', # Ubuntu Xenial Xerus
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ubuntu 16.04 is EOL and unsupported. Please keep it out.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind if I remove this commit and do a force push ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at all. In fact, that's what I do all the time.

'20.04' => '7.4', # Ubuntu Foccal Fossal
'22.04' => '8.1', # Ubuntu Jellyfish
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd imagine 22.04 is closer to Debian Bullseye. It's mostly about the mod_packages list. So I'd suggest to change line 348 with a versioncmp. Possibly Ubuntu 20.04 is also more like Debian Bullseye than this list.

Copy link
Author

@xchotard-talentsoft xchotard-talentsoft Aug 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit I don't know exactly what the versioncmp() does here. I would use the following block code instead (from L348 to L355:

if ($facts['os']['family'] == 'Debian') {
  $php_version = $facts['os']['release']['major'] ? {
    '9'      => '7.0',  # Debian Stretch
    '10'     => '7.3',  # Debian Buster
    '11'     => '7.4',  # Debian Bullseye
    '18.04'  => '7.2',  # Ubuntu Bionic
    '20.04'  => '7.4',  # Ubuntu Focal
    '22.04'  => '8.1',  # Ubuntu Jammy
    default  => '7.4',  # Debian Bullseye, Ubuntu Focal
  }
...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the $php_version part should be pulled out of the versioncmp and I swear I considered the exact same thing but failed to write it down. However, the list of modules can use a versioncmp. It's still not entirely correct, like #2281 should clean things up further.

@david22swan
Copy link
Member

david22swan commented Aug 17, 2022

@xchotard-talentsoft
Hey, as part of my scheduled work to get all our our modules to support Ubuntu 22.04 have finalized the work started by you and added it to the pdksync PR shown here and was hoping you could give it a quick review.

Hope there's no offense taken, just need to get this done so I can move onto my next task.
Pinging @ekohl as well

@david22swan
Copy link
Member

@xchotard-talentsoft Closing this as the need has bee filled by another PR.
If there is anything else that you wanted to get in with it please re-open and comment.

Thank you for all your work

@david22swan david22swan closed this Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP module not working on Ubuntu 22.04 / Jammy
5 participants