improve missing service handling for targetGroupBinding (#1888)

Author: M00nF1sh

main.go

@@ -104,7 +104,7 @@ func main() {
 	sgReconciler := networking.NewDefaultSecurityGroupReconciler(sgManager, ctrl.Log)
 	subnetResolver := networking.NewDefaultSubnetsResolver(cloud.EC2(), cloud.VpcID(), controllerCFG.ClusterName, ctrl.Log.WithName("subnets-resolver"))
 	tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(),
-		podInfoRepo, podENIResolver, nodeENIResolver, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, ctrl.Log)
+		podInfoRepo, podENIResolver, nodeENIResolver, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
 	ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),
 		finalizerManager, sgManager, sgReconciler, subnetResolver,
 		controllerCFG, ctrl.Log.WithName("controllers").WithName("ingress"))

pkg/backend/endpoint_resolver.go

@@ -2,15 +2,19 @@ package backend
 
 import (
 	"context"
+	"fmt"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+var ErrNotFound = errors.New("backend not found")
+
 // TODO: for pod endpoints, we currently rely on endpoints events, we might change to use pod events directly in the future.
 // under current implementation with pod readinessGate enabled, an unready endpoint but not match our inclusionCriteria won't be registered,
 // and it won't turn ready due to blocked by readinessGate, and no future endpoint events will trigger.
@@ -55,9 +59,13 @@ func (r *defaultEndpointResolver) ResolvePodEndpoints(ctx context.Context, svcKe
 	if err != nil {
 		return nil, false, err
 	}
+
 	epsKey := k8s.NamespacedName(svc) // k8s Endpoints have same name as k8s Service
 	eps := &corev1.Endpoints{}
 	if err := r.k8sClient.Get(ctx, epsKey, eps); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, false, fmt.Errorf("%w: %v", ErrNotFound, err.Error())
+		}
 		return nil, false, err
 	}
 
@@ -149,11 +157,14 @@ func (r *defaultEndpointResolver) ResolveNodePortEndpoints(ctx context.Context,
 func (r *defaultEndpointResolver) findServiceAndServicePort(ctx context.Context, svcKey types.NamespacedName, port intstr.IntOrString) (*corev1.Service, corev1.ServicePort, error) {
 	svc := &corev1.Service{}
 	if err := r.k8sClient.Get(ctx, svcKey, svc); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, corev1.ServicePort{}, fmt.Errorf("%w: %v", ErrNotFound, err.Error())
+		}
 		return nil, corev1.ServicePort{}, err
 	}
 	svcPort, err := k8s.LookupServicePort(svc, port)
 	if err != nil {
-		return nil, corev1.ServicePort{}, err
+		return nil, corev1.ServicePort{}, fmt.Errorf("%w: %v", ErrNotFound, err.Error())
 	}
 
 	return svc, svcPort, nil

pkg/backend/endpoint_resolver_test.go

@@ -3,6 +3,7 @@ package backend
 import (
 	"context"
 	"errors"
+	"fmt"
 	"github.com/golang/mock/gomock"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -131,6 +132,21 @@ func Test_defaultEndpointResolver_ResolvePodEndpoints(t *testing.T) {
 			},
 		},
 	}
+	svc1WithoutHTTPPort := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: testNS,
+			Name:      "svc-1",
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeClusterIP,
+			Ports: []corev1.ServicePort{
+				{
+					Name: "https",
+					Port: 443,
+				},
+			},
+		},
+	}
 	ep1A := &corev1.Endpoints{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: testNS,
@@ -704,6 +720,56 @@ func Test_defaultEndpointResolver_ResolvePodEndpoints(t *testing.T) {
 			},
 			wantContainsPotentialReadyEndpoints: true,
 		},
+		{
+			name: "service not found",
+			env: env{
+				services:      []*corev1.Service{},
+				endpointsList: []*corev1.Endpoints{},
+			},
+			fields: fields{
+				podInfoRepoGetCalls: []podInfoRepoGetCall{},
+			},
+			args: args{
+				svcKey: k8s.NamespacedName(svc1),
+				port:   intstr.FromString("http"),
+				opts:   nil,
+			},
+			want:                                []PodEndpoint{},
+			wantContainsPotentialReadyEndpoints: false,
+			wantErr:                             fmt.Errorf("%w: %v", ErrNotFound, "services \"svc-1\" not found"),
+		},
+		{
+			name: "service port not found",
+			env: env{
+				services:      []*corev1.Service{svc1WithoutHTTPPort},
+				endpointsList: []*corev1.Endpoints{},
+			},
+			fields: fields{
+				podInfoRepoGetCalls: []podInfoRepoGetCall{},
+			},
+			args: args{
+				svcKey: k8s.NamespacedName(svc1),
+				port:   intstr.FromString("http"),
+				opts:   nil,
+			},
+			wantErr: fmt.Errorf("%w: %v", ErrNotFound, "unable to find port http on service test-ns/svc-1"),
+		},
+		{
+			name: "endpoints not found",
+			env: env{
+				services:      []*corev1.Service{svc1},
+				endpointsList: []*corev1.Endpoints{},
+			},
+			fields: fields{
+				podInfoRepoGetCalls: []podInfoRepoGetCall{},
+			},
+			args: args{
+				svcKey: k8s.NamespacedName(svc1),
+				port:   intstr.FromString("http"),
+				opts:   nil,
+			},
+			wantErr: fmt.Errorf("%w: %v", ErrNotFound, "endpoints \"svc-1\" not found"),
+		},
 	}
 
 	for _, tt := range tests {
@@ -850,6 +916,21 @@ func Test_defaultEndpointResolver_ResolveNodePortEndpoints(t *testing.T) {
 			},
 		},
 	}
+	svc1WithoutHTTPPort := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: testNS,
+			Name:      "svc-1",
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeClusterIP,
+			Ports: []corev1.ServicePort{
+				{
+					Name: "https",
+					Port: 443,
+				},
+			},
+		},
+	}
 	svc2 := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: testNS,
@@ -957,6 +1038,32 @@ func Test_defaultEndpointResolver_ResolveNodePortEndpoints(t *testing.T) {
 			},
 			wantErr: errors.New("service type must be either 'NodePort' or 'LoadBalancer': test-ns/svc-2"),
 		},
+		{
+			name: "service not found",
+			env: env{
+				nodes:    []*corev1.Node{node1, node2, node3, node4},
+				services: []*corev1.Service{},
+			},
+			args: args{
+				svcKey: k8s.NamespacedName(svc1),
+				port:   intstr.FromString("http"),
+				opts:   []EndpointResolveOption{WithNodeSelector(labels.Everything())},
+			},
+			wantErr: fmt.Errorf("%w: %v", ErrNotFound, "services \"svc-1\" not found"),
+		},
+		{
+			name: "service port not found",
+			env: env{
+				nodes:    []*corev1.Node{node1, node2, node3, node4},
+				services: []*corev1.Service{svc1WithoutHTTPPort},
+			},
+			args: args{
+				svcKey: k8s.NamespacedName(svc1),
+				port:   intstr.FromString("http"),
+				opts:   []EndpointResolveOption{WithNodeSelector(labels.Everything())},
+			},
+			wantErr: fmt.Errorf("%w: %v", ErrNotFound, "unable to find port http on service test-ns/svc-1"),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/k8s/events.go

@@ -24,5 +24,6 @@ const (
 	TargetGroupBindingEventReasonFailedRemoveFinalizer  = "FailedRemoveFinalizer"
 	TargetGroupBindingEventReasonFailedUpdateStatus     = "FailedUpdateStatus"
 	TargetGroupBindingEventReasonFailedCleanup          = "FailedCleanup"
+	TargetGroupBindingEventReasonBackendNotFound        = "BackendNotFound"
 	TargetGroupBindingEventReasonSuccessfullyReconciled = "SuccessfullyReconciled"
 )

pkg/targetgroupbinding/resource_manager.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"k8s.io/client-go/tools/record"
 	"time"
 
 	awssdk "github.com/aws/aws-sdk-go/aws"
@@ -38,7 +39,7 @@ type ResourceManager interface {
 func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELBV2,
 	podInfoRepo k8s.PodInfoRepo, podENIResolver networking.PodENIInfoResolver, nodeENIResolver networking.NodeENIInfoResolver,
 	sgManager networking.SecurityGroupManager, sgReconciler networking.SecurityGroupReconciler,
-	vpcID string, clusterName string, logger logr.Logger) *defaultResourceManager {
+	vpcID string, clusterName string, eventRecorder record.EventRecorder, logger logr.Logger) *defaultResourceManager {
 	targetsManager := NewCachedTargetsManager(elbv2Client, logger)
 	endpointResolver := backend.NewDefaultEndpointResolver(k8sClient, podInfoRepo, logger)
 	networkingManager := NewDefaultNetworkingManager(k8sClient, podENIResolver, nodeENIResolver, sgManager, sgReconciler, vpcID, clusterName, logger)
@@ -47,6 +48,7 @@ func NewDefaultResourceManager(k8sClient client.Client, elbv2Client services.ELB
 		targetsManager:    targetsManager,
 		endpointResolver:  endpointResolver,
 		networkingManager: networkingManager,
+		eventRecorder:     eventRecorder,
 		logger:            logger,
 
 		targetHealthRequeueDuration: defaultTargetHealthRequeueDuration,
@@ -61,6 +63,7 @@ type defaultResourceManager struct {
 	targetsManager    TargetsManager
 	endpointResolver  backend.EndpointResolver
 	networkingManager NetworkingManager
+	eventRecorder     record.EventRecorder
 	logger            logr.Logger
 
 	targetHealthRequeueDuration time.Duration
@@ -95,6 +98,10 @@ func (m *defaultResourceManager) reconcileWithIPTargetType(ctx context.Context,
 	}
 	endpoints, containsPotentialReadyEndpoints, err := m.endpointResolver.ResolvePodEndpoints(ctx, svcKey, tgb.Spec.ServiceRef.Port, resolveOpts...)
 	if err != nil {
+		if errors.Is(err, backend.ErrNotFound) {
+			m.eventRecorder.Event(tgb, corev1.EventTypeWarning, k8s.TargetGroupBindingEventReasonBackendNotFound, err.Error())
+			return m.Cleanup(ctx, tgb)
+		}
 		return err
 	}
 
@@ -146,6 +153,10 @@ func (m *defaultResourceManager) reconcileWithInstanceTargetType(ctx context.Con
 	resolveOpts := []backend.EndpointResolveOption{backend.WithNodeSelector(nodeSelector)}
 	endpoints, err := m.endpointResolver.ResolveNodePortEndpoints(ctx, svcKey, tgb.Spec.ServiceRef.Port, resolveOpts...)
 	if err != nil {
+		if errors.Is(err, backend.ErrNotFound) {
+			m.eventRecorder.Event(tgb, corev1.EventTypeWarning, k8s.TargetGroupBindingEventReasonBackendNotFound, err.Error())
+			return m.Cleanup(ctx, tgb)
+		}
 		return err
 	}
 	tgARN := tgb.Spec.TargetGroupARN