Skip to content

Commit

Permalink
feat: resolve health check port name for NLB
Browse files Browse the repository at this point in the history
  • Loading branch information
fad3t committed Oct 3, 2023
1 parent eacb0c3 commit 873ac0b
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 23 deletions.
10 changes: 5 additions & 5 deletions pkg/service/model_build_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ func (t *defaultModelBuildTask) buildListeners(ctx context.Context, scheme elbv2
}

for _, port := range t.service.Spec.Ports {
_, err := t.buildListener(ctx, port, *cfg, scheme)
_, err := t.buildListener(ctx, t.service, port, *cfg, scheme)
if err != nil {
return err
}
}
return nil
}

func (t *defaultModelBuildTask) buildListener(ctx context.Context, port corev1.ServicePort, cfg listenerConfig,
func (t *defaultModelBuildTask) buildListener(ctx context.Context, svc *corev1.Service, port corev1.ServicePort, cfg listenerConfig,
scheme elbv2model.LoadBalancerScheme) (*elbv2model.Listener, error) {
lsSpec, err := t.buildListenerSpec(ctx, port, cfg, scheme)
lsSpec, err := t.buildListenerSpec(ctx, svc, port, cfg, scheme)
if err != nil {
return nil, err
}
Expand All @@ -39,7 +39,7 @@ func (t *defaultModelBuildTask) buildListener(ctx context.Context, port corev1.S
return ls, nil
}

func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, port corev1.ServicePort, cfg listenerConfig,
func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, svc *corev1.Service, port corev1.ServicePort, cfg listenerConfig,
scheme elbv2model.LoadBalancerScheme) (elbv2model.ListenerSpec, error) {
tgProtocol := elbv2model.Protocol(port.Protocol)
listenerProtocol := elbv2model.Protocol(port.Protocol)
Expand All @@ -55,7 +55,7 @@ func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, port core
if err != nil {
return elbv2model.ListenerSpec{}, err
}
targetGroup, err := t.buildTargetGroup(ctx, port, tgProtocol, scheme)
targetGroup, err := t.buildTargetGroup(ctx, svc, port, tgProtocol, scheme)
if err != nil {
return elbv2model.ListenerSpec{}, err
}
Expand Down
37 changes: 24 additions & 13 deletions pkg/service/model_build_target_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
healthCheckPortTrafficPort = "traffic-port"
)

func (t *defaultModelBuildTask) buildTargetGroup(ctx context.Context, port corev1.ServicePort, tgProtocol elbv2model.Protocol, scheme elbv2model.LoadBalancerScheme) (*elbv2model.TargetGroup, error) {
func (t *defaultModelBuildTask) buildTargetGroup(ctx context.Context, svc *corev1.Service, port corev1.ServicePort, tgProtocol elbv2model.Protocol, scheme elbv2model.LoadBalancerScheme) (*elbv2model.TargetGroup, error) {
svcPort := intstr.FromInt(int(port.Port))
tgResourceID := t.buildTargetGroupResourceID(k8s.NamespacedName(t.service), svcPort)
if targetGroup, exists := t.tgByResID[tgResourceID]; exists {
Expand All @@ -40,7 +40,7 @@ func (t *defaultModelBuildTask) buildTargetGroup(ctx context.Context, port corev
if err != nil {
return nil, err
}
healthCheckConfig, err := t.buildTargetGroupHealthCheckConfig(ctx, targetType)
healthCheckConfig, err := t.buildTargetGroupHealthCheckConfig(ctx, svc, targetType)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -89,22 +89,22 @@ func (t *defaultModelBuildTask) buildTargetGroupSpec(ctx context.Context, tgProt
}, nil
}

func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfig(ctx context.Context, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfig(ctx context.Context, svc *corev1.Service, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
if targetType == elbv2model.TargetTypeInstance && t.service.Spec.ExternalTrafficPolicy == corev1.ServiceExternalTrafficPolicyTypeLocal &&
t.service.Spec.Type == corev1.ServiceTypeLoadBalancer {
return t.buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx)
return t.buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx, svc, targetType)
}
return t.buildTargetGroupHealthCheckConfigDefault(ctx)
return t.buildTargetGroupHealthCheckConfigDefault(ctx, svc, targetType)
}

func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx context.Context) (*elbv2model.TargetGroupHealthCheckConfig, error) {
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx context.Context, svc *corev1.Service, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
healthCheckProtocol, err := t.buildTargetGroupHealthCheckProtocol(ctx, t.defaultHealthCheckProtocol)
if err != nil {
return nil, err
}
healthCheckPathPtr := t.buildTargetGroupHealthCheckPath(ctx, t.defaultHealthCheckPath, healthCheckProtocol)
healthCheckMatcherPtr := t.buildTargetGroupHealthCheckMatcher(ctx, healthCheckProtocol)
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, t.defaultHealthCheckPort)
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, svc, t.defaultHealthCheckPort, targetType)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -137,14 +137,14 @@ func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx con
}, nil
}

func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx context.Context) (*elbv2model.TargetGroupHealthCheckConfig, error) {
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx context.Context, svc *corev1.Service, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
healthCheckProtocol, err := t.buildTargetGroupHealthCheckProtocol(ctx, t.defaultHealthCheckProtocolForInstanceModeLocal)
if err != nil {
return nil, err
}
healthCheckPathPtr := t.buildTargetGroupHealthCheckPath(ctx, t.defaultHealthCheckPathForInstanceModeLocal, healthCheckProtocol)
healthCheckMatcherPtr := t.buildTargetGroupHealthCheckMatcher(ctx, healthCheckProtocol)
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, t.defaultHealthCheckPortForInstanceModeLocal)
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, svc, t.defaultHealthCheckPortForInstanceModeLocal, targetType)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -276,17 +276,28 @@ func (t *defaultModelBuildTask) buildTargetGroupPort(_ context.Context, targetTy
return 1
}

func (t *defaultModelBuildTask) buildTargetGroupHealthCheckPort(_ context.Context, defaultHealthCheckPort string) (intstr.IntOrString, error) {
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckPort(_ context.Context, svc *corev1.Service, defaultHealthCheckPort string, targetType elbv2model.TargetType) (intstr.IntOrString, error) {
rawHealthCheckPort := defaultHealthCheckPort
t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixHCPort, &rawHealthCheckPort, t.service.Annotations)
if rawHealthCheckPort == healthCheckPortTrafficPort {
return intstr.FromString(rawHealthCheckPort), nil
}
portVal, err := strconv.ParseInt(rawHealthCheckPort, 10, 64)
healthCheckPort := intstr.Parse(rawHealthCheckPort)
if healthCheckPort.Type == intstr.Int {
return healthCheckPort, nil
}

svcPort, err := k8s.LookupServicePort(svc, healthCheckPort)
if err != nil {
return intstr.IntOrString{}, errors.Errorf("health check port \"%v\" not supported", rawHealthCheckPort)
return intstr.IntOrString{}, errors.Wrap(err, "failed to resolve healthCheckPort")
}
if targetType == elbv2model.TargetTypeInstance {
return intstr.FromInt(int(svcPort.NodePort)), nil
}
if svcPort.TargetPort.Type == intstr.Int {
return svcPort.TargetPort, nil
}
return intstr.FromInt(int(portVal)), nil
return intstr.IntOrString{}, errors.New("cannot use named healthCheckPort for IP TargetType when service's targetPort is a named port")
}

func (t *defaultModelBuildTask) buildTargetGroupHealthCheckProtocol(_ context.Context, defaultHealthCheckProtocol elbv2model.Protocol) (elbv2model.Protocol, error) {
Expand Down
107 changes: 103 additions & 4 deletions pkg/service/model_build_target_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func Test_defaultModelBuilderTask_buildTargetHealthCheck(t *testing.T) {
defaultHealthCheckHealthyThresholdForInstanceModeLocal: 2,
defaultHealthCheckUnhealthyThresholdForInstanceModeLocal: 2,
}
hc, err := builder.buildTargetGroupHealthCheckConfig(context.Background(), tt.targetType)
hc, err := builder.buildTargetGroupHealthCheckConfig(context.Background(), tt.svc, tt.targetType)
if tt.wantError {
assert.Error(t, err)
} else {
Expand Down Expand Up @@ -1860,6 +1860,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
testName string
svc *corev1.Service
defaultPort string
targetType elbv2.TargetType
want intstr.IntOrString
wantErr error
}{
Expand All @@ -1868,6 +1869,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
svc: &corev1.Service{},
defaultPort: "traffic-port",
want: intstr.FromString("traffic-port"),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "with annotation",
Expand All @@ -1880,6 +1882,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
},
defaultPort: "traffic-port",
want: intstr.FromInt(34576),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "unsupported annotation value",
Expand All @@ -1891,19 +1894,115 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
},
},
defaultPort: "traffic-port",
wantErr: errors.New("health check port \"a34576\" not supported"),
wantErr: errors.New("failed to resolve healthCheckPort: unable to find port a34576 on service /"),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "default health check nodeport",
svc: &corev1.Service{},
defaultPort: "31227",
want: intstr.FromInt(31227),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "invalid default",
svc: &corev1.Service{},
defaultPort: "abs",
wantErr: errors.New("health check port \"abs\" not supported"),
wantErr: errors.New("failed to resolve healthCheckPort: unable to find port abs on service /"),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "resolve port name instance",
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-healthcheck-port": "health",
},
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Name: "traffic",
Port: 80,
TargetPort: intstr.FromInt(80),
NodePort: 31227,
Protocol: corev1.ProtocolTCP,
},
{
Name: "health",
Port: 1234,
TargetPort: intstr.FromInt(1234),
NodePort: 30987,
Protocol: corev1.ProtocolTCP,
},
},
},
},
defaultPort: "8080",
want: intstr.FromInt(30987),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "invalid port name",
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-healthcheck-port": "absent",
},
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Name: "traffic",
Port: 80,
TargetPort: intstr.FromInt(80),
NodePort: 31227,
Protocol: corev1.ProtocolTCP,
},
{
Name: "health",
Port: 1234,
TargetPort: intstr.FromInt(1234),
NodePort: 30987,
Protocol: corev1.ProtocolTCP,
},
},
},
},
defaultPort: "8080",
wantErr: errors.New("failed to resolve healthCheckPort: unable to find port absent on service /"),
targetType: elbv2.TargetTypeInstance,
},
{
testName: "resolve port name IP",
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"service.beta.kubernetes.io/aws-load-balancer-healthcheck-port": "health",
},
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Name: "traffic",
Port: 80,
TargetPort: intstr.FromInt(80),
NodePort: 31227,
Protocol: corev1.ProtocolTCP,
},
{
Name: "health",
Port: 1234,
TargetPort: intstr.FromInt(1234),
NodePort: 30987,
Protocol: corev1.ProtocolTCP,
},
},
},
},
defaultPort: "8080",
want: intstr.FromInt(1234),
targetType: elbv2.TargetTypeIP,
},
}
for _, tt := range tests {
Expand All @@ -1914,7 +2013,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
service: tt.svc,
defaultHealthCheckPort: tt.defaultPort,
}
got, err := builder.buildTargetGroupHealthCheckPort(context.Background(), tt.defaultPort)
got, err := builder.buildTargetGroupHealthCheckPort(context.Background(), tt.svc, tt.defaultPort, tt.targetType)
if tt.wantErr != nil {
assert.EqualError(t, err, tt.wantErr.Error())
} else {
Expand Down
2 changes: 1 addition & 1 deletion pkg/targetgroupbinding/networking_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package targetgroupbinding

import (
"context"
libErrors "errors"
"fmt"
"net"
"strings"
"sync"
libErrors "errors"

awssdk "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
Expand Down

0 comments on commit 873ac0b

Please sign in to comment.