Added option to notify httpd service for mod_php

[thorstenk]

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.

[bastelfreak]

Could you update this to undef and make it optional? nil is pretty uncommon in Puppet and we try to stick with undef.

[thorstenk]

Thanks for the hint. Should be fixed now.

[bastelfreak]

Can you please add tests for the change and rebase after #469 got merged? That will fix the current acceptance tests issues on travis.

[thorstenk]

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.

[bastelfreak]

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

[thorstenk]

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

[bastelfreak]

hey @thorstenk. Any update on this?

[vox-pupuli-tasks]

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]

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]

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