(MODULES-6677) make sure event mpm is disabled

[eputnam]

Currently, on Debian 9 specifically, the event mod may be enabled when trying to use a different mpm. This ensures event is disabled if another mpm is chosen. Also closes #1766

[ekohl]

Isn't this basically equivalent to a file with ensure => absent or does a2dismod do more than that? In that case you could even generalize it to if $mpm != 'event' instead of the OS checks.

[eputnam]

I assumed it did something else but it looks like all it does is remove the symlink in mods-enabled.

I'm hesitant to generalize the conditional only because caution. However, I can't think of a situation where you wouldn't want to have event disabled if it wasn't the selected mpm. Further, maybe there should be logic to exclude any mpm that's not the selected one.

[ekohl]

Further, maybe there should be logic to exclude any mpm that's not the selected one.

Heading to directory purging here :)

[hunner]

Yes, that's basically symlink management.

The a2*mod commands actually were previously used by the apache module via the a2mod type (it's still there, deprecated and never removed; see https://github.com/puppetlabs/puppetlabs-apache/blob/3.0.0/lib/puppet/type/a2mod.rb). We tried writing providers to give similar functionality on several OSs, but it was very difficult to track all of the different changes across many OSs and each OS version.

The apache module was probably the first to specifically deviate from replicating OS defaults (RedHat-like on redhats and Debian-like on debian) and instead go for a one-size-fits-all. We did keep the mod symlink style via apache::mod but relied on recursive purging to keep everything inline inside the conf directory. This was 0.7.0 iirc.

What specifically causes the mpm module .load files to clash and not be purged? Is it a dependency between the mods and the file purging? Why do we have to exec out to a2*mod at all?

[mpdude]

Just hit this bug with Ubuntu 17.10, will give your PR a try

[mpdude]

Is there a simple :ref spec I can use in librarian-puppet to install a PR branch like this?

[ekohl]

@mpdude I guess mod 'puppetlabs-apache', :git => 'https://github.com/eputnam/puppetlabs-apache' :ref => 'mpm_event_disable_debian9_itk' should work

[mpdude]

👍 Works for me on Ubuntu 17.10.

[eputnam]

ran into some hard-to-get-around duplicate declarations when trying to use file resource, so I reverted back to the execs.

[maxdelorme]

If apache > 2.4 and prefork (required for php in my case) then Package[$packagename] is not installed
see https://github.com/eputnam/puppetlabs-apache/blob/mpm_event_disable_debian9_itk/manifests/mpm.pp#L106
and the before condition : https://github.com/eputnam/puppetlabs-apache/blob/mpm_event_disable_debian9_itk/manifests/mpm.pp#L88 can not be verified

resulting in error :
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Could not find resource 'Package[apache2-mpm-prefork]' in parameter 'before' (file: /etc/puppetlabs/code/environments/<my_env>/modules/apache/manifests/mpm.pp, line: 88) on node <my_node>

[mpdude]

Is there anything I can do to help this getting merged?

Update: Just gave Ubuntu 18.04 a try and this PR does not seem to do the trick.

[zivis]

Is there anything i can do, to help getting this or this: #1766 merged?
If something is not okay with neither of those solutions we can work on it…

[cmseal]

This doesn't include a condition for 'prefork' on Ubuntu 18.04... could this be included after line 75:

      if $mpm == 'prefork' and ( ( $::operatingsystem == 'Ubuntu' and $::operatingsystemrelease == '18.04' ) or ( $::operatingsystem == 'Debian' and versioncmp($::operatingsystemrelease, '9.0.0') >= 0 ) ) {
         exec { '/usr/sbin/a2dismod mpm_event':
           onlyif  => "/usr/bin/test -e ${mod_enabled_dir}/mpm_event.load",
           require => Package['httpd'],
           before  => Package[$packagename],
         }
       }

[zivis]

@mpdude @cmseal @maxdelorme i have reworked #1766 you can have a look at.
It should work for the scenarios mentioned here.

[HelenCampbell]

Closing due to #1766 being merged. Thank you everyone for your work and input!