Add swap_file resource from the swap cookbook #6990
Conversation
tas50
changed the title
tas50
changed the title
tas50
changed the title
lib/chef/resource/swap_file.rb
Outdated
| if new_resource.swappiness | ||
| include_recipe "sysctl::default" | ||
|
|
||
| sysctl_param "vm.swappiness" do |
There was a problem hiding this comment.
Should use sysctl as the new primary name.
There was a problem hiding this comment.
Done and the include is gone too
| do_create(dd_command) | ||
| end | ||
| end | ||
| if new_resource.swappiness |
There was a problem hiding this comment.
I'm mixed on this feature. Notably if you have more than one swap, they would compete? But that's pretty rare.
There was a problem hiding this comment.
Separation of concerns suggests that this option shouldn't be here. And it creates a really weird API where this is just a facade over calling the sysctl resource to set the swappiness.
There was a problem hiding this comment.
Two fold answer:
First part: It's an optional feature and it's off by default. It makes a lot of sense to manage your swapiness when you configure your swap file. For users that want to just get swap going they can set a value and it'll just work. If they don't turn it on, then they won't get it, which seems to follow "the chef way".
Second part: I think the scenario where we have multiple swap files and the users specify different swapiness values it super edge case. You could have multiple swap files and even set the swapiness on both without issue. You could specify it on one and it would work. You just can't have 2 different values and I think it should be pretty clear to anyone who would want swapiness set why that isn't possible.
There was a problem hiding this comment.
3rd part: Users of the cookbook are utilizing this property so removing it breaks that API in a sad way
lib/chef/resource/swap_file.rb
Outdated
| compatible_filesystems.any? { |fs| result.include? fs } | ||
| end | ||
|
|
||
| # we can remove this when we only support Chef 13 |
There was a problem hiding this comment.
you only need to support Chef 14 now
Copied as-is from the cookbook Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
|
Yep the recipe was a leftover from the pre-1.0 cookbook that was very much attribute driven still. |
|
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. |
This is copied as-is from the swap cookbook and included here with permission of Vargo
Signed-off-by: Tim Smith [email protected]