Merge pull request #4900 from mtulio/CORS-3214-refact-rtb

🌱 refactor route discover/creation flow

Author: k8s-ci-robot

pkg/cloud/services/network/routetables.go

@@ -60,30 +60,10 @@ func (s *Service) reconcileRouteTables() error {
 	for i := range subnets {
 		sn := &subnets[i]
 		// We need to compile the minimum routes for this subnet first, so we can compare it or create them.
-		var routes []*ec2.Route
-		if sn.IsPublic {
-			if s.scope.VPC().InternetGatewayID == nil {
-				return errors.Errorf("failed to create routing tables: internet gateway for %q is nil", s.scope.VPC().ID)
-			}
-			routes = append(routes, s.getGatewayPublicRoute())
-			if sn.IsIPv6 {
-				routes = append(routes, s.getGatewayPublicIPv6Route())
-			}
-		} else {
-			natGatewayID, err := s.getNatGatewayForSubnet(sn)
-			if err != nil {
-				return err
-			}
-			routes = append(routes, s.getNatGatewayPrivateRoute(natGatewayID))
-			if sn.IsIPv6 {
-				if !s.scope.VPC().IsIPv6Enabled() {
-					// Safety net because EgressOnlyInternetGateway needs the ID from the ipv6 block.
-					// if, for whatever reason by this point that is not available, we don't want to
-					// panic because of a nil pointer access. This should never occur. Famous last words though.
-					return errors.Errorf("ipv6 block missing for ipv6 enabled subnet, can't create egress only internet gateway")
-				}
-				routes = append(routes, s.getEgressOnlyInternetGateway())
-			}
+		routes, err := s.getRoutesForSubnet(sn)
+		if err != nil {
+			record.Warnf(s.scope.InfraCluster(), "FailedRouteTableRoutes", "Failed to get routes for managed RouteTable for subnet %s: %v", sn.ID, err)
+			return errors.Wrapf(err, "failed to discover routes on route table %s", sn.ID)
 		}
 
 		if rt, ok := subnetRouteMap[sn.GetResourceID()]; ok {
@@ -145,7 +125,7 @@ func (s *Service) reconcileRouteTables() error {
 	return nil
 }
 
-func (s *Service) fixMismatchedRouting(specRoute *ec2.Route, currentRoute *ec2.Route, rt *ec2.RouteTable) error {
+func (s *Service) fixMismatchedRouting(specRoute *ec2.CreateRouteInput, currentRoute *ec2.Route, rt *ec2.RouteTable) error {
 	var input *ec2.ReplaceRouteInput
 	if specRoute.DestinationCidrBlock != nil {
 		if (currentRoute.DestinationCidrBlock != nil &&
@@ -280,7 +260,7 @@ func (s *Service) describeVpcRouteTables() ([]*ec2.RouteTable, error) {
 	return out.RouteTables, nil
 }
 
-func (s *Service) createRouteTableWithRoutes(routes []*ec2.Route, isPublic bool, zone string) (*infrav1.RouteTable, error) {
+func (s *Service) createRouteTableWithRoutes(routes []*ec2.CreateRouteInput, isPublic bool, zone string) (*infrav1.RouteTable, error) {
 	out, err := s.EC2Client.CreateRouteTableWithContext(context.TODO(), &ec2.CreateRouteTableInput{
 		VpcId: aws.String(s.scope.VPC().ID),
 		TagSpecifications: []*ec2.TagSpecification{
@@ -296,17 +276,8 @@ func (s *Service) createRouteTableWithRoutes(routes []*ec2.Route, isPublic bool,
 	for i := range routes {
 		route := routes[i]
 		if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) {
-			if _, err := s.EC2Client.CreateRouteWithContext(context.TODO(), &ec2.CreateRouteInput{
-				RouteTableId:                out.RouteTable.RouteTableId,
-				DestinationCidrBlock:        route.DestinationCidrBlock,
-				DestinationIpv6CidrBlock:    route.DestinationIpv6CidrBlock,
-				EgressOnlyInternetGatewayId: route.EgressOnlyInternetGatewayId,
-				GatewayId:                   route.GatewayId,
-				InstanceId:                  route.InstanceId,
-				NatGatewayId:                route.NatGatewayId,
-				NetworkInterfaceId:          route.NetworkInterfaceId,
-				VpcPeeringConnectionId:      route.VpcPeeringConnectionId,
-			}); err != nil {
+			route.RouteTableId = out.RouteTable.RouteTableId
+			if _, err := s.EC2Client.CreateRouteWithContext(context.TODO(), route); err != nil {
 				return false, err
 			}
 			return true, nil
@@ -341,29 +312,29 @@ func (s *Service) associateRouteTable(rt *infrav1.RouteTable, subnetID string) e
 	return nil
 }
 
-func (s *Service) getNatGatewayPrivateRoute(natGatewayID string) *ec2.Route {
-	return &ec2.Route{
+func (s *Service) getNatGatewayPrivateRoute(natGatewayID string) *ec2.CreateRouteInput {
+	return &ec2.CreateRouteInput{
 		NatGatewayId:         aws.String(natGatewayID),
 		DestinationCidrBlock: aws.String(services.AnyIPv4CidrBlock),
 	}
 }
 
-func (s *Service) getEgressOnlyInternetGateway() *ec2.Route {
-	return &ec2.Route{
+func (s *Service) getEgressOnlyInternetGateway() *ec2.CreateRouteInput {
+	return &ec2.CreateRouteInput{
 		DestinationIpv6CidrBlock:    aws.String(services.AnyIPv6CidrBlock),
 		EgressOnlyInternetGatewayId: s.scope.VPC().IPv6.EgressOnlyInternetGatewayID,
 	}
 }
 
-func (s *Service) getGatewayPublicRoute() *ec2.Route {
-	return &ec2.Route{
+func (s *Service) getGatewayPublicRoute() *ec2.CreateRouteInput {
+	return &ec2.CreateRouteInput{
 		DestinationCidrBlock: aws.String(services.AnyIPv4CidrBlock),
 		GatewayId:            aws.String(*s.scope.VPC().InternetGatewayID),
 	}
 }
 
-func (s *Service) getGatewayPublicIPv6Route() *ec2.Route {
-	return &ec2.Route{
+func (s *Service) getGatewayPublicIPv6Route() *ec2.CreateRouteInput {
+	return &ec2.CreateRouteInput{
 		DestinationIpv6CidrBlock: aws.String(services.AnyIPv6CidrBlock),
 		GatewayId:                aws.String(*s.scope.VPC().InternetGatewayID),
 	}
@@ -394,3 +365,45 @@ func (s *Service) getRouteTableTagParams(id string, public bool, zone string) in
 		Additional:  additionalTags,
 	}
 }
+
+func (s *Service) getRoutesToPublicSubnet(sn *infrav1.SubnetSpec) ([]*ec2.CreateRouteInput, error) {
+	var routes []*ec2.CreateRouteInput
+
+	if s.scope.VPC().InternetGatewayID == nil {
+		return routes, errors.Errorf("failed to create routing tables: internet gateway for %q is nil", s.scope.VPC().ID)
+	}
+
+	routes = append(routes, s.getGatewayPublicRoute())
+	if sn.IsIPv6 {
+		routes = append(routes, s.getGatewayPublicIPv6Route())
+	}
+
+	return routes, nil
+}
+
+func (s *Service) getRoutesToPrivateSubnet(sn *infrav1.SubnetSpec) (routes []*ec2.CreateRouteInput, err error) {
+	natGatewayID, err := s.getNatGatewayForSubnet(sn)
+	if err != nil {
+		return routes, err
+	}
+
+	routes = append(routes, s.getNatGatewayPrivateRoute(natGatewayID))
+	if sn.IsIPv6 {
+		if !s.scope.VPC().IsIPv6Enabled() {
+			// Safety net because EgressOnlyInternetGateway needs the ID from the ipv6 block.
+			// if, for whatever reason by this point that is not available, we don't want to
+			// panic because of a nil pointer access. This should never occur. Famous last words though.
+			return routes, errors.Errorf("ipv6 block missing for ipv6 enabled subnet, can't create route for egress only internet gateway")
+		}
+		routes = append(routes, s.getEgressOnlyInternetGateway())
+	}
+
+	return routes, nil
+}
+
+func (s *Service) getRoutesForSubnet(sn *infrav1.SubnetSpec) ([]*ec2.CreateRouteInput, error) {
+	if sn.IsPublic {
+		return s.getRoutesToPublicSubnet(sn)
+	}
+	return s.getRoutesToPrivateSubnet(sn)
+}