Skip to content

Commit 227315a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #4021 from zac-nixon/znixon/mc-same-cluster
[feat] allow multiple targetgroupbindings to reference same tg arn if using multicluster mode
2 parents 7118c22 + 92ae3dc commit 227315a

File tree

3 files changed

+144
-7
lines changed

3 files changed

+144
-7
lines changed

‎docs/guide/targetgroupbinding/targetgroupbinding.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ spec:
257257
name: awesome-service # route traffic to the awesome-service
258258
port: 80
259259
targetGroupARN: <arn-to-targetGroup>
260-
multiClusterTargetGroup: "true"
260+
multiClusterTargetGroup: true
261261
```
262262

263263

‎webhooks/elbv2/targetgroupbinding_validator.go

+19-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package elbv2
33
import (
44
"context"
55
"fmt"
6+
"k8s.io/apimachinery/pkg/types"
67
"regexp"
78
"strings"
89

@@ -90,6 +91,9 @@ func (v *targetGroupBindingValidator) ValidateUpdate(ctx context.Context, obj ru
9091
if err := v.checkAssumeRoleConfig(tgb); err != nil {
9192
return err
9293
}
94+
if err := v.checkExistingTargetGroups(tgb); err != nil {
95+
return err
96+
}
9397
return nil
9498
}
9599

@@ -162,17 +166,29 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG
162166
}
163167

164168
// checkExistingTargetGroups will check for unique TargetGroup per TargetGroupBinding
165-
func (v *targetGroupBindingValidator) checkExistingTargetGroups(tgb *elbv2api.TargetGroupBinding) error {
169+
func (v *targetGroupBindingValidator) checkExistingTargetGroups(updatedTgb *elbv2api.TargetGroupBinding) error {
166170
ctx := context.Background()
167171
tgbList := elbv2api.TargetGroupBindingList{}
168172
if err := v.k8sClient.List(ctx, &tgbList); err != nil {
169173
return errors.Wrap(err, "failed to list TargetGroupBindings in the cluster")
170174
}
175+
176+
duplicateTGBs := make([]types.NamespacedName, 0)
177+
multiClusterSupported := updatedTgb.Spec.MultiClusterTargetGroup
178+
171179
for _, tgbObj := range tgbList.Items {
172-
if tgbObj.Spec.TargetGroupARN == tgb.Spec.TargetGroupARN {
173-
return errors.Errorf("TargetGroup %v is already bound to TargetGroupBinding %v", tgb.Spec.TargetGroupARN, k8s.NamespacedName(&tgbObj).String())
180+
if tgbObj.UID != updatedTgb.UID && tgbObj.Spec.TargetGroupARN == updatedTgb.Spec.TargetGroupARN {
181+
if !tgbObj.Spec.MultiClusterTargetGroup {
182+
multiClusterSupported = false
183+
}
184+
duplicateTGBs = append(duplicateTGBs, k8s.NamespacedName(&tgbObj))
174185
}
175186
}
187+
188+
if len(duplicateTGBs) != 0 && !multiClusterSupported {
189+
return errors.Errorf("TargetGroup %v is already bound to following TargetGroupBindings %v. Please enable MultiCluster mode on all TargetGroupBindings referencing %v or choose a different Target Group ARN.", updatedTgb.Spec.TargetGroupARN, duplicateTGBs, updatedTgb.Spec.TargetGroupARN)
190+
}
191+
176192
return nil
177193
}
178194

‎webhooks/elbv2/targetgroupbinding_validator_test.go

+124-3
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,14 @@ func Test_targetGroupBindingValidator_ValidateUpdate(t *testing.T) {
462462
}
463463
for _, tt := range tests {
464464
t.Run(tt.name, func(t *testing.T) {
465+
k8sSchema := runtime.NewScheme()
466+
clientgoscheme.AddToScheme(k8sSchema)
467+
elbv2api.AddToScheme(k8sSchema)
468+
k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build()
469+
465470
v := &targetGroupBindingValidator{
466-
logger: logr.New(&log.NullLogSink{}),
471+
logger: logr.New(&log.NullLogSink{}),
472+
k8sClient: k8sClient,
467473
}
468474
err := v.ValidateUpdate(context.Background(), tt.args.obj, tt.args.oldObj)
469475
if tt.wantErr != nil {
@@ -954,6 +960,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
954960
ObjectMeta: metav1.ObjectMeta{
955961
Name: "tgb1",
956962
Namespace: "ns1",
963+
UID: "tgb1",
957964
},
958965
Spec: elbv2api.TargetGroupBindingSpec{
959966
TargetGroupARN: "tg-1",
@@ -971,6 +978,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
971978
ObjectMeta: metav1.ObjectMeta{
972979
Name: "tgb1",
973980
Namespace: "ns1",
981+
UID: "tgb1",
974982
},
975983
Spec: elbv2api.TargetGroupBindingSpec{
976984
TargetGroupARN: "tg-1",
@@ -984,6 +992,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
984992
ObjectMeta: metav1.ObjectMeta{
985993
Name: "tgb2",
986994
Namespace: "ns2",
995+
UID: "tgb2",
987996
},
988997
Spec: elbv2api.TargetGroupBindingSpec{
989998
TargetGroupARN: "tg-2",
@@ -1001,6 +1010,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10011010
ObjectMeta: metav1.ObjectMeta{
10021011
Name: "tgb1",
10031012
Namespace: "ns1",
1013+
UID: "tgb1",
10041014
},
10051015
Spec: elbv2api.TargetGroupBindingSpec{
10061016
TargetGroupARN: "tg-1",
@@ -1011,6 +1021,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10111021
ObjectMeta: metav1.ObjectMeta{
10121022
Name: "tgb2",
10131023
Namespace: "ns2",
1024+
UID: "tgb2",
10141025
},
10151026
Spec: elbv2api.TargetGroupBindingSpec{
10161027
TargetGroupARN: "tg-2",
@@ -1021,19 +1032,32 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10211032
ObjectMeta: metav1.ObjectMeta{
10221033
Name: "tgb3",
10231034
Namespace: "ns3",
1035+
UID: "tgb3",
10241036
},
10251037
Spec: elbv2api.TargetGroupBindingSpec{
10261038
TargetGroupARN: "tg-3",
10271039
TargetType: nil,
10281040
},
10291041
},
1042+
{
1043+
ObjectMeta: metav1.ObjectMeta{
1044+
Name: "tgb22",
1045+
Namespace: "ns1",
1046+
UID: "tgb22",
1047+
},
1048+
Spec: elbv2api.TargetGroupBindingSpec{
1049+
TargetGroupARN: "tg-22",
1050+
TargetType: nil,
1051+
},
1052+
},
10301053
},
10311054
},
10321055
args: args{
10331056
tgb: &elbv2api.TargetGroupBinding{
10341057
ObjectMeta: metav1.ObjectMeta{
10351058
Name: "tgb22",
10361059
Namespace: "ns1",
1060+
UID: "tgb22",
10371061
},
10381062
Spec: elbv2api.TargetGroupBindingSpec{
10391063
TargetGroupARN: "tg-22",
@@ -1051,6 +1075,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10511075
ObjectMeta: metav1.ObjectMeta{
10521076
Name: "tgb1",
10531077
Namespace: "ns1",
1078+
UID: "tgb1",
10541079
},
10551080
Spec: elbv2api.TargetGroupBindingSpec{
10561081
TargetGroupARN: "tg-1",
@@ -1064,14 +1089,106 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10641089
ObjectMeta: metav1.ObjectMeta{
10651090
Name: "tgb2",
10661091
Namespace: "ns1",
1092+
UID: "tgb2",
1093+
},
1094+
Spec: elbv2api.TargetGroupBindingSpec{
1095+
TargetGroupARN: "tg-1",
1096+
TargetType: nil,
1097+
},
1098+
},
1099+
},
1100+
wantErr: errors.New("TargetGroup tg-1 is already bound to following TargetGroupBindings [ns1/tgb1]. Please enable MultiCluster mode on all TargetGroupBindings referencing tg-1 or choose a different Target Group ARN."),
1101+
},
1102+
{
1103+
name: "[ok] duplicate target groups with multi cluster support",
1104+
env: env{
1105+
existingTGBs: []elbv2api.TargetGroupBinding{
1106+
{
1107+
ObjectMeta: metav1.ObjectMeta{
1108+
Name: "tgb1",
1109+
Namespace: "ns1",
1110+
UID: "tgb1",
1111+
},
1112+
Spec: elbv2api.TargetGroupBindingSpec{
1113+
TargetGroupARN: "tg-1",
1114+
TargetType: nil,
1115+
MultiClusterTargetGroup: true,
1116+
},
1117+
},
1118+
},
1119+
},
1120+
args: args{
1121+
tgb: &elbv2api.TargetGroupBinding{
1122+
ObjectMeta: metav1.ObjectMeta{
1123+
Name: "tgb2",
1124+
Namespace: "ns1",
1125+
UID: "tgb2",
1126+
},
1127+
Spec: elbv2api.TargetGroupBindingSpec{
1128+
TargetGroupARN: "tg-1",
1129+
TargetType: nil,
1130+
MultiClusterTargetGroup: true,
1131+
},
1132+
},
1133+
},
1134+
wantErr: nil,
1135+
},
1136+
{
1137+
name: "[err] try to add binding without multicluster support while multiple bindings are using the same tg arn",
1138+
env: env{
1139+
existingTGBs: []elbv2api.TargetGroupBinding{
1140+
{
1141+
ObjectMeta: metav1.ObjectMeta{
1142+
Name: "tgb1",
1143+
Namespace: "ns1",
1144+
UID: "tgb1",
1145+
},
1146+
Spec: elbv2api.TargetGroupBindingSpec{
1147+
TargetGroupARN: "tg-1",
1148+
TargetType: nil,
1149+
MultiClusterTargetGroup: true,
1150+
},
1151+
},
1152+
{
1153+
ObjectMeta: metav1.ObjectMeta{
1154+
Name: "tgb3",
1155+
Namespace: "ns1",
1156+
UID: "tgb3",
1157+
},
1158+
Spec: elbv2api.TargetGroupBindingSpec{
1159+
TargetGroupARN: "tg-1",
1160+
TargetType: nil,
1161+
MultiClusterTargetGroup: true,
1162+
},
1163+
},
1164+
{
1165+
ObjectMeta: metav1.ObjectMeta{
1166+
Name: "tgb4",
1167+
Namespace: "ns1",
1168+
UID: "tgb4",
1169+
},
1170+
Spec: elbv2api.TargetGroupBindingSpec{
1171+
TargetGroupARN: "tg-1",
1172+
TargetType: nil,
1173+
MultiClusterTargetGroup: true,
1174+
},
1175+
},
1176+
},
1177+
},
1178+
args: args{
1179+
tgb: &elbv2api.TargetGroupBinding{
1180+
ObjectMeta: metav1.ObjectMeta{
1181+
Name: "tgb2",
1182+
Namespace: "ns1",
1183+
UID: "tgb2",
10671184
},
10681185
Spec: elbv2api.TargetGroupBindingSpec{
10691186
TargetGroupARN: "tg-1",
10701187
TargetType: nil,
10711188
},
10721189
},
10731190
},
1074-
wantErr: errors.New("TargetGroup tg-1 is already bound to TargetGroupBinding ns1/tgb1"),
1191+
wantErr: errors.New("TargetGroup tg-1 is already bound to following TargetGroupBindings [ns1/tgb1 ns1/tgb3 ns1/tgb4]. Please enable MultiCluster mode on all TargetGroupBindings referencing tg-1 or choose a different Target Group ARN."),
10751192
},
10761193
{
10771194
name: "[err] duplicate target groups - one target group binding",
@@ -1081,6 +1198,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10811198
ObjectMeta: metav1.ObjectMeta{
10821199
Name: "tgb1",
10831200
Namespace: "ns1",
1201+
UID: "tgb1",
10841202
},
10851203
Spec: elbv2api.TargetGroupBindingSpec{
10861204
TargetGroupARN: "tg-1",
@@ -1091,6 +1209,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
10911209
ObjectMeta: metav1.ObjectMeta{
10921210
Name: "tgb2",
10931211
Namespace: "ns2",
1212+
UID: "tgb2",
10941213
},
10951214
Spec: elbv2api.TargetGroupBindingSpec{
10961215
TargetGroupARN: "tg-111",
@@ -1101,6 +1220,7 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
11011220
ObjectMeta: metav1.ObjectMeta{
11021221
Name: "tgb3",
11031222
Namespace: "ns3",
1223+
UID: "tgb3",
11041224
},
11051225
Spec: elbv2api.TargetGroupBindingSpec{
11061226
TargetGroupARN: "tg-3",
@@ -1114,14 +1234,15 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) {
11141234
ObjectMeta: metav1.ObjectMeta{
11151235
Name: "tgb111",
11161236
Namespace: "ns1",
1237+
UID: "tgb111",
11171238
},
11181239
Spec: elbv2api.TargetGroupBindingSpec{
11191240
TargetGroupARN: "tg-111",
11201241
TargetType: nil,
11211242
},
11221243
},
11231244
},
1124-
wantErr: errors.New("TargetGroup tg-111 is already bound to TargetGroupBinding ns2/tgb2"),
1245+
wantErr: errors.New("TargetGroup tg-111 is already bound to following TargetGroupBindings [ns2/tgb2]. Please enable MultiCluster mode on all TargetGroupBindings referencing tg-111 or choose a different Target Group ARN."),
11251246
},
11261247
}
11271248
for _, tt := range tests {

0 commit comments

Comments
 (0)