Add sysctl_param resource from the sysctl cookbook #7022
Conversation
Copied from the cookbook with modifications made there Signed-off-by: Tim Smith <[email protected]>
tas50
changed the title
lib/chef/resource/sysctl_param.rb
Outdated
| value get_sysctl_value(key) | ||
| if node.normal["sysctl"]["backup"][key].empty? | ||
| node.normal["sysctl"]["backup"][key] = value | ||
| end |
There was a problem hiding this comment.
i still think this is the weirdest feature ever.
this key could be saved on the node for three years and then :removed, and who the hell knows what is going to happen...
action :remove as a concept feels like "you're doing config management wrong".
There was a problem hiding this comment.
I think action :remove is fine to clean up the underlying files but I too am unsure why we are storing this stuff into node attributes. What problem does this solve?
lib/chef/resource/sysctl_param.rb
Outdated
| class Resource | ||
| class SysctlParam < Chef::Resource | ||
| resource_name :sysctl_param | ||
| provides :sysctl_param |
There was a problem hiding this comment.
I sort of think this should be:
resource_name :sysctl
provides :sysctl
provides :sysctl_param # back compat with the cookbook
There was a problem hiding this comment.
Also even if we don't want to have two names, still don't need both resource_name and provides for the same sym.
There was a problem hiding this comment.
I pushed a rename commit
lib/chef/resource/sysctl_param.rb
Outdated
|
|
||
| def get_sysctl_value(key) | ||
| o = shell_out("sysctl -n -e #{key}") | ||
| raise "Unknown sysctl key!" if o.error! |
There was a problem hiding this comment.
Should include the key name in the error message so it is clearer.
lib/chef/resource/sysctl_param.rb
Outdated
| value get_sysctl_value(key) | ||
| if node.normal["sysctl"]["backup"][key].empty? | ||
| node.normal["sysctl"]["backup"][key] = value | ||
| end |
There was a problem hiding this comment.
I think action :remove is fine to clean up the underlying files but I too am unsure why we are storing this stuff into node attributes. What problem does this solve?
lib/chef/resource/sysctl_param.rb
Outdated
| class Resource | ||
| class SysctlParam < Chef::Resource | ||
| resource_name :sysctl_param | ||
| provides :sysctl_param |
There was a problem hiding this comment.
Also even if we don't want to have two names, still don't need both resource_name and provides for the same sym.
lib/chef/resource/sysctl_param.rb
Outdated
| end | ||
|
|
||
| execute "sysctl -p" do | ||
| command "sysctl -p" |
There was a problem hiding this comment.
Redundant command and action.
Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
lib/chef/resource/sysctl.rb
Outdated
| end | ||
|
|
||
| execute "sysctl -p" do | ||
| command "sysctl -p" |
There was a problem hiding this comment.
Still redundant. Doesn't hurt anything but easy to DRY.
lib/chef/resource/sysctl.rb
Outdated
|
|
||
| def get_sysctl_value(key) | ||
| o = shell_out("sysctl -n -e #{key}") | ||
| raise "Unknown sysctl key #{key}!" if o.error! |
There was a problem hiding this comment.
error! raises. so this is ... a bit meaningless :) did you mean error?
There was a problem hiding this comment.
Oh nice that never worked in the cookbook either. I'll get that fixed
lib/chef/resource/sysctl.rb
Outdated
|
|
||
| load_current_value do | ||
| value get_sysctl_value(key) | ||
| if node.normal["sysctl"]["backup"][key].empty? |
There was a problem hiding this comment.
Don't we discourage normal ?
Still needs a bit of testing to see if there's a better way to do the reload, but here goes nothing Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
tas50
changed the title
|
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. |
Copied from the cookbook with modifications made there
Signed-off-by: Tim Smith [email protected]