thias / puppet-sysctl

Puppet module to manage sysctl parameters
Other
35 stars 86 forks source link

Needs the value quoted as some contain spaces #3

Closed skmike closed 10 years ago

skmike commented 11 years ago

file: modules/sysctl/manifests/init.pp changed the line: command => "/sbin/sysctl -w ${title}=${value}", to command => "/sbin/sysctl -w ${title}=\'${value}\'",

skmike commented 11 years ago

slight update to cope with spaces and the lack thereof: exec { "sysctl-${title}": command => "/sbin/sysctl -w ${title}=\'${value}\'", unless => "/sbin/sysctl -n ${title} | /bin/grep -q -e '^${value}\$' | /bin/sed -e's/[ ][ ]/ /g'", } and exec { "update-sysctl.conf-${title}": command => "sed -i -e 's/${title}[ ]=./${title} = ${value}/' /etc/sysctl.conf", unless => "/bin/bash -c \"! egrep '^${title}[ ]=' /etc/sysctl.conf || egrep '^${title}[ ]=[ ]${value}\$' /etc/sysctl.conf\"", path => [ '/usr/sbin', '/sbin', '/usr/bin', '/bin' ], }

thias commented 11 years ago

Good catch. But are you sure about the added sed invocation? Shouldn't you squeeze the spaces before grepping for the value?

skmike commented 11 years ago

No problems. Not really - the spaces are added by sysctl. Might also convert the "value" as well to be single spaced for the grep, just to be paranoid.

As a quick aside - as I'm very new to puppet, can you point me in the right direction for setting a sysctl value using either a json or yaml hiera override - I can get hiera to find the correct file, but I can't get it to pass that value to your module. I'm probably missing something obvious. e.g. if you have a site wide setting for net.core.rmem_max and want to override it on a few boxes, how would you do it?

Rgds, Mike.

On 2 July 2013 16:41, Matthias Saou notifications@github.com wrote:

Good catch. But are you sure about the added sed invocation? Shouldn't you squeeze the spaces before grepping for the value?

— Reply to this email directly or view it on GitHubhttps://github.com/thias/puppet-sysctl/issues/3#issuecomment-20354013 .

thwarted commented 10 years ago

It might be better to have the exec require => File["/etc/sysctl.d/${sysctl_d_file}"] and then do sysctl -p /etc/sysctl.d/${sysctl_d_file} in the exec rather than -w. This way you don't need to worry about spacing and quoting on the command line.

As for updating /etc/sysctl.conf, I usually try to leave that alone and override its contents with files in /etc/sysctl.d.

thias commented 10 years ago

Running sysctl -p /etc/sysctl.d/${sysctl_d_file} does make a lot of sense.

As for updating /etc/sysctl.conf, it's unfortunately required since people/tools/scripts might be running the typical sysctl -p which doesn't take into account the /etc/sysctl.d/ content. The typical example on RHEL6 is net.ipv4.ip_forward.

thias commented 10 years ago

This should now be fixed in 3ee4320fb1f7d1f5aefb9e9df51e48ec13e4c5b1 after using @thwarted 's suggestion. Please let me know otherwise!

thwarted commented 10 years ago

On CentOS, /etc/init.d/functions and /etc/sysconfig/network-scripts/ifup both read the files out of sysctl.d, but I agree, yeah it is a problem for things that only mess with /etc/sysctl.conf. After putting some thought into it, I'm going to move the contents of /etc/sysctl.conf into /etc/sysctl.d in the setups I'm responsible for to avoid this.