diff --git a/network.go b/network.go index 1337c71c4..5885e47be 100644 --- a/network.go +++ b/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 { diff --git a/network_test.go b/network_test.go index 36af7d85c..1d955661f 100644 --- a/network_test.go +++ b/network_test.go @@ -307,6 +307,11 @@ 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"}, } @@ -314,9 +319,31 @@ func TestListRoutes(t *testing.T) { 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]),