Sous-Chefs-Boneyard / sysctl

Development repository for the sysctl cookbook
https://supermarket.chef.io/cookbooks/sysctl
Apache License 2.0
42 stars 79 forks source link

Option to immediately persist to file #5

Closed mal closed 10 years ago

mal commented 10 years ago

We're currently facing an issue where we update the value of net.core.somaxconn and then try to run an application (3rd party) that checks that value is greater than 1024.

Because the values aren't persisted until the end of the chef run, the application sees the old value as it starts up and errors out and causes the entire chef run to fail.

sysctl_param('net.core.somaxconn') do
  value 4096
end

Chef::Log.warn `sysctl net.core.somaxconn` #=>  default system value

I suggest having the values persisted in two cases;

  1. When all values set via the params attribute have been updated
  2. During an LWRP call when a new resource variable is set immediate true for example
svanzoest commented 10 years ago

Hi @mal

The value should be set immediately, however, it won't be in the configuration file until the end of the chef run. Is the 3rd party appliation checking the configuration file or running sysctl or looking at /proc/sys directly?

mal commented 10 years ago

It appears to be looking at /proc/sys directly.

mal commented 10 years ago

Ah, I think the issue is that I set default.sysctl.params.net.core.somaxconn in my application cookbook's attributes file, expecting it to be applied when I called include_recipe 'sysctl'. but when that didn't seem to work I used the LWRP like so;

sysctl_param 'net.core.somaxconn' do
  value node.sysctl.params.net.core.somaxconn
end

Which I now realise does nothing because the LWRP thinks it's already set.

Is it possible to use attributes for this case, or is the LWRP the only way to go?

svanzoest commented 10 years ago

I primarily use the params attribute that are setup for the whole node and set them pretty earlier in the run list. You can call ruby_block[save-sysctl-params'] from a notification to trigger having them applied and written out the file. We changed this recently to support the LWRP use cases better.

You can also just put sysctl::persist early in the run list, so they will be applied before any other cookbooks run that depend on those kernel parameters to be configured.

Hmm.. maybe I will rename sysctl::apply as that makes a bit more sense for attributes anyways, as that call applies the parameters by loading in the configuration file after it has written it. It isn't just about persistence as the current name implies.

mal commented 10 years ago

I'm pretty sure looking at the code that it's impossible to get attributes applied during convergence.

This line means that regardless of when you notify ruby_block[save-sysctl-params] the template is never written until the end of the run, regardless of where anything is placed in the run_list.

I'll have a poke about tomorrow and put a PR in or something to get your feedback.

svanzoest commented 10 years ago

@mal I assume the 0.4.0 version of the cookbook works the way you are expecting. It is possible that this broke when improving the lwrp flow. I have been struggling making both the attributes and lwrp way work in the way that the users of both intended. It looks like that delay got switched from an immediate. We can adjust that in the sysctl::persist recipe as that has been the intend.

The biggest issue, is that it is tricky to use the current chefspec and serverspec tools to write a test to ensure that it is applied immediately. I thought I had that test in place, but the tests pass, however, this still got through the cracks.

svanzoest commented 10 years ago

@mal when you get a chance can you see if the current master resolves this? I also renamed sysctl::persist to sysctl::apply and updated the README to make the usage of this more clear. Let me know what you think.

svanzoest commented 10 years ago

This works for me in 0.6.0, feel free to reopen the ticket if this doesn't resolve it for you.

mal commented 10 years ago

Thanks for fixing it! I've not had a chance to test it, but from a quick look at the code I'm sure it function as you describe. I'll let you know if I have any trouble with it when we get around to rolling out the new version. Thanks again :grinning:

lock[bot] commented 6 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.