-
-
Notifications
You must be signed in to change notification settings - Fork 792
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
added ip forwarding flag #115
Conversation
sysctl_param 'net.ipv4.ip_forward' do | ||
value 1 | ||
only_if { node['platform'] == 'debian' } | ||
unless node['docker']['ip_forward'] == true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this does the opposite of what I'd expect this attribute to do: setting ip_forward to true would not enable IP forwarding.
In any event, for conditionals surrounding a single resource, Chef convention is to use only_if
and not_if
inside the block, just like the only_if
that's already inside sysctl_param
. The fix would be changing this to: only_if { node['platform'] == 'debian' && node['docker']['ip_forward'] }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right, I wasn't sure if multiple conditions in the only_if were supported.
ExecStartPre=/usr/sbin/sysctl -w net.ipv4.ip_forward=1 net.ipv6.conf.all.forwarding=1 | ||
<% else -%> | ||
ExecStartPre=/usr/sbin/sysctl -w net.ipv6.conf.all.forwarding=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you create a second attribute for IPv6? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation detail: IPv4 would be inside its own if/end block while IPv6 would be inside its own if/end block.
Thanks for your hard work so far! I think we'll be good to merge this with these last changes. |
I'm merging this in now and making the IPv6 forwarding changes myself. Will be included in next release. Thanks again! |
No description provided.