From 0c76756a844b21d5496753e1f58cf0a1a215f808 Mon Sep 17 00:00:00 2001 From: Chris Telfer Date: Tue, 9 Oct 2018 10:04:31 -0400 Subject: [PATCH] Make DSR an overlay-specific driver "option" Allow DSR to be a configurable option through a generic option to the overlay driver. On the one hand this approach makes sense insofar as only overlay networks can currently perform load balancing. On the other hand, this approach has several issues. First, should we create another type of swarm scope network, this will prevent it working. Second, the service core code is separate from the driver code and the driver code can't influence the core data structures. So the driver code can't set this option itself. Therefore, implementing in this way requires some hack code to test for this option in controller.NewNetwork. A more correct approach would be to make this a generic option for any network. Then the driver could ignore, reject or be unaware of the option depending on the chosen model. This would require changes to: * libnetwork - naturally * the docker API - to carry the option * swarmkit - to propagate the option * the docker CLI - to support the option * moby - to translate the API option into a libnetwork option Given the urgency of requests to address this issue, this approach will be saved for a future iteration. Signed-off-by: Chris Telfer --- controller.go | 37 ++++++++++++++++------ network.go | 81 ++++++++++++++++++++++++++++-------------------- sandbox.go | 14 ++++----- service_linux.go | 21 +++++-------- 4 files changed, 90 insertions(+), 63 deletions(-) diff --git a/controller.go b/controller.go index f956ddb280..2896011dbf 100644 --- a/controller.go +++ b/controller.go @@ -700,6 +700,9 @@ func (c *controller) RegisterDriver(networkType string, driver driverapi.Driver, return nil } +// XXX This should be made driver agnostic. See comment below. +const overlayDSROptionString = "dsr" + // NewNetwork creates a new network of the specified network type. The options // are network specific and modeled in a generic way. func (c *controller) NewNetwork(networkType, name string, id string, options ...NetworkOption) (Network, error) { @@ -723,15 +726,16 @@ func (c *controller) NewNetwork(networkType, name string, id string, options ... defaultIpam := defaultIpamForNetworkType(networkType) // Construct the network object network := &network{ - name: name, - networkType: networkType, - generic: map[string]interface{}{netlabel.GenericData: make(map[string]string)}, - ipamType: defaultIpam, - id: id, - created: time.Now(), - ctrlr: c, - persist: true, - drvOnce: &sync.Once{}, + name: name, + networkType: networkType, + generic: map[string]interface{}{netlabel.GenericData: make(map[string]string)}, + ipamType: defaultIpam, + id: id, + created: time.Now(), + ctrlr: c, + persist: true, + drvOnce: &sync.Once{}, + loadBalancerMode: loadBalancerModeDefault, } network.processOptions(options...) @@ -829,6 +833,21 @@ func (c *controller) NewNetwork(networkType, name string, id string, options ... } }() + // XXX If the driver type is "overlay" check the options for DSR + // being set. If so, set the network's load balancing mode to DSR. + // This should really be done in a network option, but due to + // time pressure to get this in without adding changes to moby, + // swarm and CLI, it is being implemented as a driver-specific + // option. Unfortunately, drivers can't influence the core + // "libnetwork.network" data type. Hence we need this hack code + // to implement in this manner. + if gval, ok := network.generic[netlabel.GenericData]; ok && network.networkType == "overlay" { + optMap := gval.(map[string]string) + if _, ok := optMap[overlayDSROptionString]; ok { + network.loadBalancerMode = loadBalancerModeDSR + } + } + addToStore: // First store the endpoint count, then the network. To avoid to // end up with a datastore containing a network and not an epCnt, diff --git a/network.go b/network.go index 052aa8febe..0a4a2277b0 100644 --- a/network.go +++ b/network.go @@ -199,43 +199,50 @@ func (i *IpamInfo) UnmarshalJSON(data []byte) error { } type network struct { - ctrlr *controller - name string - networkType string - id string - created time.Time - scope string // network data scope - labels map[string]string - ipamType string - ipamOptions map[string]string - addrSpace string - ipamV4Config []*IpamConf - ipamV6Config []*IpamConf - ipamV4Info []*IpamInfo - ipamV6Info []*IpamInfo - enableIPv6 bool - postIPv6 bool - epCnt *endpointCnt - generic options.Generic - dbIndex uint64 - dbExists bool - persist bool - stopWatchCh chan struct{} - drvOnce *sync.Once - resolverOnce sync.Once - resolver []Resolver - internal bool - attachable bool - inDelete bool - ingress bool - driverTables []networkDBTable - dynamic bool - configOnly bool - configFrom string - loadBalancerIP net.IP + ctrlr *controller + name string + networkType string + id string + created time.Time + scope string // network data scope + labels map[string]string + ipamType string + ipamOptions map[string]string + addrSpace string + ipamV4Config []*IpamConf + ipamV6Config []*IpamConf + ipamV4Info []*IpamInfo + ipamV6Info []*IpamInfo + enableIPv6 bool + postIPv6 bool + epCnt *endpointCnt + generic options.Generic + dbIndex uint64 + dbExists bool + persist bool + stopWatchCh chan struct{} + drvOnce *sync.Once + resolverOnce sync.Once + resolver []Resolver + internal bool + attachable bool + inDelete bool + ingress bool + driverTables []networkDBTable + dynamic bool + configOnly bool + configFrom string + loadBalancerIP net.IP + loadBalancerMode string sync.Mutex } +const ( + loadBalancerModeNAT = "NAT" + loadBalancerModeDSR = "DSR" + loadBalancerModeDefault = loadBalancerModeNAT +) + func (n *network) Name() string { n.Lock() defer n.Unlock() @@ -475,6 +482,7 @@ func (n *network) CopyTo(o datastore.KVObject) error { dstN.configOnly = n.configOnly dstN.configFrom = n.configFrom dstN.loadBalancerIP = n.loadBalancerIP + dstN.loadBalancerMode = n.loadBalancerMode // copy labels if dstN.labels == nil { @@ -592,6 +600,7 @@ func (n *network) MarshalJSON() ([]byte, error) { netMap["configOnly"] = n.configOnly netMap["configFrom"] = n.configFrom netMap["loadBalancerIP"] = n.loadBalancerIP + netMap["loadBalancerMode"] = n.loadBalancerMode return json.Marshal(netMap) } @@ -705,6 +714,10 @@ func (n *network) UnmarshalJSON(b []byte) (err error) { if v, ok := netMap["loadBalancerIP"]; ok { n.loadBalancerIP = net.ParseIP(v.(string)) } + n.loadBalancerMode = loadBalancerModeDefault + if v, ok := netMap["loadBalancerMode"]; ok { + n.loadBalancerMode = v.(string) + } // Reconcile old networks with the recently added `--ipv6` flag if !n.enableIPv6 { n.enableIPv6 = len(n.ipamV6Info) > 0 diff --git a/sandbox.go b/sandbox.go index 185b0adbdd..03c9215786 100644 --- a/sandbox.go +++ b/sandbox.go @@ -730,7 +730,7 @@ func (sb *sandbox) DisableService() (err error) { return nil } -func releaseOSSboxResources(osSbox osl.Sandbox, ep *endpoint, ingress bool) { +func releaseOSSboxResources(osSbox osl.Sandbox, ep *endpoint) { for _, i := range osSbox.Info().Interfaces() { // Only remove the interfaces owned by this endpoint from the sandbox. if ep.hasInterface(i.SrcName()) { @@ -743,9 +743,10 @@ func releaseOSSboxResources(osSbox osl.Sandbox, ep *endpoint, ingress bool) { ep.Lock() joinInfo := ep.joinInfo vip := ep.virtualIP + lbModeIsDSR := ep.network.loadBalancerMode == loadBalancerModeDSR ep.Unlock() - if len(vip) > 0 && !ingress { + if len(vip) > 0 && lbModeIsDSR { ipNet := &net.IPNet{IP: vip, Mask: net.CIDRMask(32, 32)} if err := osSbox.RemoveAliasIP(osSbox.GetLoopbackIfaceName(), ipNet); err != nil { logrus.WithError(err).Debugf("failed to remove virtual ip %v to loopback", ipNet) @@ -768,7 +769,6 @@ func (sb *sandbox) releaseOSSbox() { sb.Lock() osSbox := sb.osSbox sb.osSbox = nil - ingress := sb.ingress sb.Unlock() if osSbox == nil { @@ -776,7 +776,7 @@ func (sb *sandbox) releaseOSSbox() { } for _, ep := range sb.getConnectedEndpoints() { - releaseOSSboxResources(osSbox, ep, ingress) + releaseOSSboxResources(osSbox, ep) } osSbox.Destroy() @@ -840,6 +840,7 @@ func (sb *sandbox) populateNetworkResources(ep *endpoint) error { ep.Lock() joinInfo := ep.joinInfo i := ep.iface + lbModeIsDSR := ep.network.loadBalancerMode == loadBalancerModeDSR ep.Unlock() if ep.needResolver() { @@ -864,7 +865,7 @@ func (sb *sandbox) populateNetworkResources(ep *endpoint) error { return fmt.Errorf("failed to add interface %s to sandbox: %v", i.srcName, err) } - if len(ep.virtualIP) > 0 && !sb.ingress { + if len(ep.virtualIP) > 0 && lbModeIsDSR { if sb.loadBalancerNID == "" { if err := sb.osSbox.DisableARPForVIP(i.srcName); err != nil { return fmt.Errorf("failed disable ARP for VIP: %v", err) @@ -925,10 +926,9 @@ func (sb *sandbox) clearNetworkResources(origEp *endpoint) error { sb.Lock() osSbox := sb.osSbox inDelete := sb.inDelete - ingress := sb.ingress sb.Unlock() if osSbox != nil { - releaseOSSboxResources(osSbox, ep, ingress) + releaseOSSboxResources(osSbox, ep) } sb.Lock() diff --git a/service_linux.go b/service_linux.go index 5e9e0e03e0..451f760b61 100644 --- a/service_linux.go +++ b/service_linux.go @@ -142,7 +142,7 @@ func (n *network) addLBBackend(ip net.IP, lb *loadBalancer) { } logrus.Debugf("Creating service for vip %s fwMark %d ingressPorts %#v in sbox %.7s (%.7s)", lb.vip, lb.fwMark, lb.service.ingressPorts, sb.ID(), sb.ContainerID()) - if err := invokeFWMarker(sb.Key(), lb.vip, lb.fwMark, lb.service.ingressPorts, eIP, false, n.ingress); err != nil { + if err := invokeFWMarker(sb.Key(), lb.vip, lb.fwMark, lb.service.ingressPorts, eIP, false, n.loadBalancerMode); err != nil { logrus.Errorf("Failed to add firewall mark rule in sbox %.7s (%.7s): %v", sb.ID(), sb.ContainerID(), err) return } @@ -158,7 +158,7 @@ func (n *network) addLBBackend(ip net.IP, lb *loadBalancer) { Address: ip, Weight: 1, } - if !n.ingress { + if n.loadBalancerMode == loadBalancerModeDSR { d.ConnectionFlags = ipvs.ConnFwdDirectRoute } @@ -206,7 +206,7 @@ func (n *network) rmLBBackend(ip net.IP, lb *loadBalancer, rmService bool, fullR Address: ip, Weight: 1, } - if !n.ingress { + if n.loadBalancerMode == loadBalancerModeDSR { d.ConnectionFlags = ipvs.ConnFwdDirectRoute } @@ -237,7 +237,7 @@ func (n *network) rmLBBackend(ip net.IP, lb *loadBalancer, rmService bool, fullR } } - if err := invokeFWMarker(sb.Key(), lb.vip, lb.fwMark, lb.service.ingressPorts, eIP, true, n.ingress); err != nil { + if err := invokeFWMarker(sb.Key(), lb.vip, lb.fwMark, lb.service.ingressPorts, eIP, true, n.loadBalancerMode); err != nil { logrus.Errorf("Failed to delete firewall mark rule in sbox %.7s (%.7s): %v", sb.ID(), sb.ContainerID(), err) } @@ -572,7 +572,7 @@ func readPortsFromFile(fileName string) ([]*PortConfig, error) { // Invoke fwmarker reexec routine to mark vip destined packets with // the passed firewall mark. -func invokeFWMarker(path string, vip net.IP, fwMark uint32, ingressPorts []*PortConfig, eIP *net.IPNet, isDelete bool, isIngress bool) error { +func invokeFWMarker(path string, vip net.IP, fwMark uint32, ingressPorts []*PortConfig, eIP *net.IPNet, isDelete bool, lbMode string) error { var ingressPortsFile string if len(ingressPorts) != 0 { @@ -590,14 +590,9 @@ func invokeFWMarker(path string, vip net.IP, fwMark uint32, ingressPorts []*Port addDelOpt = "-D" } - isIngressOpt := "false" - if isIngress { - isIngressOpt = "true" - } - cmd := &exec.Cmd{ Path: reexec.Self(), - Args: append([]string{"fwmarker"}, path, vip.String(), fmt.Sprintf("%d", fwMark), addDelOpt, ingressPortsFile, eIP.String(), isIngressOpt), + Args: append([]string{"fwmarker"}, path, vip.String(), fmt.Sprintf("%d", fwMark), addDelOpt, ingressPortsFile, eIP.String(), lbMode), Stdout: os.Stdout, Stderr: os.Stderr, } @@ -656,8 +651,8 @@ func fwMarker() { os.Exit(5) } - isIngressOpt := os.Args[7] - if addDelOpt == "-A" && isIngressOpt == "true" { + lbMode := os.Args[7] + if addDelOpt == "-A" && lbMode == loadBalancerModeNAT { eIP, subnet, err := net.ParseCIDR(os.Args[6]) if err != nil { logrus.Errorf("Failed to parse endpoint IP %s: %v", os.Args[6], err)