network: While updating routes, do not delete routes with proto "kernel"

Routes with proto kernel are automatically added by the kernel
when an interface is added with ip assigned as a subnet address.
With github.com/#1936, these routes will no
longer be sent from the runtime.

Depends-on: github.com/#1936

Fixes: #623

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>

network.go

@@ -11,6 +11,7 @@ import (
 	"net"
 	"os"
 	"reflect"
+	"strings"
 	"sync"
 
 	"golang.org/x/sys/unix"
@@ -356,6 +357,11 @@ func (s *sandbox) deleteRoutes(netHandle *netlink.Handle) error {
 			continue
 		}
 
+		// If route is installed by kernel, do not delete
+		if initRoute.Protocol == unix.RTPROT_KERNEL {
+			continue
+		}
+
 		err = netHandle.RouteDel(&initRoute)
 		if err != nil {
 			//If there was an error deleting some of the initial routes,
@@ -528,6 +534,14 @@ func (s *sandbox) processRoute(netHandle *netlink.Handle, route *types.Route) (*
 	return netRoute, nil
 }
 
+func checkDuplicateRoute(rt, netRoute *netlink.Route) bool {
+	if rt.Dst == nil {
+		return netRoute.Dst == nil && rt.Gw.Equal(netRoute.Gw)
+	}
+
+	return netRoute.Dst != nil && (rt.Dst.String() == netRoute.Dst.String()) && rt.Src.Equal(netRoute.Src)
+}
+
 func (s *sandbox) updateRoute(netHandle *netlink.Handle, route *types.Route, add bool) (err error) {
 	s.network.routesLock.Lock()
 	defer s.network.routesLock.Unlock()
@@ -547,10 +561,30 @@ func (s *sandbox) updateRoute(netHandle *netlink.Handle, route *types.Route, add
 
 	if add {
 		if err := netHandle.RouteAdd(netRoute); err != nil {
-			return grpcStatus.Errorf(codes.Internal, "Could not add route dest(%s)/gw(%s)/dev(%s): %v",
+			// Doing a string comparison here for lack of error codes from upstream
+			if strings.Contains(err.Error(), "file exists") {
+				agentLog.Infof("Route exists, will try to delete duplicate route first")
+
+				rts, _ := netHandle.RouteList(nil, netlink.FAMILY_ALL)
+				for _, rt := range rts {
+					if checkDuplicateRoute(&rt, netRoute) {
+						// Delete route first
+						if err := netHandle.RouteDel(&rt); err != nil {
+							return grpcStatus.Errorf(codes.Internal, "Could not add route dest(%s)/gw(%s)/dev(%s), duplicate route exists, error deleting existing route: %v", route.Dest, route.Gateway, route.Device, err)
+						}
+
+						if err := netHandle.RouteAdd(netRoute); err != nil {
+							break
+						}
+
+						s.network.routes = append(s.network.routes, *route)
+						return nil
+					}
+				}
+			}
+			return grpcStatus.Errorf(codes.Internal, "Could not add/replace route dest(%s)/gw(%s)/dev(%s): %v",
 				route.Dest, route.Gateway, route.Device, err)
 		}
-
 		// Add route to sandbox route list.
 		s.network.routes = append(s.network.routes, *route)
 	} else {

network_test.go

@@ -307,16 +307,43 @@ func TestListRoutes(t *testing.T) {
 	//Test a simple route setup:
 	inputRoutesSimple := []*types.Route{
 		{Dest: "", Gateway: "192.168.0.1", Source: "", Scope: 0, Device: "ifc-name"},
+	}
+
+	expectedRoutes := []*types.Route{
+		{Dest: "", Gateway: "192.168.0.1", Source: "", Scope: 0, Device: "ifc-name"},
+		// This route is auto-added by kernel, and we no longer delete kernel proto routes
 		{Dest: "192.168.0.0/16", Gateway: "", Source: "192.168.0.2", Scope: 253, Device: "ifc-name"},
 	}
 
 	testRoutes := &pb.Routes{
 		Routes: inputRoutesSimple,
 	}
 
-	s.updateRoutes(netHandle, testRoutes)
+	_, err := s.updateRoutes(netHandle, testRoutes)
+	assert.Nil(err)
 	results, err := s.listRoutes(nil)
 	assert.Nil(err, "Expected to list all routes")
+
+	assert.True(reflect.DeepEqual(results.Routes[0], expectedRoutes[0]),
+		"Route listed didn't match: got %+v, expecting %+v", results.Routes[0], expectedRoutes[0])
+	assert.True(reflect.DeepEqual(results.Routes[1], expectedRoutes[1]),
+		"Route listed didn't match: got %+v, expecting %+v", results.Routes[1], expectedRoutes[1])
+
+	inputRoutesSimple = []*types.Route{
+		{Dest: "", Gateway: "192.168.0.1", Source: "", Scope: 0, Device: "ifc-name"},
+		// This works too, in case a duplicate route added by kernel exists, this route will over-ride it
+		{Dest: "192.168.0.0/16", Gateway: "", Source: "192.168.0.2", Scope: 253, Device: "ifc-name"},
+	}
+
+	testRoutes = &pb.Routes{
+		Routes: inputRoutesSimple,
+	}
+
+	_, err = s.updateRoutes(netHandle, testRoutes)
+	assert.Nil(err)
+	results, err = s.listRoutes(nil)
+	assert.Nil(err, "Expected to list all routes")
+
 	assert.True(reflect.DeepEqual(results.Routes[0], inputRoutesSimple[0]),
 		"Route listed didn't match: got %+v, expecting %+v", results.Routes[0], inputRoutesSimple[0])
 	assert.True(reflect.DeepEqual(results.Routes[1], inputRoutesSimple[1]),