From 1f5cf20c0f9276f2dbbac363420560617bc5bee4 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Mon, 30 Apr 2018 11:18:59 -0700 Subject: [PATCH] network: Don't store the network info as pointers if slices used By storing the network information such as routes as pointers, we end up having issues if we try to remove some elements from the list as at the same time this list is parsed through by a loop. For interfaces, we can use maps instead, because we have a way to keep track of them based on their names. Maps are safer and allow to keep pointers in this case. In both cases, routes and interfaces list can be modified properly while we loop over every items. Fixes #226 Signed-off-by: Sebastien Boeuf --- grpc.go | 1 + network.go | 20 ++++++++------------ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/grpc.go b/grpc.go index 0011cfc36d..0a06a7eaf8 100644 --- a/grpc.go +++ b/grpc.go @@ -853,6 +853,7 @@ func (a *agentGRPC) CreateSandbox(ctx context.Context, req *pb.CreateSandboxRequ a.sandbox.id = req.Hostname a.sandbox.containers = make(map[string]*container) + a.sandbox.network.ifaces = make(map[string]*pb.Interface) a.sandbox.network.dns = req.Dns a.sandbox.running = true diff --git a/network.go b/network.go index 5e4e8ed5f1..de951e1cc8 100644 --- a/network.go +++ b/network.go @@ -25,10 +25,10 @@ import ( // related information. type network struct { ifacesLock sync.Mutex - ifaces []*pb.Interface + ifaces map[string]*pb.Interface routesLock sync.Mutex - routes []*pb.Route + routes []pb.Route dns []string } @@ -139,7 +139,7 @@ func (s *sandbox) addInterface(netHandle *netlink.Handle, iface *pb.Interface) ( } // Update sandbox interface list. - s.network.ifaces = append(s.network.ifaces, iface) + s.network.ifaces[iface.Name] = iface return iface, nil } @@ -172,12 +172,7 @@ func (s *sandbox) removeInterface(netHandle *netlink.Handle, iface *pb.Interface } // Update sandbox interface list. - for idx, sIface := range s.network.ifaces { - if sIface.Name == iface.Name { - s.network.ifaces = append(s.network.ifaces[:idx], s.network.ifaces[idx+1:]...) - break - } - } + delete(s.network.ifaces, iface.Name) return nil, nil } @@ -465,7 +460,7 @@ func (s *sandbox) updateRoute(netHandle *netlink.Handle, route *pb.Route, add bo } // Add route to sandbox route list. - s.network.routes = append(s.network.routes, route) + s.network.routes = append(s.network.routes, *route) } else { if err := netHandle.RouteDel(netRoute); err != nil { return grpcStatus.Errorf(codes.Internal, "Could not remove route dest(%s)/gw(%s)/dev(%s): %v", @@ -508,8 +503,9 @@ func (s *sandbox) removeNetwork() error { } defer netHandle.Delete() - for _, route := range s.network.routes { - if err := s.removeRoute(netHandle, route); err != nil { + routeList := s.network.routes + for _, route := range routeList { + if err := s.removeRoute(netHandle, &route); err != nil { return grpcStatus.Errorf(codes.Internal, "Could not remove network route %v: %v", route, err) }