From 74f6aa654b4b2e7246ada25b30724531b906b030 Mon Sep 17 00:00:00 2001 From: Yang Yang Date: Thu, 26 Mar 2020 20:28:40 -0700 Subject: [PATCH] fix aws loadbalancer nodePort cannot change issue --- .../aws/aws_loadbalancer.go | 7 +- .../aws/aws_loadbalancer_test.go | 124 ++++++++++++++++++ 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go index 9021b64e138..dd46751876d 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go @@ -452,13 +452,14 @@ var invalidELBV2NameRegex = regexp.MustCompile("[^[:alnum:]]") // buildTargetGroupName will build unique name for targetGroup of service & port. // the name is in format k8s-{namespace:8}-{name:8}-{uuid:10} (chosen to benefit most common use cases). -// Note: targetProtocol & targetType are included since they cannot be modified on existing targetGroup. -func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePort int64, targetProtocol string, targetType string) string { +// Note: nodePort & targetProtocol & targetType are included since they cannot be modified on existing targetGroup. +func (c *Cloud) buildTargetGroupName(serviceName types.NamespacedName, servicePort int64, nodePort int64, targetProtocol string, targetType string) string { hasher := sha1.New() _, _ = hasher.Write([]byte(c.tagging.clusterID())) _, _ = hasher.Write([]byte(serviceName.Namespace)) _, _ = hasher.Write([]byte(serviceName.Name)) _, _ = hasher.Write([]byte(strconv.FormatInt(servicePort, 10))) + _, _ = hasher.Write([]byte(strconv.FormatInt(nodePort, 10))) _, _ = hasher.Write([]byte(targetProtocol)) _, _ = hasher.Write([]byte(targetType)) tgUUID := hex.EncodeToString(hasher.Sum(nil)) @@ -527,7 +528,7 @@ func (c *Cloud) ensureTargetGroup(targetGroup *elbv2.TargetGroup, serviceName ty dirty := false if targetGroup == nil { targetType := "instance" - name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficProtocol, targetType) + name := c.buildTargetGroupName(serviceName, mapping.FrontendPort, mapping.TrafficPort, mapping.TrafficProtocol, targetType) klog.Infof("Creating load balancer target group for %v with name: %s", serviceName, name) input := &elbv2.CreateTargetGroupInput{ VpcId: aws.String(vpcID), diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go index a437e3d3ad0..16574b058c0 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go @@ -19,6 +19,7 @@ limitations under the License. package aws import ( + "k8s.io/apimachinery/pkg/types" "testing" "github.com/aws/aws-sdk-go/aws" @@ -296,3 +297,126 @@ func TestElbListenersAreEqual(t *testing.T) { }) } } + +func TestBuildTargetGroupName(t *testing.T) { + type args struct { + serviceName types.NamespacedName + servicePort int64 + nodePort int64 + targetProtocol string + targetType string + } + tests := []struct { + name string + clusterID string + args args + want string + }{ + { + name: "base case", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: "TCP", + targetType: "instance", + }, + want: "k8s-default-servicea-0aeb5b75af", + }, + { + name: "base case & clusterID changed", + clusterID: "cluster-b", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: "TCP", + targetType: "instance", + }, + want: "k8s-default-servicea-5d3a0a69a8", + }, + { + name: "base case & serviceNamespace changed", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "another", Name: "service-a"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: "TCP", + targetType: "instance", + }, + want: "k8s-another-servicea-f3a3263315", + }, + { + name: "base case & serviceName changed", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-b"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: "TCP", + targetType: "instance", + }, + want: "k8s-default-serviceb-9a3c03b25e", + }, + { + name: "base case & servicePort changed", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 9090, + nodePort: 8080, + targetProtocol: "TCP", + targetType: "instance", + }, + want: "k8s-default-servicea-6e07474ff4", + }, + { + name: "base case & nodePort changed", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 80, + nodePort: 9090, + targetProtocol: "TCP", + targetType: "instance", + }, + want: "k8s-default-servicea-6cb2d0201c", + }, + { + name: "base case & targetProtocol changed", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: "UDP", + targetType: "instance", + }, + want: "k8s-default-servicea-70495e628e", + }, + { + name: "base case & targetType changed", + clusterID: "cluster-a", + args: args{ + serviceName: types.NamespacedName{Namespace: "default", Name: "service-a"}, + servicePort: 80, + nodePort: 8080, + targetProtocol: "TCP", + targetType: "ip", + }, + want: "k8s-default-servicea-fff6dd8028", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Cloud{ + tagging: awsTagging{ClusterID: tt.clusterID}, + } + if got := c.buildTargetGroupName(tt.args.serviceName, tt.args.servicePort, tt.args.nodePort, tt.args.targetProtocol, tt.args.targetType); got != tt.want { + assert.Equal(t, tt.want, got) + } + }) + } +}