Skip to content

Commit

Permalink
Remove unneeded API calls
Browse files Browse the repository at this point in the history 
The API call to describe the route is not needed, as the route ref
contains all the information needed to delete routes.
  • Loading branch information
@dhild
dhild committed Apr 16, 2024
1 parent 9db2860 commit 9656bdb
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 64 deletions.
26 changes: 18 additions & 8 deletions pkg/virtualrouter/routes_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,7 @@ func (m *defaultRoutesManager) remove(ctx context.Context, ms *appmesh.Mesh, sdk
// Only reconcile routes which need to be removed before we remove the corresponding listener
taintedRefs := taintedSDKRouteRefs(vr.Spec.Routes, sdkVR, sdkRouteRefs)
for _, sdkRouteRef := range taintedRefs {
sdkRoute, err := m.findSDKRoute(ctx, sdkRouteRef)
if err != nil {
return err
}
if sdkRoute == nil {
return errors.Errorf("route not found: %v", aws.StringValue(sdkRouteRef.RouteName))
}
if err = m.deleteSDKRoute(ctx, sdkRoute); err != nil {
if err = m.deleteSDKRouteByRef(ctx, sdkRouteRef); err != nil {
return err
}
}
Expand Down Expand Up @@ -230,6 +223,23 @@ func (m *defaultRoutesManager) deleteSDKRoute(ctx context.Context, sdkRoute *app
return nil
}

func (m *defaultRoutesManager) deleteSDKRouteByRef(ctx context.Context, sdkRouteRef *appmeshsdk.RouteRef) error {
_, err := m.appMeshSDK.DeleteRouteWithContext(ctx, &appmeshsdk.DeleteRouteInput{
MeshName: sdkRouteRef.MeshName,
MeshOwner: sdkRouteRef.MeshOwner,
VirtualRouterName: sdkRouteRef.VirtualRouterName,
RouteName: sdkRouteRef.RouteName,
})
if err != nil {
var awsErr awserr.Error
if ok := errors.As(err, &awsErr); ok && awsErr.Code() == "NotFoundException" {
return nil
}
return err
}
return nil
}

type routeAndSDKRouteRef struct {
route appmesh.Route
sdkRouteRef *appmeshsdk.RouteRef
Expand Down
90 changes: 34 additions & 56 deletions pkg/virtualrouter/routes_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1066,22 +1066,15 @@ func Test_defaultRoutesManager_remove(t *testing.T) {
}
tests := []struct {
name string
sdkRoutes []*appmeshsdk.RouteData
sdkRouteRefs []*appmeshsdk.RouteRef
args args
wantDeleteRoutes []*appmeshsdk.DeleteRouteInput
}{
{
name: "preserve existing unchanged routes",
sdkRoutes: []*appmeshsdk.RouteData{
name: "preserve existing matching routes",
sdkRouteRefs: []*appmeshsdk.RouteRef{
{
RouteName: aws.String("route-1"),
Spec: &appmeshsdk.RouteSpec{
TcpRoute: &appmeshsdk.TcpRoute{
Match: &appmeshsdk.TcpRouteMatch{
Port: aws.Int64(8000),
},
},
},
},
},
args: args{
Expand Down Expand Up @@ -1116,22 +1109,13 @@ func Test_defaultRoutesManager_remove(t *testing.T) {
wantDeleteRoutes: []*appmeshsdk.DeleteRouteInput{},
},
{
name: "tainted routes are removed",
sdkRoutes: []*appmeshsdk.RouteData{
name: "routes with changes are removed",
sdkRouteRefs: []*appmeshsdk.RouteRef{
{
RouteName: aws.String("route-1"),
Metadata: &appmeshsdk.ResourceMetadata{
MeshOwner: aws.String("111111111111"),
},
MeshName: aws.String("mesh-1"),
VirtualRouterName: aws.String("virtual-router-1"),
Spec: &appmeshsdk.RouteSpec{
TcpRoute: &appmeshsdk.TcpRoute{
Match: &appmeshsdk.TcpRouteMatch{
Port: aws.Int64(8000),
},
},
},
},
{
RouteName: aws.String("route-2"),
},
},
args: args{
Expand All @@ -1145,6 +1129,12 @@ func Test_defaultRoutesManager_remove(t *testing.T) {
Protocol: aws.String("tcp"),
},
},
{
PortMapping: &appmeshsdk.PortMapping{
Port: aws.Int64(9000),
Protocol: aws.String("http"),
},
},
},
},
},
Expand All @@ -1159,24 +1149,32 @@ func Test_defaultRoutesManager_remove(t *testing.T) {
},
},
},
{
Name: "route-2",
HTTPRoute: &appmesh.HTTPRoute{
Match: appmesh.HTTPRouteMatch{
Port: aws.Int64(8080),
},
},
},
},
},
},
},
wantDeleteRoutes: []*appmeshsdk.DeleteRouteInput{
{
MeshName: aws.String("mesh-1"),
MeshOwner: aws.String("111111111111"),
RouteName: aws.String("route-1"),
VirtualRouterName: aws.String("virtual-router-1"),
RouteName: aws.String("route-1"),
},
{
RouteName: aws.String("route-2"),
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := &fakeAppMesh{
existingRoutes: tt.sdkRoutes,
existingRouteRefs: tt.sdkRouteRefs,
}
m := &defaultRoutesManager{
appMeshSDK: f,
Expand All @@ -1194,46 +1192,26 @@ func Test_defaultRoutesManager_remove(t *testing.T) {
type fakeAppMesh struct {
services.AppMesh

existingRoutes []*appmeshsdk.RouteData
deletedRoutes []*appmeshsdk.DeleteRouteInput
existingRouteRefs []*appmeshsdk.RouteRef
deletedRoutes []*appmeshsdk.DeleteRouteInput
}

func (f *fakeAppMesh) ListRoutesPagesWithContext(_ aws.Context, _ *appmeshsdk.ListRoutesInput, callback func(*appmeshsdk.ListRoutesOutput, bool) bool, _ ...request.Option) error {
if len(f.existingRoutes) > 0 {
routes := make([]*appmeshsdk.RouteRef, len(f.existingRoutes))
for i, route := range f.existingRoutes {
routes[i] = &appmeshsdk.RouteRef{
RouteName: route.RouteName,
}
}
if len(f.existingRouteRefs) > 0 {
callback(&appmeshsdk.ListRoutesOutput{
Routes: routes,
Routes: f.existingRouteRefs,
}, false)
}
return nil
}

func (f *fakeAppMesh) DescribeRouteWithContext(_ aws.Context, params *appmeshsdk.DescribeRouteInput, _ ...request.Option) (*appmeshsdk.DescribeRouteOutput, error) {
for _, route := range f.existingRoutes {
if aws.StringValue(route.RouteName) != aws.StringValue(params.RouteName) {
continue
}
return &appmeshsdk.DescribeRouteOutput{
Route: route,
}, nil
}
return nil, fmt.Errorf("not found")
}

func (f *fakeAppMesh) DeleteRouteWithContext(_ aws.Context, params *appmeshsdk.DeleteRouteInput, _ ...request.Option) (*appmeshsdk.DeleteRouteOutput, error) {
f.deletedRoutes = append(f.deletedRoutes, params)
for _, route := range f.existingRoutes {
if aws.StringValue(route.RouteName) != aws.StringValue(params.RouteName) {
for _, ref := range f.existingRouteRefs {
if aws.StringValue(ref.RouteName) != aws.StringValue(params.RouteName) {
continue
}
return &appmeshsdk.DeleteRouteOutput{
Route: route,
}, nil
return &appmeshsdk.DeleteRouteOutput{}, nil
}
return nil, fmt.Errorf("not found")
}

0 comments on commit 9656bdb

Please sign in to comment.