From de15b859ba082b06eebceb3e2c55c37e7ce820e8 Mon Sep 17 00:00:00 2001 From: Fabian Ruff Date: Mon, 16 Apr 2018 17:46:57 +0200 Subject: [PATCH 1/2] fix route deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a follow-up to #56258 which only half of the work done. The DeleteRoute method failed to delete routes when it can’t find the corresponding node in OpenStack. --- .../providers/openstack/openstack_routes.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_routes.go b/pkg/cloudprovider/providers/openstack/openstack_routes.go index 8af30d9202b..4504523ed7c 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_routes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_routes.go @@ -225,10 +225,16 @@ func (r *Routes) DeleteRoute(ctx context.Context, clusterName string, route *clo ip, _, _ := net.ParseCIDR(route.DestinationCIDR) isCIDRv6 := ip.To4() == nil - addr, err := getAddressByName(r.compute, route.TargetNode, isCIDRv6) - if err != nil { - return err + var addr string + + // Blackhole routes are orphaned and have no target in OpenStack + if !route.Blackhole { + var err error + addr, err = getAddressByName(r.compute, route.TargetNode, isCIDRv6) + if err != nil { + return err + } } router, err := routers.Get(r.network, r.opts.RouterID).Extract() @@ -239,7 +245,7 @@ func (r *Routes) DeleteRoute(ctx context.Context, clusterName string, route *clo routes := router.Routes index := -1 for i, item := range routes { - if item.DestinationCIDR == route.DestinationCIDR && item.NextHop == addr { + if item.DestinationCIDR == route.DestinationCIDR && (route.Blackhole || item.NextHop == addr) { index = i break } @@ -255,7 +261,8 @@ func (r *Routes) DeleteRoute(ctx context.Context, clusterName string, route *clo routes = routes[:len(routes)-1] unwind, err := updateRoutes(r.network, router, routes) - if err != nil { + // If this was a blackhole route we are done, there are no ports to update + if err != nil || route.Blackhole { return err } defer onFailure.call(unwind) From 65d4147139a4c1a485f2fdf92695675d22aca001 Mon Sep 17 00:00:00 2001 From: Fabian Ruff Date: Wed, 18 Apr 2018 13:15:41 +0200 Subject: [PATCH 2/2] ensure we delete orphaned routes with matching next-hops only There might be a valid route with the same DestinationCIDR pointing to a running node. --- .../providers/openstack/openstack_routes.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/openstack_routes.go b/pkg/cloudprovider/providers/openstack/openstack_routes.go index 4504523ed7c..3f41c7339d9 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_routes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_routes.go @@ -83,9 +83,12 @@ func (r *Routes) ListRoutes(ctx context.Context, clusterName string) ([]*cloudpr var routes []*cloudprovider.Route for _, item := range router.Routes { nodeName, foundNode := nodeNamesByAddr[item.NextHop] + if !foundNode { + nodeName = types.NodeName(item.NextHop) + } route := cloudprovider.Route{ Name: item.DestinationCIDR, - TargetNode: nodeName, //empty if NextHop is unknown + TargetNode: nodeName, //contains the nexthop address if node was not found Blackhole: !foundNode, DestinationCIDR: item.DestinationCIDR, } @@ -228,7 +231,7 @@ func (r *Routes) DeleteRoute(ctx context.Context, clusterName string, route *clo var addr string - // Blackhole routes are orphaned and have no target in OpenStack + // Blackhole routes are orphaned and have no counterpart in OpenStack if !route.Blackhole { var err error addr, err = getAddressByName(r.compute, route.TargetNode, isCIDRv6) @@ -245,7 +248,7 @@ func (r *Routes) DeleteRoute(ctx context.Context, clusterName string, route *clo routes := router.Routes index := -1 for i, item := range routes { - if item.DestinationCIDR == route.DestinationCIDR && (route.Blackhole || item.NextHop == addr) { + if item.DestinationCIDR == route.DestinationCIDR && (item.NextHop == addr || route.Blackhole && item.NextHop == string(route.TargetNode)) { index = i break }