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

MODULES-1309 - Make package and service names configurable #436

Merged
merged 1 commit into from Dec 3, 2014

Conversation

underscorgan
Copy link
Contributor

This was motivated by a need to make this work on Debian Jessie.

if $::operatingsystemrelease =~ /^6\./ and versioncmp($::iptables_persistent_version, '0.5.0') < 0 {
$service = undef
$package = 'iptables-persistent'
} elsif $::operatingsystem == 'Debian' and versioncmp($::operatingsystemrelease, '8') >= 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect that line 31 of firewall::params will not do what we intend in my environment.

# facter -p operatingsystemrelease
jessie/sid 

also, do we need to account for both packages?

# dpkg -l | grep iptables
ii  iptables                       1.4.21-2+b1                   amd64        administration tools for packet filtering and NAT
ii  iptables-persistent            1.0.2                         all          boot-time loader for netfilter rules, iptables plugin

@hesco
Copy link
Contributor

hesco commented Dec 3, 2014

Thanks Morgan:

You wound up touching a few more files than I had imagined on first blush with this issue.
I reviewed the patch thoroughly, and other than the notes above, (and the fact that I have
run no tests) this looks like it ought to get the job done. I assume this is also slated for 1.3.0?
Looking forward to seeing it soon.

-- Hugh

@underscorgan underscorgan force-pushed the MODULES-1309 branch 2 times, most recently from 615cdd2 to d598280 Compare December 3, 2014 22:26
@underscorgan
Copy link
Contributor Author

@hesco versioncmp is magic and knows that jessie/sid >= 8.0 :)

and the iptables package is defined at https://github.com/puppetlabs/puppetlabs-firewall/blob/master/manifests/linux.pp#L22-L24, so we should be good for that. I think the general thought is that firewall::linux::debian will be included from firewall

@underscorgan
Copy link
Contributor Author

@hesco oh, and yes, this should make it in for 1.3.0

) {
$ensure = running,
$service = $::firewall::params::service,
$package = $::firewall::params::package,
Copy link
Contributor

Choose a reason for hiding this comment

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

This was motivated by a need to make this work on Debian Jessie.
hunner added a commit that referenced this pull request Dec 3, 2014
MODULES-1309 - Make package and service names configurable
@hunner hunner merged commit db17e48 into puppetlabs:master Dec 3, 2014
@underscorgan underscorgan deleted the MODULES-1309 branch December 3, 2014 23:21
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

3 participants