From 4f9aa7c7b5be928a1a7223889676cee51ef565f3 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 20 Sep 2017 16:07:35 -0500 Subject: [PATCH] sdn: disable hostport handling when CRIO is used This is a workaround because CRIO's CNI driver has different hostport behavior than Kubernetes' CNI driver. Kube leaves all hostport handling to the CNI plugin itself, while CRIO does hostport handling internally, leading to duplicate work if the plugin also handles hostports. For now, detect the runtime based on socket path and disable openshift-sdn hostport handling if the runtime is CRIO. The real fix for this is to add hostport handling to Kube's CNI driver, but in the "split" mode discussed upstream where kube's CNI driver handles the port reservations on the host, while the plugin handles the actual iptables rules. CRIO should be converted to this scheme as well, and plugins will indicate with capabilities in the CNI JSON whether they support this scheme or not. At that point we can remove this hack and just have openshift-sdn advertise portmapping support via the CNI JSON. --- .../server/kubernetes/network/sdn_linux.go | 8 ++++++ pkg/network/node/node.go | 3 ++- pkg/network/node/pod.go | 27 +++++++++++++------ 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/server/kubernetes/network/sdn_linux.go b/pkg/cmd/server/kubernetes/network/sdn_linux.go index 155dda9297d9..def367c72870 100644 --- a/pkg/cmd/server/kubernetes/network/sdn_linux.go +++ b/pkg/cmd/server/kubernetes/network/sdn_linux.go @@ -1,6 +1,8 @@ package network import ( + "strings" + "k8s.io/kubernetes/pkg/apis/componentconfig" kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" @@ -22,6 +24,11 @@ func NewSDNInterfaces(options configapi.NodeConfig, originClient *osclient.Clien } } + // dockershim + kube CNI driver delegates hostport handling to plugins, + // while CRI-O handles hostports itself. Thus we need to disable the + // SDN's hostport handling when run under CRI-O. + enableHostports := !strings.Contains(runtimeEndpoint, "crio") + node, err := sdnnode.New(&sdnnode.OsdnNodeConfig{ PluginName: options.NetworkConfig.NetworkPluginName, Hostname: options.NodeName, @@ -33,6 +40,7 @@ func NewSDNInterfaces(options configapi.NodeConfig, originClient *osclient.Clien KubeInformers: internalKubeInformers, IPTablesSyncPeriod: proxyconfig.IPTables.SyncPeriod.Duration, ProxyMode: proxyconfig.Mode, + EnableHostports: enableHostports, }) if err != nil { return nil, nil, err diff --git a/pkg/network/node/node.go b/pkg/network/node/node.go index 29168703b294..63bb93d9bd13 100644 --- a/pkg/network/node/node.go +++ b/pkg/network/node/node.go @@ -69,6 +69,7 @@ type OsdnNodeConfig struct { SelfIP string RuntimeEndpoint string MTU uint32 + EnableHostports bool OSClient *osclient.Client KClient kclientset.Interface @@ -177,7 +178,7 @@ func New(c *OsdnNodeConfig) (network.NodeInterface, error) { kClient: c.KClient, osClient: c.OSClient, oc: oc, - podManager: newPodManager(c.KClient, policy, c.MTU, oc), + podManager: newPodManager(c.KClient, policy, c.MTU, oc, c.EnableHostports), localIP: c.SelfIP, hostName: c.Hostname, useConnTrack: useConnTrack, diff --git a/pkg/network/node/pod.go b/pkg/network/node/pod.go index 5018e3cd56b9..55661f65e6c8 100644 --- a/pkg/network/node/pod.go +++ b/pkg/network/node/pod.go @@ -69,6 +69,8 @@ type podManager struct { mtu uint32 ovs *ovsController + enableHostports bool + // Things only accessed through the processCNIRequests() goroutine // and thus can be set from Start() ipamConfig []byte @@ -76,13 +78,14 @@ type podManager struct { } // Creates a new live podManager; used by node code0 -func newPodManager(kClient kclientset.Interface, policy osdnPolicy, mtu uint32, ovs *ovsController) *podManager { +func newPodManager(kClient kclientset.Interface, policy osdnPolicy, mtu uint32, ovs *ovsController, enableHostports bool) *podManager { pm := newDefaultPodManager() pm.kClient = kClient pm.policy = policy pm.mtu = mtu pm.podHandler = pm pm.ovs = ovs + pm.enableHostports = enableHostports return pm } @@ -152,7 +155,9 @@ func getIPAMConfig(clusterNetwork *net.IPNet, localSubnet string) ([]byte, error // Start the CNI server and start processing requests from it func (m *podManager) Start(socketPath string, localSubnetCIDR string, clusterNetwork *net.IPNet) error { - m.hostportSyncer = kubehostport.NewHostportSyncer() + if m.enableHostports { + m.hostportSyncer = kubehostport.NewHostportSyncer() + } var err error if m.ipamConfig, err = getIPAMConfig(clusterNetwork, localSubnetCIDR); err != nil { @@ -500,8 +505,10 @@ func (m *podManager) setup(req *cniserver.PodRequest) (cnitypes.Result, *running defer func() { if !success { m.ipamDel(req.SandboxID) - if err := m.hostportSyncer.SyncHostports(Tun0, m.getRunningPods()); err != nil { - glog.Warningf("failed syncing hostports: %v", err) + if m.hostportSyncer != nil { + if err := m.hostportSyncer.SyncHostports(Tun0, m.getRunningPods()); err != nil { + glog.Warningf("failed syncing hostports: %v", err) + } } } }() @@ -512,8 +519,10 @@ func (m *podManager) setup(req *cniserver.PodRequest) (cnitypes.Result, *running return nil, nil, err } podPortMapping := kubehostport.ConstructPodPortMapping(&v1Pod, podIP) - if err := m.hostportSyncer.OpenPodHostportsAndSync(podPortMapping, Tun0, m.getRunningPods()); err != nil { - return nil, nil, err + if m.hostportSyncer != nil { + if err := m.hostportSyncer.OpenPodHostportsAndSync(podPortMapping, Tun0, m.getRunningPods()); err != nil { + return nil, nil, err + } } var hostVethName, contVethMac string @@ -632,8 +641,10 @@ func (m *podManager) teardown(req *cniserver.PodRequest) error { errList = append(errList, err) } - if err := m.hostportSyncer.SyncHostports(Tun0, m.getRunningPods()); err != nil { - errList = append(errList, err) + if m.hostportSyncer != nil { + if err := m.hostportSyncer.SyncHostports(Tun0, m.getRunningPods()); err != nil { + errList = append(errList, err) + } } return kerrors.NewAggregate(errList)