Skip to content

Commit f558e21

Browse files
committed
🐛 fix/network/rtb: delete rtb handling err when failed to create routes
Route tables must be deleted after fail to create, otherwise it will generate a new route table every new reconciliator loop. This fix ensure route table is deleted when failed to create, raising a wanring to the recorder when failed to create, and eventually, failed to delete.
1 parent 6afad25 commit f558e21

File tree

1 file changed

+33
-21
lines changed

1 file changed

+33
-21
lines changed

pkg/cloud/services/network/routetables.go

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,32 @@ func (s *Service) describeVpcRouteTablesBySubnet() (map[string]*ec2.RouteTable,
214214
return res, nil
215215
}
216216

217+
func (s *Service) deleteRouteTable(rt *ec2.RouteTable) error {
218+
for _, as := range rt.Associations {
219+
if as.SubnetId == nil {
220+
continue
221+
}
222+
223+
if _, err := s.EC2Client.DisassociateRouteTableWithContext(context.TODO(), &ec2.DisassociateRouteTableInput{AssociationId: as.RouteTableAssociationId}); err != nil {
224+
record.Warnf(s.scope.InfraCluster(), "FailedDisassociateRouteTable", "Failed to disassociate managed RouteTable %q from Subnet %q: %v", *rt.RouteTableId, *as.SubnetId, err)
225+
return errors.Wrapf(err, "failed to disassociate route table %q from subnet %q", *rt.RouteTableId, *as.SubnetId)
226+
}
227+
228+
record.Eventf(s.scope.InfraCluster(), "SuccessfulDisassociateRouteTable", "Disassociated managed RouteTable %q from subnet %q", *rt.RouteTableId, *as.SubnetId)
229+
s.scope.Debug("Deleted association between route table and subnet", "route-table-id", *rt.RouteTableId, "subnet-id", *as.SubnetId)
230+
}
231+
232+
if _, err := s.EC2Client.DeleteRouteTableWithContext(context.TODO(), &ec2.DeleteRouteTableInput{RouteTableId: rt.RouteTableId}); err != nil {
233+
record.Warnf(s.scope.InfraCluster(), "FailedDeleteRouteTable", "Failed to delete managed RouteTable %q: %v", *rt.RouteTableId, err)
234+
return errors.Wrapf(err, "failed to delete route table %q", *rt.RouteTableId)
235+
}
236+
237+
record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteRouteTable", "Deleted managed RouteTable %q", *rt.RouteTableId)
238+
s.scope.Info("Deleted route table", "route-table-id", *rt.RouteTableId)
239+
240+
return nil
241+
}
242+
217243
func (s *Service) deleteRouteTables() error {
218244
if s.scope.VPC().IsUnmanaged(s.scope.Name()) {
219245
s.scope.Trace("Skipping routing tables deletion in unmanaged mode")
@@ -226,27 +252,10 @@ func (s *Service) deleteRouteTables() error {
226252
}
227253

228254
for _, rt := range rts {
229-
for _, as := range rt.Associations {
230-
if as.SubnetId == nil {
231-
continue
232-
}
233-
234-
if _, err := s.EC2Client.DisassociateRouteTableWithContext(context.TODO(), &ec2.DisassociateRouteTableInput{AssociationId: as.RouteTableAssociationId}); err != nil {
235-
record.Warnf(s.scope.InfraCluster(), "FailedDisassociateRouteTable", "Failed to disassociate managed RouteTable %q from Subnet %q: %v", *rt.RouteTableId, *as.SubnetId, err)
236-
return errors.Wrapf(err, "failed to disassociate route table %q from subnet %q", *rt.RouteTableId, *as.SubnetId)
237-
}
238-
239-
record.Eventf(s.scope.InfraCluster(), "SuccessfulDisassociateRouteTable", "Disassociated managed RouteTable %q from subnet %q", *rt.RouteTableId, *as.SubnetId)
240-
s.scope.Debug("Deleted association between route table and subnet", "route-table-id", *rt.RouteTableId, "subnet-id", *as.SubnetId)
241-
}
242-
243-
if _, err := s.EC2Client.DeleteRouteTableWithContext(context.TODO(), &ec2.DeleteRouteTableInput{RouteTableId: rt.RouteTableId}); err != nil {
244-
record.Warnf(s.scope.InfraCluster(), "FailedDeleteRouteTable", "Failed to delete managed RouteTable %q: %v", *rt.RouteTableId, err)
245-
return errors.Wrapf(err, "failed to delete route table %q", *rt.RouteTableId)
255+
err := s.deleteRouteTable(rt)
256+
if err != nil {
257+
return err
246258
}
247-
248-
record.Eventf(s.scope.InfraCluster(), "SuccessfulDeleteRouteTable", "Deleted managed RouteTable %q", *rt.RouteTableId)
249-
s.scope.Info("Deleted route table", "route-table-id", *rt.RouteTableId)
250259
}
251260
return nil
252261
}
@@ -302,8 +311,11 @@ func (s *Service) createRouteTableWithRoutes(routes []*ec2.Route, isPublic bool,
302311
}
303312
return true, nil
304313
}, awserrors.RouteTableNotFound, awserrors.NATGatewayNotFound, awserrors.GatewayNotFound); err != nil {
305-
// TODO(vincepri): cleanup the route table if this fails.
306314
record.Warnf(s.scope.InfraCluster(), "FailedCreateRoute", "Failed to create route %s for RouteTable %q: %v", route.GoString(), *out.RouteTable.RouteTableId, err)
315+
errDel := s.deleteRouteTable(out.RouteTable)
316+
if errDel != nil {
317+
record.Warnf(s.scope.InfraCluster(), "FailedDeleteRouteTable", "Failed to delete managed RouteTable %q: %v", *out.RouteTable.RouteTableId, errDel)
318+
}
307319
return nil, errors.Wrapf(err, "failed to create route in route table %q: %s", *out.RouteTable.RouteTableId, route.GoString())
308320
}
309321
record.Eventf(s.scope.InfraCluster(), "SuccessfulCreateRoute", "Created route %s for RouteTable %q", route.GoString(), *out.RouteTable.RouteTableId)

0 commit comments

Comments
 (0)