openshift / openshift-sdn

Apache License 2.0
69 stars 63 forks source link

Ensure bridge-nf-call-iptables is always disabled on startup #251

Closed dcbw closed 8 years ago

dcbw commented 8 years ago

The Kubernetes proxy and external entities could set bridge-nf-call-iptables to 1 so we need to ensure it's still 0 after the node starts up. In particular the Kube proxy runs after SDN setup (because it relies on the endpoint filters that the SDN can provide) so there's no better way to ensure that bridge-nf-call-iptables stays 0 than a goroutine and loop.

dcbw commented 8 years ago

@liggitt is this more what you're thinking of? Associated Origin change is then:

    config.RunSDN()
    config.RunProxy()

-       // HACK: RunProxy resets bridge-nf-call-iptables from what openshift-sdn requires
    if config.SDNPlugin != nil {
-               sdnPluginName := nodeConfig.NetworkConfig.NetworkPluginName
-               if sdnPluginName == osdn.SingleTenantPluginName() || sdnPluginName == osdn.MultiTenantPluginName() {
-                       if err := sysctl.SetSysctl("net/bridge/bridge-nf-call-iptables", 0); err != nil {
-                               glog.Warningf("Could not set net.bridge.bridge-nf-call-iptables sysctl: %s", err)
-                       }
-               }
+               config.SDNPlugin.NodeInitialized()
        }

        return nil
liggitt commented 8 years ago

I'm still trying to understand the core issue. Is this the required order:

  1. start sdn plugin (via StartNode())
  2. start the proxy (does this depend on things done in the sdn plugin in StartNode()?)
  3. tell the sdn plugin the proxy is started?

If so, we should change the StartNode() interface to take a proxyReady chan struct{}, and inside StartNode(), the sdn plugin can do:

go func(){
  defer util.HandleCrash()
  <-proxyReady
  ... do the sysctl.SetSysctl bits ...
}()
liggitt commented 8 years ago

talked on irc... I think @dcbw is going to check upstream whether it makes sense to optionally prevent the proxy from messing with the sysctl setting (or have the proxy set it to what we want), instead of compensating by having the SDN plugin set it, then fix it up

danwinship commented 8 years ago

That kinda ties in to https://github.com/kubernetes/kubernetes/issues/19766, which is about not setting hairpin mode if the plugin doesn't need it

dcbw commented 8 years ago

@liggitt @danwinship https://github.com/kubernetes/kubernetes/pull/20647

dcbw commented 8 years ago

If that gets accepted, it should be pretty easy to plumb through start_node.go and server.go.

dcbw commented 8 years ago

Closing this one in favor of https://github.com/kubernetes/kubernetes/pull/20647