puppet-php icon indicating copy to clipboard operation
puppet-php copied to clipboard

Added option to notify httpd service for mod_php

Open thorstenk opened this issue 7 years ago • 8 comments

Pull Request (PR) description

If php::settings are changed and Apaches mod_php is used the Apache httpd must be restarted to re-read the config files.

This PR adds an option to identify the name of the Puppet Service resource to enable a restart of Apache httpd.

thorstenk avatar Jul 10 '18 13:07 thorstenk

Can you please add tests for the change and rebase after https://github.com/voxpupuli/puppet-php/pull/469 got merged? That will fix the current acceptance tests issues on travis.

bastelfreak avatar Aug 11 '18 21:08 bastelfreak

I have rebased my pull request which seems to fix the Travis tests.

I have no experience in writing Puppet tests and none of the existing tests gave me a hint how to test a service restart after a configuration change. As long as usage of mod_php is declared unsupported a test for this specific feature might be less important.

thorstenk avatar Aug 14 '18 13:08 thorstenk

I had something like this in mind:

 spec/acceptance/php_spec.rb | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/spec/acceptance/php_spec.rb b/spec/acceptance/php_spec.rb
index e00c743..426185c 100644
--- a/spec/acceptance/php_spec.rb
+++ b/spec/acceptance/php_spec.rb
@@ -3,7 +3,8 @@ require 'spec_helper_acceptance'
 describe 'php with default settings' do
   context 'default parameters' do
     it 'works with defaults' do
-      pp = 'include php'
+      install_module_from_forge('puppetlabs/apache', '>= 3.2.0 < 4.0.0')
+      pp = 'include apache; include php'
       # Run it twice and test for idempotency
       apply_manifest(pp, catch_failures: true)
       apply_manifest(pp, catch_changes: true)
@@ -32,4 +33,18 @@ describe 'php with default settings' do
       it { is_expected.to be_enabled }
     end
   end
+
+  context 'with apache' do
+    it 'works' do
+      pp = <<-EOS
+      class { 'php':
+        apache_config       => true,
+        apache_service_name => 'httpd',
+      }
+      EOS
+      # Run it twice and test for idempotency
+      apply_manifest(pp, catch_failures: true)
+      apply_manifest(pp, catch_changes: true)
+    end
+  end
 end

bastelfreak avatar Aug 19 '18 16:08 bastelfreak

@bastelfreak Thanks for the hint. I'll try that.

thorstenk avatar Aug 20 '18 08:08 thorstenk

hey @thorstenk. Any update on this?

bastelfreak avatar Oct 06 '18 21:10 bastelfreak

Dear @thorstenk, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

vox-pupuli-tasks[bot] avatar Jan 04 '20 12:01 vox-pupuli-tasks[bot]

Dear @thorstenk, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

vox-pupuli-tasks[bot] avatar Jan 04 '20 12:01 vox-pupuli-tasks[bot]

Dear @thorstenk, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

vox-pupuli-tasks[bot] avatar Jan 15 '20 12:01 vox-pupuli-tasks[bot]