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

Don't purge mods-available dir when separate enable dir is used #561

Merged
merged 1 commit into from
Jan 24, 2014

Conversation

domcleal
Copy link
Contributor

@domcleal domcleal commented Jan 8, 2014

Purging /etc/apache2/mods-available on Debian when there are separate available/enabled directories is poor behaviour, since this isn't active configuration. When purged, you lose track of which modules are available but that this module doesn't handle.

This patch disables the purge behaviour on the module config directory, but only when there is also a separate module enabled directory.

@igalic
Copy link
Contributor

igalic commented Jan 9, 2014

is this sort of something that you could cover by specifying your default_mods?
Oh, funny. I just realized I didn't document that you can actually specify an array of modules in that option.
I wonder how @apenney didn't notice this :O

@domcleal
Copy link
Contributor Author

domcleal commented Jan 9, 2014

Yes, it's possible to enable a module via that or apache::mod, that's not an issue.

My concern was that the current behaviour deletes example configuration files supplied by the apache2 package on Debian. I'd run this module in the default config on Ubuntu 12.04, then found it deleted the contents of /etc/apache2/mods-available, so "a2enmod version" didn't work as the .load file had been deleted from mods-available.

Sure, I can add the version module in using the module and it will recreate the .load file, but since the mods-available directory contains non-active configuration files, I don't think the module needs to delete them, it's unnecessary.

@igalic
Copy link
Contributor

igalic commented Jan 9, 2014

+1 on that, you're absolutely right.

@igalic
Copy link
Contributor

igalic commented Jan 9, 2014

Would you mind adding a test to acceptance/spec_class.rb also?

@domcleal
Copy link
Contributor Author

domcleal commented Jan 9, 2014

The acceptance test nodeset only seem to contain an EL6 node, so I don't think we could test it as-is. There's already some spec coverage, so I'm not sure it's worth the effort of me trying to set up beaker!

@igalic
Copy link
Contributor

igalic commented Jan 9, 2014

I'd disagree with that notion: https://github.com/puppetlabs/puppetlabs-apache/tree/master/spec/acceptance/nodesets you just need to change the default to test a different platform.

@domcleal
Copy link
Contributor Author

domcleal commented Jan 9, 2014

I see, though I'm unable to run Beaker as it doesn't work with vagrant-libvirt at the moment (a rabbit hole I don't have time for now, sorry). Hopefully the rspec-puppet coverage will suffice.

@blkperl
Copy link
Contributor

blkperl commented Jan 24, 2014

Restarted travis

@igalic
Copy link
Contributor

igalic commented Jan 24, 2014

Yeah, let's say it works for now, and we open an issue to add more tests.

igalic added a commit that referenced this pull request Jan 24, 2014
Don't purge mods-available dir when separate enable dir is used
@igalic igalic merged commit 2bfda40 into puppetlabs:master Jan 24, 2014
@domcleal
Copy link
Contributor Author

Thank you @igalic :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants