Skip to content

Commit 631041d

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3419 from fad3t/feat/nlb-resolve-port-name
feat: resolve health check port name for NLB
2 parents 36c6c4f + d06fdcf commit 631041d

File tree

3 files changed

+124
-14
lines changed

3 files changed

+124
-14
lines changed

‎pkg/service/model_build_target_group.go‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,19 @@ func (t *defaultModelBuildTask) buildTargetGroupSpec(ctx context.Context, tgProt
9292
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfig(ctx context.Context, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
9393
if targetType == elbv2model.TargetTypeInstance && t.service.Spec.ExternalTrafficPolicy == corev1.ServiceExternalTrafficPolicyTypeLocal &&
9494
t.service.Spec.Type == corev1.ServiceTypeLoadBalancer {
95-
return t.buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx)
95+
return t.buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx, targetType)
9696
}
97-
return t.buildTargetGroupHealthCheckConfigDefault(ctx)
97+
return t.buildTargetGroupHealthCheckConfigDefault(ctx, targetType)
9898
}
9999

100-
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx context.Context) (*elbv2model.TargetGroupHealthCheckConfig, error) {
100+
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx context.Context, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
101101
healthCheckProtocol, err := t.buildTargetGroupHealthCheckProtocol(ctx, t.defaultHealthCheckProtocol)
102102
if err != nil {
103103
return nil, err
104104
}
105105
healthCheckPathPtr := t.buildTargetGroupHealthCheckPath(ctx, t.defaultHealthCheckPath, healthCheckProtocol)
106106
healthCheckMatcherPtr := t.buildTargetGroupHealthCheckMatcher(ctx, healthCheckProtocol)
107-
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, t.defaultHealthCheckPort)
107+
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, t.defaultHealthCheckPort, targetType)
108108
if err != nil {
109109
return nil, err
110110
}
@@ -137,14 +137,14 @@ func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigDefault(ctx con
137137
}, nil
138138
}
139139

140-
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx context.Context) (*elbv2model.TargetGroupHealthCheckConfig, error) {
140+
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckConfigForInstanceModeLocal(ctx context.Context, targetType elbv2model.TargetType) (*elbv2model.TargetGroupHealthCheckConfig, error) {
141141
healthCheckProtocol, err := t.buildTargetGroupHealthCheckProtocol(ctx, t.defaultHealthCheckProtocolForInstanceModeLocal)
142142
if err != nil {
143143
return nil, err
144144
}
145145
healthCheckPathPtr := t.buildTargetGroupHealthCheckPath(ctx, t.defaultHealthCheckPathForInstanceModeLocal, healthCheckProtocol)
146146
healthCheckMatcherPtr := t.buildTargetGroupHealthCheckMatcher(ctx, healthCheckProtocol)
147-
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, t.defaultHealthCheckPortForInstanceModeLocal)
147+
healthCheckPort, err := t.buildTargetGroupHealthCheckPort(ctx, t.defaultHealthCheckPortForInstanceModeLocal, targetType)
148148
if err != nil {
149149
return nil, err
150150
}
@@ -276,17 +276,28 @@ func (t *defaultModelBuildTask) buildTargetGroupPort(_ context.Context, targetTy
276276
return 1
277277
}
278278

279-
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckPort(_ context.Context, defaultHealthCheckPort string) (intstr.IntOrString, error) {
279+
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckPort(_ context.Context, defaultHealthCheckPort string, targetType elbv2model.TargetType) (intstr.IntOrString, error) {
280280
rawHealthCheckPort := defaultHealthCheckPort
281281
t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixHCPort, &rawHealthCheckPort, t.service.Annotations)
282282
if rawHealthCheckPort == healthCheckPortTrafficPort {
283283
return intstr.FromString(rawHealthCheckPort), nil
284284
}
285-
portVal, err := strconv.ParseInt(rawHealthCheckPort, 10, 64)
285+
healthCheckPort := intstr.Parse(rawHealthCheckPort)
286+
if healthCheckPort.Type == intstr.Int {
287+
return healthCheckPort, nil
288+
}
289+
290+
svcPort, err := k8s.LookupServicePort(t.service, healthCheckPort)
286291
if err != nil {
287-
return intstr.IntOrString{}, errors.Errorf("health check port \"%v\" not supported", rawHealthCheckPort)
292+
return intstr.IntOrString{}, errors.Wrap(err, "failed to resolve healthCheckPort")
293+
}
294+
if targetType == elbv2model.TargetTypeInstance {
295+
return intstr.FromInt(int(svcPort.NodePort)), nil
296+
}
297+
if svcPort.TargetPort.Type == intstr.Int {
298+
return svcPort.TargetPort, nil
288299
}
289-
return intstr.FromInt(int(portVal)), nil
300+
return intstr.IntOrString{}, errors.New("cannot use named healthCheckPort for IP TargetType when service's targetPort is a named port")
290301
}
291302

292303
func (t *defaultModelBuildTask) buildTargetGroupHealthCheckProtocol(_ context.Context, defaultHealthCheckProtocol elbv2model.Protocol) (elbv2model.Protocol, error) {

‎pkg/service/model_build_target_group_test.go‎

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,6 +1860,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
18601860
testName string
18611861
svc *corev1.Service
18621862
defaultPort string
1863+
targetType elbv2.TargetType
18631864
want intstr.IntOrString
18641865
wantErr error
18651866
}{
@@ -1868,6 +1869,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
18681869
svc: &corev1.Service{},
18691870
defaultPort: "traffic-port",
18701871
want: intstr.FromString("traffic-port"),
1872+
targetType: elbv2.TargetTypeInstance,
18711873
},
18721874
{
18731875
testName: "with annotation",
@@ -1880,6 +1882,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
18801882
},
18811883
defaultPort: "traffic-port",
18821884
want: intstr.FromInt(34576),
1885+
targetType: elbv2.TargetTypeInstance,
18831886
},
18841887
{
18851888
testName: "unsupported annotation value",
@@ -1891,19 +1894,115 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
18911894
},
18921895
},
18931896
defaultPort: "traffic-port",
1894-
wantErr: errors.New("health check port \"a34576\" not supported"),
1897+
wantErr: errors.New("failed to resolve healthCheckPort: unable to find port a34576 on service /"),
1898+
targetType: elbv2.TargetTypeInstance,
18951899
},
18961900
{
18971901
testName: "default health check nodeport",
18981902
svc: &corev1.Service{},
18991903
defaultPort: "31227",
19001904
want: intstr.FromInt(31227),
1905+
targetType: elbv2.TargetTypeInstance,
19011906
},
19021907
{
19031908
testName: "invalid default",
19041909
svc: &corev1.Service{},
19051910
defaultPort: "abs",
1906-
wantErr: errors.New("health check port \"abs\" not supported"),
1911+
wantErr: errors.New("failed to resolve healthCheckPort: unable to find port abs on service /"),
1912+
targetType: elbv2.TargetTypeInstance,
1913+
},
1914+
{
1915+
testName: "resolve port name instance",
1916+
svc: &corev1.Service{
1917+
ObjectMeta: metav1.ObjectMeta{
1918+
Annotations: map[string]string{
1919+
"service.beta.kubernetes.io/aws-load-balancer-healthcheck-port": "health",
1920+
},
1921+
},
1922+
Spec: corev1.ServiceSpec{
1923+
Ports: []corev1.ServicePort{
1924+
{
1925+
Name: "traffic",
1926+
Port: 80,
1927+
TargetPort: intstr.FromInt(80),
1928+
NodePort: 31227,
1929+
Protocol: corev1.ProtocolTCP,
1930+
},
1931+
{
1932+
Name: "health",
1933+
Port: 1234,
1934+
TargetPort: intstr.FromInt(1234),
1935+
NodePort: 30987,
1936+
Protocol: corev1.ProtocolTCP,
1937+
},
1938+
},
1939+
},
1940+
},
1941+
defaultPort: "8080",
1942+
want: intstr.FromInt(30987),
1943+
targetType: elbv2.TargetTypeInstance,
1944+
},
1945+
{
1946+
testName: "invalid port name",
1947+
svc: &corev1.Service{
1948+
ObjectMeta: metav1.ObjectMeta{
1949+
Annotations: map[string]string{
1950+
"service.beta.kubernetes.io/aws-load-balancer-healthcheck-port": "absent",
1951+
},
1952+
},
1953+
Spec: corev1.ServiceSpec{
1954+
Ports: []corev1.ServicePort{
1955+
{
1956+
Name: "traffic",
1957+
Port: 80,
1958+
TargetPort: intstr.FromInt(80),
1959+
NodePort: 31227,
1960+
Protocol: corev1.ProtocolTCP,
1961+
},
1962+
{
1963+
Name: "health",
1964+
Port: 1234,
1965+
TargetPort: intstr.FromInt(1234),
1966+
NodePort: 30987,
1967+
Protocol: corev1.ProtocolTCP,
1968+
},
1969+
},
1970+
},
1971+
},
1972+
defaultPort: "8080",
1973+
wantErr: errors.New("failed to resolve healthCheckPort: unable to find port absent on service /"),
1974+
targetType: elbv2.TargetTypeInstance,
1975+
},
1976+
{
1977+
testName: "resolve port name IP",
1978+
svc: &corev1.Service{
1979+
ObjectMeta: metav1.ObjectMeta{
1980+
Annotations: map[string]string{
1981+
"service.beta.kubernetes.io/aws-load-balancer-healthcheck-port": "health",
1982+
},
1983+
},
1984+
Spec: corev1.ServiceSpec{
1985+
Ports: []corev1.ServicePort{
1986+
{
1987+
Name: "traffic",
1988+
Port: 80,
1989+
TargetPort: intstr.FromInt(80),
1990+
NodePort: 31227,
1991+
Protocol: corev1.ProtocolTCP,
1992+
},
1993+
{
1994+
Name: "health",
1995+
Port: 1234,
1996+
TargetPort: intstr.FromInt(1234),
1997+
NodePort: 30987,
1998+
Protocol: corev1.ProtocolTCP,
1999+
},
2000+
},
2001+
},
2002+
},
2003+
defaultPort: "8080",
2004+
want: intstr.FromInt(1234),
2005+
targetType: elbv2.TargetTypeIP,
19072006
},
19082007
}
19092008
for _, tt := range tests {
@@ -1914,7 +2013,7 @@ func Test_defaultModelBuilder_buildTargetGroupHealthCheckPort(t *testing.T) {
19142013
service: tt.svc,
19152014
defaultHealthCheckPort: tt.defaultPort,
19162015
}
1917-
got, err := builder.buildTargetGroupHealthCheckPort(context.Background(), tt.defaultPort)
2016+
got, err := builder.buildTargetGroupHealthCheckPort(context.Background(), tt.defaultPort, tt.targetType)
19182017
if tt.wantErr != nil {
19192018
assert.EqualError(t, err, tt.wantErr.Error())
19202019
} else {

‎pkg/targetgroupbinding/networking_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ package targetgroupbinding
22

33
import (
44
"context"
5+
libErrors "errors"
56
"fmt"
67
"net"
78
"strings"
89
"sync"
9-
libErrors "errors"
1010

1111
awssdk "github.com/aws/aws-sdk-go/aws"
1212
"github.com/aws/aws-sdk-go/aws/awserr"

0 commit comments

Comments
 (0)