fix aws loadbalancer nodePort cannot change issue

Author: M00nF1sh

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),

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)
+			}
+		})
+	}
+}