-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Selectively disable iptables proxy's bridge-nf-call-iptables=1 behavior #20647
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ import ( | |
"encoding/base32" | ||
"fmt" | ||
"net" | ||
"os" | ||
"reflect" | ||
"strconv" | ||
"strings" | ||
|
@@ -190,12 +191,18 @@ func NewProxier(ipt utiliptables.Interface, exec utilexec.Interface, syncPeriod | |
return nil, fmt.Errorf("can't set sysctl %s: %v", sysctlRouteLocalnet, err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this sysctl call still appropriate for the proxier to set, rather than the network plugin? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can move it to a plugin when we start using plugins by default. For now I'd like to keep things as they are unless absolutely necessary. the no-op plugin should really no-op. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worse, there was a separate PR, somewhere, that made the no-op plugin go away and be a nil pointer. |
||
} | ||
|
||
// Load the module. It's OK if this fails (e.g. the module is not present) | ||
// because we'll catch the error on the sysctl, which is what we actually | ||
// care about. | ||
exec.Command("modprobe", "br-netfilter").CombinedOutput() | ||
if err := utilsysctl.SetSysctl(sysctlBridgeCallIptables, 1); err != nil { | ||
glog.Warningf("can't set sysctl %s: %v", sysctlBridgeCallIptables, err) | ||
// Proxy needs br_netfilter and bridge-nf-call-iptables=1 when containers | ||
// are connected to a Linux bridge (but not SDN bridges). Until most | ||
// plugins handle this, log when config is missing | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you log something here in case someone already has a plugin that needs br-netfilter and they're getting it for free? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ok, but not quite sure what you're asking for... logging that the load failed? it's expected to fail in a bunch of systems because br-netfilter used to be built-in to the bridge module, but got split sometime in the late 3.1x timeframe. So new systems need this, older ones don't. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you just run modinfo and log something like: br-netfilter module [not] loaded? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'll doing that. |
||
warnBrNetfilter := false | ||
if _, err := os.Stat("/sys/module/br_netfilter"); os.IsNotExist(err) { | ||
warnBrNetfilter = true | ||
} | ||
if val, err := utilsysctl.GetSysctl(sysctlBridgeCallIptables); err == nil && val != 1 { | ||
warnBrNetfilter = true | ||
} | ||
if warnBrNetfilter { | ||
glog.Infof("missing br-netfilter module or unset br-nf-call-iptables; proxy may not work as intended") | ||
} | ||
|
||
// Generate the masquerade mark to use for SNAT rules. | ||
|
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.
does this imply that the plugin will need to know parts of the kube-proxy config (at least the mode)? is there a way for the plugins to get that information currently?
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.
Yes, plugins will obviously need to know about proxy configuration if their networking setup will have an effect on the proxy. We were just naive before as assumed everyone did everything the same way :(
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.
ok, so how would a plugin find out (for example) if you were running the kube-proxy in userspace or iptables mode?