Skip to content

Commit

Permalink
agent: network: Don't loop over a list being potentially modified
Browse files Browse the repository at this point in the history
When the pod is destroyed, it removes all the routes and interfaces
previously setup. The list of routes and interfaces is maintained
by the agent, referring to list of pointers. When the agent tries
to rely on this list to remove routes and interfaces, the list is
updated by the code removing them, leading to weird situations
where the loop does not properly go over the expected items of the
list.

This commit duplicates the list of routes and interfaces as non
pointers, so that we make sure that any removal from the function
UpdateRoute() or UpdateInterface() will not result in modifying
the initial list being parsed.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <[email protected]>
  • Loading branch information
Sebastien Boeuf committed Apr 26, 2018
1 parent 74e720e commit 1a653c9
Showing 1 changed file with 27 additions and 13 deletions.
40 changes: 27 additions & 13 deletions network.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,19 +508,33 @@ func (s *sandbox) removeNetwork() error {
}
defer netHandle.Delete()

for _, route := range s.network.routes {
if err := s.removeRoute(netHandle, route); err != nil {
return grpcStatus.Errorf(codes.Internal, "Could not remove network route %v: %v",
route, err)
}
}

for _, iface := range s.network.ifaces {
if _, err := s.removeInterface(netHandle, iface); err != nil {
return grpcStatus.Errorf(codes.Internal, "Could not remove network interface %v: %v",
iface, err)
}
}
// Create a duplicate of the route list, with no pointers,
// otherwise this leads to a mix when routes are removed.
var routeList []pb.Route
for _, route := range s.network.routes {
routeList = append(routeList, *route)
}

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)
}
}

// Create a duplicate of the interface list, with no pointers,
// otherwise this leads to a mix when interfaces are removed.
var ifaceList []pb.Interface
for _, iface := range s.network.ifaces {
ifaceList = append(ifaceList, *iface)
}

for _, iface := range ifaceList {
if _, err := s.removeInterface(netHandle, &iface); err != nil {
return grpcStatus.Errorf(codes.Internal, "Could not remove network interface %v: %v",
iface, err)
}
}

if err := removeDNS(s.network.dns); err != nil {
return grpcStatus.Errorf(codes.Internal, "Could not remove network DNS: %v", err)
Expand Down

0 comments on commit 1a653c9

Please sign in to comment.