Skip to content

Commit

Permalink
Merge pull request #3419 from fad3t/feat/nlb-resolve-port-name
Browse files Browse the repository at this point in the history 
feat: resolve health check port name for NLB
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 16, 2023
2 parents 36c6c4f + d06fdcf commit 631041d
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 14 deletions.
31 changes: 21 additions & 10 deletions pkg/service/model_build_target_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,19 @@ func (t *defaultModelBuildTask) buildTargetGroupSpec(ctx context.Context, tgProt
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfig(ctx context.Context, 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, targetType)
}
return t.buildTargetGroupHealthCheckConfigDefault(ctx)
return t.buildTargetGroupHealthCheckConfigDefault(ctx, targetType)
}

func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx context.Context) (*elbv2model.TargetGroupHealthCheckConfig, error) {
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx context.Context, 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, 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, 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, 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, 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(t.service, 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
105 changes: 102 additions & 3 deletions pkg/service/model_build_target_group_test.go
View file
Original file line number Diff line number Diff line change
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.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
View file
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 631041d

Please sign in to comment.