Code markups and minor refactorings

backend/common.go

@@ -15,16 +15,11 @@
 package backend
 
 import (
-	"bytes"
 	"net"
-	"sync"
-	"time"
 
 	"golang.org/x/net/context"
 
 	"github.com/coreos/flannel/subnet"
-	log "github.com/golang/glog"
-	"github.com/vishvananda/netlink"
 )
 
 type ExternalInterface struct {
@@ -49,195 +44,3 @@ type Network interface {
 }
 
 type BackendCtor func(sm subnet.Manager, ei *ExternalInterface) (Backend, error)
-
-type SimpleNetwork struct {
-	SubnetLease *subnet.Lease
-	ExtIface    *ExternalInterface
-}
-
-func (n *SimpleNetwork) Lease() *subnet.Lease {
-	return n.SubnetLease
-}
-
-func (n *SimpleNetwork) MTU() int {
-	return n.ExtIface.Iface.MTU
-}
-
-func (_ *SimpleNetwork) Run(ctx context.Context) {
-	<-ctx.Done()
-}
-
-const (
-	routeCheckRetries = 10
-)
-
-type RouteNetwork struct {
-	SimpleNetwork
-	BackendType string
-	routes      []netlink.Route
-	SM          subnet.Manager
-	GetRoute    func(lease *subnet.Lease) *netlink.Route
-	Mtu         int
-	LinkIndex   int
-}
-
-func (n *RouteNetwork) MTU() int {
-	return n.Mtu
-}
-
-func (n *RouteNetwork) Run(ctx context.Context) {
-	wg := sync.WaitGroup{}
-
-	log.Info("Watching for new subnet leases")
-	evts := make(chan []subnet.Event)
-	wg.Add(1)
-	go func() {
-		subnet.WatchLeases(ctx, n.SM, n.SubnetLease, evts)
-		wg.Done()
-	}()
-
-	n.routes = make([]netlink.Route, 0, 10)
-	wg.Add(1)
-	go func() {
-		n.routeCheck(ctx)
-		wg.Done()
-	}()
-
-	defer wg.Wait()
-
-	for {
-		select {
-		case evtBatch := <-evts:
-			n.handleSubnetEvents(evtBatch)
-
-		case <-ctx.Done():
-			return
-		}
-	}
-}
-
-func (n *RouteNetwork) handleSubnetEvents(batch []subnet.Event) {
-	for _, evt := range batch {
-		switch evt.Type {
-		case subnet.EventAdded:
-			log.Infof("Subnet added: %v via %v", evt.Lease.Subnet, evt.Lease.Attrs.PublicIP)
-
-			if evt.Lease.Attrs.BackendType != n.BackendType {
-				log.Warningf("Ignoring non-%v subnet: type=%v", n.BackendType, evt.Lease.Attrs.BackendType)
-				continue
-			}
-			route := n.GetRoute(&evt.Lease)
-
-			n.addToRouteList(*route)
-			// Check if route exists before attempting to add it
-			routeList, err := netlink.RouteListFiltered(netlink.FAMILY_V4, &netlink.Route{Dst: route.Dst}, netlink.RT_FILTER_DST)
-			if err != nil {
-				log.Warningf("Unable to list routes: %v", err)
-			}
-			if len(routeList) > 0 && !routeEqual(routeList[0], *route) {
-				// Same Dst different Gw or different link index. Remove it, correct route will be added below.
-				log.Warningf("Replacing existing route to %v via %v dev index %d with %v via %v dev index %d.", evt.Lease.Subnet, routeList[0].Gw, routeList[0].LinkIndex, evt.Lease.Subnet, evt.Lease.Attrs.PublicIP, route.LinkIndex)
-				if err := netlink.RouteDel(&routeList[0]); err != nil {
-					log.Errorf("Error deleting route to %v: %v", evt.Lease.Subnet, err)
-					continue
-				}
-				n.removeFromRouteList(routeList[0])
-			}
-			if len(routeList) > 0 && routeEqual(routeList[0], *route) {
-				// Same Dst and same Gw, keep it and do not attempt to add it.
-				log.Infof("Route to %v via %v dev index %d already exists, skipping.", evt.Lease.Subnet, evt.Lease.Attrs.PublicIP, routeList[0].LinkIndex)
-			} else if err := netlink.RouteAdd(route); err != nil {
-				log.Errorf("Error adding route to %v via %v dev index %d: %v", evt.Lease.Subnet, evt.Lease.Attrs.PublicIP, route.LinkIndex, err)
-				continue
-			}
-
-		case subnet.EventRemoved:
-			log.Info("Subnet removed: ", evt.Lease.Subnet)
-
-			if evt.Lease.Attrs.BackendType != n.BackendType {
-				log.Warningf("Ignoring non-%v subnet: type=%v", n.BackendType, evt.Lease.Attrs.BackendType)
-				continue
-			}
-
-			route := n.GetRoute(&evt.Lease)
-			// Always remove the route from the route list.
-			n.removeFromRouteList(*route)
-
-			if err := netlink.RouteDel(route); err != nil {
-				log.Errorf("Error deleting route to %v: %v", evt.Lease.Subnet, err)
-				continue
-			}
-
-		default:
-			log.Error("Internal error: unknown event type: ", int(evt.Type))
-		}
-	}
-}
-
-func (n *RouteNetwork) addToRouteList(route netlink.Route) {
-	for _, r := range n.routes {
-		if routeEqual(r, route) {
-			return
-		}
-	}
-	n.routes = append(n.routes, route)
-}
-
-func (n *RouteNetwork) removeFromRouteList(route netlink.Route) {
-	for index, r := range n.routes {
-		if routeEqual(r, route) {
-			n.routes = append(n.routes[:index], n.routes[index+1:]...)
-			return
-		}
-	}
-}
-
-func (n *RouteNetwork) routeCheck(ctx context.Context) {
-	for {
-		select {
-		case <-ctx.Done():
-			return
-		case <-time.After(routeCheckRetries * time.Second):
-			n.checkSubnetExistInRoutes()
-		}
-	}
-}
-
-func (n *RouteNetwork) checkSubnetExistInRoutes() {
-	routeList, err := netlink.RouteList(nil, netlink.FAMILY_V4)
-	if err == nil {
-		for _, route := range n.routes {
-			exist := false
-			for _, r := range routeList {
-				if r.Dst == nil {
-					continue
-				}
-				if routeEqual(r, route) {
-					exist = true
-					break
-				}
-			}
-			if !exist {
-				if err := netlink.RouteAdd(&route); err != nil {
-					if nerr, ok := err.(net.Error); !ok {
-						log.Errorf("Error recovering route to %v: %v, %v", route.Dst, route.Gw, nerr)
-					}
-					continue
-				} else {
-					log.Infof("Route recovered %v : %v", route.Dst, route.Gw)
-				}
-			}
-		}
-	} else {
-		log.Errorf("Error fetching route list. Will automatically retry: %v", err)
-	}
-}
-
-func routeEqual(x, y netlink.Route) bool {
-	// For ipip backend, when enabling directrouting, link index of some routes may change
-	// For both ipip and host-gw backend, link index may also change if updating ExtIface
-	if x.Dst.IP.Equal(y.Dst.IP) && x.Gw.Equal(y.Gw) && bytes.Equal(x.Dst.Mask, y.Dst.Mask) && x.LinkIndex == y.LinkIndex {
-		return true
-	}
-	return false
-}

backend/ipip/ipip.go

@@ -22,7 +22,7 @@ import (
 	"github.com/coreos/flannel/backend"
 	"github.com/coreos/flannel/pkg/ip"
 	"github.com/coreos/flannel/subnet"
-	"github.com/golang/glog"
+	log "github.com/golang/glog"
 	"github.com/vishvananda/netlink"
 	"golang.org/x/net/context"
 )
@@ -53,12 +53,14 @@ func (be *IPIPBackend) RegisterNetwork(ctx context.Context, config *subnet.Confi
 	cfg := struct {
 		DirectRouting bool
 	}{}
+
 	if len(config.Backend) > 0 {
 		if err := json.Unmarshal(config.Backend, &cfg); err != nil {
 			return nil, fmt.Errorf("error decoding IPIP backend config: %v", err)
 		}
 	}
-	glog.Infof("IPIP config: DirectRouting=%v", cfg.DirectRouting)
+
+	log.Infof("IPIP config: DirectRouting=%v", cfg.DirectRouting)
 
 	n := &backend.RouteNetwork{
 		SimpleNetwork: backend.SimpleNetwork{
@@ -82,10 +84,13 @@ func (be *IPIPBackend) RegisterNetwork(ctx context.Context, config *subnet.Confi
 	default:
 		return nil, fmt.Errorf("failed to acquire lease: %v", err)
 	}
+
 	link, err := be.configureIPIPDevice(n.SubnetLease)
+
 	if err != nil {
 		return nil, err
 	}
+
 	n.Mtu = link.MTU
 	n.LinkIndex = link.Index
 	n.GetRoute = func(lease *subnet.Lease) *netlink.Route {
@@ -95,16 +100,20 @@ func (be *IPIPBackend) RegisterNetwork(ctx context.Context, config *subnet.Confi
 			LinkIndex: n.LinkIndex,
 			Flags:     int(netlink.FLAG_ONLINK),
 		}
+
 		if cfg.DirectRouting {
 			dr, err := ip.DirectRouting(lease.Attrs.PublicIP.ToIP())
+
 			if err != nil {
-				glog.Error(err)
+				log.Error(err)
 			}
+
 			if dr {
-				glog.V(2).Infof("configure route to %v via direct routing", lease.Attrs.PublicIP.String())
+				log.V(2).Infof("configure route to %v via direct routing", lease.Attrs.PublicIP.String())
 				route.LinkIndex = n.ExtIface.Iface.Index
 			}
 		}
+
 		return &route
 	}
 
@@ -122,33 +131,39 @@ func (be *IPIPBackend) configureIPIPDevice(lease *subnet.Lease) (*netlink.Iptun,
 	// and set local attribute of flannel.ipip to distinguish these two devices.
 	// Considering tunl0 might be used by users, so choose the later option.
 	link := &netlink.Iptun{LinkAttrs: netlink.LinkAttrs{Name: tunnelName}, Local: be.extIface.IfaceAddr}
+
 	if err := netlink.LinkAdd(link); err != nil {
 		if err != syscall.EEXIST {
 			return nil, err
 		}
+
+		// The link already exists, so check existing link attributes.
 		existing, err := netlink.LinkByName(tunnelName)
 		if err != nil {
 			return nil, err
 		}
-		// flannel will never make the following situations happen. They can only be caused by a user, so get them to sort it out.
+
+		// If there's an exists device but it's not an ipip/IpTun device then get the user to fix it (flannel shouldn't
+		// delete a user's device)
 		if existing.Type() != "ipip" {
-			return nil, fmt.Errorf("%v isn't an ipip mode device, please fix it and try again", tunnelName)
+			return nil, fmt.Errorf("%v isn't an ipip mode device, please remove device and try again", tunnelName)
 		}
 		ipip, ok := existing.(*netlink.Iptun)
 		if !ok {
-			return nil, fmt.Errorf("%s isn't an iptun device %#v", tunnelName, link)
-		}
-		// Don't set remote attribute making flannel.ipip an one to many tunnel device.
-		if ipip.Remote != nil && ipip.Remote.String() != "0.0.0.0" {
-			return nil, fmt.Errorf("remote address %v of tunnel %s is not 0.0.0.0, please fix it and try again", ipip.Remote, tunnelName)
+			return nil, fmt.Errorf("%s isn't an iptun device (%#v), please remove device and try again", tunnelName, link)
 		}
-		// local attribute may change if a user changes iface configuration, we need to recreate the device to ensure local attribute is expected.
-		if ipip.Local == nil || !ipip.Local.Equal(be.extIface.IfaceAddr) {
-			glog.Warningf("%q already exists with incompatable local attribute: %v; recreating device", tunnelName, ipip.Local)
+
+		// local attribute may change if a user changes iface configuration, we need to recreate the device to ensure
+		// local and remote attribute is expected.
+		// local should be equal to the extIface.IfaceAddr and remote should be nil (or equal to 0.0.0.0)
+		if ipip.Local == nil || !ipip.Local.Equal(be.extIface.IfaceAddr) || (ipip.Remote != nil && ipip.Remote.String() != "0.0.0.0") {
+			log.Warningf("%q already exists with incompatable attributes: local=%v remote=%v; recreating device",
+				tunnelName, ipip.Local, ipip.Remote)
 
 			if err = netlink.LinkDel(existing); err != nil {
 				return nil, fmt.Errorf("failed to delete interface: %v", err)
 			}
+
 			if err = netlink.LinkAdd(link); err != nil {
 				return nil, fmt.Errorf("failed to create ipip interface: %v", err)
 			}
@@ -161,10 +176,12 @@ func (be *IPIPBackend) configureIPIPDevice(lease *subnet.Lease) (*netlink.Iptun,
 	if expectMTU <= 0 {
 		return nil, fmt.Errorf("MTU %d of iface %s is too small for ipip mode to work", be.extIface.Iface.MTU, be.extIface.Iface.Name)
 	}
+
 	oldMTU := link.Attrs().MTU
 	if oldMTU > expectMTU || oldMTU == 0 {
-		glog.Infof("current MTU of %s is %d, setting it to %d", tunnelName, oldMTU, expectMTU)
+		log.Infof("current MTU of %s is %d, setting it to %d", tunnelName, oldMTU, expectMTU)
 		err := netlink.LinkSetMTU(link, expectMTU)
+
 		if err != nil {
 			return nil, fmt.Errorf("failed to set %v MTU to %d: %v", tunnelName, expectMTU, err)
 		}
@@ -178,8 +195,10 @@ func (be *IPIPBackend) configureIPIPDevice(lease *subnet.Lease) (*netlink.Iptun,
 	if err := ip.EnsureV4AddressOnLink(ip.IP4Net{IP: lease.Subnet.IP, PrefixLen: 32}, link); err != nil {
 		return nil, fmt.Errorf("failed to ensure address of interface %s: %s", link.Attrs().Name, err)
 	}
+
 	if err := netlink.LinkSetUp(link); err != nil {
 		return nil, fmt.Errorf("failed to set %v UP: %v", tunnelName, err)
 	}
+
 	return link, nil
 }

backend/route_network.go

@@ -0,0 +1,206 @@
+// Copyright 2017 flannel authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package backend
+
+import (
+	"bytes"
+	"net"
+	"sync"
+	"time"
+
+	log "github.com/golang/glog"
+	"golang.org/x/net/context"
+
+	"github.com/coreos/flannel/subnet"
+	"github.com/vishvananda/netlink"
+)
+
+const (
+	routeCheckRetries = 10
+)
+
+type RouteNetwork struct {
+	SimpleNetwork
+	BackendType string
+	routes      []netlink.Route
+	SM          subnet.Manager
+	GetRoute    func(lease *subnet.Lease) *netlink.Route
+	Mtu         int
+	LinkIndex   int
+}
+
+func (n *RouteNetwork) MTU() int {
+	return n.Mtu
+}
+
+func (n *RouteNetwork) Run(ctx context.Context) {
+	wg := sync.WaitGroup{}
+
+	log.Info("Watching for new subnet leases")
+	evts := make(chan []subnet.Event)
+	wg.Add(1)
+	go func() {
+		subnet.WatchLeases(ctx, n.SM, n.SubnetLease, evts)
+		wg.Done()
+	}()
+
+	n.routes = make([]netlink.Route, 0, 10)
+	wg.Add(1)
+	go func() {
+		n.routeCheck(ctx)
+		wg.Done()
+	}()
+
+	defer wg.Wait()
+
+	for {
+		select {
+		case evtBatch := <-evts:
+			n.handleSubnetEvents(evtBatch)
+
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+func (n *RouteNetwork) handleSubnetEvents(batch []subnet.Event) {
+	for _, evt := range batch {
+		switch evt.Type {
+		case subnet.EventAdded:
+			log.Infof("Subnet added: %v via %v", evt.Lease.Subnet, evt.Lease.Attrs.PublicIP)
+
+			if evt.Lease.Attrs.BackendType != n.BackendType {
+				log.Warningf("Ignoring non-%v subnet: type=%v", n.BackendType, evt.Lease.Attrs.BackendType)
+				continue
+			}
+			route := n.GetRoute(&evt.Lease)
+
+			n.addToRouteList(*route)
+			// Check if route exists before attempting to add it
+			routeList, err := netlink.RouteListFiltered(netlink.FAMILY_V4, &netlink.Route{Dst: route.Dst}, netlink.RT_FILTER_DST)
+			if err != nil {
+				log.Warningf("Unable to list routes: %v", err)
+			}
+
+			if len(routeList) > 0 && !routeEqual(routeList[0], *route) {
+				// Same Dst different Gw or different link index. Remove it, correct route will be added below.
+				log.Warningf("Replacing existing route to %v via %v dev index %d with %v via %v dev index %d.", evt.Lease.Subnet, routeList[0].Gw, routeList[0].LinkIndex, evt.Lease.Subnet, evt.Lease.Attrs.PublicIP, route.LinkIndex)
+				if err := netlink.RouteDel(&routeList[0]); err != nil {
+					log.Errorf("Error deleting route to %v: %v", evt.Lease.Subnet, err)
+					continue
+				}
+				n.removeFromRouteList(routeList[0])
+			}
+
+			if len(routeList) > 0 && routeEqual(routeList[0], *route) {
+				// Same Dst and same Gw, keep it and do not attempt to add it.
+				log.Infof("Route to %v via %v dev index %d already exists, skipping.", evt.Lease.Subnet, evt.Lease.Attrs.PublicIP, routeList[0].LinkIndex)
+			} else if err := netlink.RouteAdd(route); err != nil {
+				log.Errorf("Error adding route to %v via %v dev index %d: %v", evt.Lease.Subnet, evt.Lease.Attrs.PublicIP, route.LinkIndex, err)
+				continue
+			}
+
+		case subnet.EventRemoved:
+			log.Info("Subnet removed: ", evt.Lease.Subnet)
+
+			if evt.Lease.Attrs.BackendType != n.BackendType {
+				log.Warningf("Ignoring non-%v subnet: type=%v", n.BackendType, evt.Lease.Attrs.BackendType)
+				continue
+			}
+
+			route := n.GetRoute(&evt.Lease)
+			// Always remove the route from the route list.
+			n.removeFromRouteList(*route)
+
+			if err := netlink.RouteDel(route); err != nil {
+				log.Errorf("Error deleting route to %v: %v", evt.Lease.Subnet, err)
+				continue
+			}
+
+		default:
+			log.Error("Internal error: unknown event type: ", int(evt.Type))
+		}
+	}
+}
+
+func (n *RouteNetwork) addToRouteList(route netlink.Route) {
+	for _, r := range n.routes {
+		if routeEqual(r, route) {
+			return
+		}
+	}
+	n.routes = append(n.routes, route)
+}
+
+func (n *RouteNetwork) removeFromRouteList(route netlink.Route) {
+	for index, r := range n.routes {
+		if routeEqual(r, route) {
+			n.routes = append(n.routes[:index], n.routes[index+1:]...)
+			return
+		}
+	}
+}
+
+func (n *RouteNetwork) routeCheck(ctx context.Context) {
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-time.After(routeCheckRetries * time.Second):
+			n.checkSubnetExistInRoutes()
+		}
+	}
+}
+
+func (n *RouteNetwork) checkSubnetExistInRoutes() {
+	routeList, err := netlink.RouteList(nil, netlink.FAMILY_V4)
+	if err == nil {
+		for _, route := range n.routes {
+			exist := false
+			for _, r := range routeList {
+				if r.Dst == nil {
+					continue
+				}
+				if routeEqual(r, route) {
+					exist = true
+					break
+				}
+			}
+
+			if !exist {
+				if err := netlink.RouteAdd(&route); err != nil {
+					if nerr, ok := err.(net.Error); !ok {
+						log.Errorf("Error recovering route to %v: %v, %v", route.Dst, route.Gw, nerr)
+					}
+					continue
+				} else {
+					log.Infof("Route recovered %v : %v", route.Dst, route.Gw)
+				}
+			}
+		}
+	} else {
+		log.Errorf("Error fetching route list. Will automatically retry: %v", err)
+	}
+}
+
+func routeEqual(x, y netlink.Route) bool {
+	// For ipip backend, when enabling directrouting, link index of some routes may change
+	// For both ipip and host-gw backend, link index may also change if updating ExtIface
+	if x.Dst.IP.Equal(y.Dst.IP) && x.Gw.Equal(y.Gw) && bytes.Equal(x.Dst.Mask, y.Dst.Mask) && x.LinkIndex == y.LinkIndex {
+		return true
+	}
+	return false
+}

backend/simple_network.go

@@ -0,0 +1,38 @@
+// Copyright 2017 flannel authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package backend
+
+import (
+	"golang.org/x/net/context"
+
+	"github.com/coreos/flannel/subnet"
+)
+
+type SimpleNetwork struct {
+	SubnetLease *subnet.Lease
+	ExtIface    *ExternalInterface
+}
+
+func (n *SimpleNetwork) Lease() *subnet.Lease {
+	return n.SubnetLease
+}
+
+func (n *SimpleNetwork) MTU() int {
+	return n.ExtIface.Iface.MTU
+}
+
+func (_ *SimpleNetwork) Run(ctx context.Context) {
+	<-ctx.Done()
+}