Skip to content

Commit b77f802

Browse files
oliviassssoliviassss
committed
update
1 parent 2238e57 commit b77f802

File tree

2 files changed

+97
-51
lines changed

2 files changed

+97
-51
lines changed

‎pkg/networking/subnet_resolver.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
156156
}
157157
var subnets []*ec2sdk.Subnet
158158
for _, subnet := range allSubnets {
159-
if r.checkSubnetIsNotTaggedForOtherClusters(subnet) {
159+
if r.checkSubnetIsNotTaggedForOtherClusters(subnet, resolveOpts.SubnetsClusterTagCheck) {
160160
subnets = append(subnets, subnet)
161161
}
162162
}
@@ -168,15 +168,13 @@ func (r *defaultSubnetsResolver) ResolveViaDiscovery(ctx context.Context, opts .
168168
chosenSubnets = append(chosenSubnets, subnets[0])
169169
} else if len(subnets) > 1 {
170170
sort.Slice(subnets, func(i, j int) bool {
171-
if resolveOpts.SubnetsClusterTagCheck {
172-
clusterTagI := r.checkSubnetHasClusterTag(subnets[i])
173-
clusterTagJ := r.checkSubnetHasClusterTag(subnets[j])
174-
if clusterTagI != clusterTagJ {
175-
if clusterTagI {
176-
return true
177-
}
178-
return false
171+
clusterTagI := r.checkSubnetHasClusterTag(subnets[i])
172+
clusterTagJ := r.checkSubnetHasClusterTag(subnets[j])
173+
if clusterTagI != clusterTagJ {
174+
if clusterTagI {
175+
return true
179176
}
177+
return false
180178
}
181179
return awssdk.StringValue(subnets[i].SubnetId) < awssdk.StringValue(subnets[j].SubnetId)
182180
})
@@ -353,7 +351,10 @@ func (r *defaultSubnetsResolver) checkSubnetHasClusterTag(subnet *ec2sdk.Subnet)
353351
// checkSubnetIsNotTaggedForOtherClusters checks whether the subnet is tagged for the current cluster
354352
// or it doesn't contain the cluster tag at all. If the subnet contains a tag for other clusters, then
355353
// this check returns false so that the subnet does not used for the load balancer.
356-
func (r *defaultSubnetsResolver) checkSubnetIsNotTaggedForOtherClusters(subnet *ec2sdk.Subnet) bool {
354+
func (r *defaultSubnetsResolver) checkSubnetIsNotTaggedForOtherClusters(subnet *ec2sdk.Subnet, subnetsClusterTagCheck bool) bool {
355+
if !subnetsClusterTagCheck {
356+
return true
357+
}
357358
clusterResourceTagPrefix := "kubernetes.io/cluster"
358359
clusterResourceTagKey := fmt.Sprintf("kubernetes.io/cluster/%s", r.clusterName)
359360
hasClusterResourceTagPrefix := false

‎pkg/networking/subnet_resolver_test.go‎

Lines changed: 86 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
616616
wantErr: errors.New("some error"),
617617
},
618618
{
619-
name: "subnet with cluster tag gets precedence, with SubnetsClusterTagCheck enabled",
619+
name: "subnet with cluster tag gets precedence",
620620
fields: fields{
621621
vpcID: "vpc-1",
622622
clusterName: "kube-cluster",
@@ -672,7 +672,6 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
672672
opts: []SubnetsResolveOption{
673673
WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
674674
WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing),
675-
WithSubnetsClusterTagCheck(defaultSubnetsClusterTagCheck),
676675
},
677676
},
678677
want: []*ec2sdk.Subnet{
@@ -691,7 +690,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
691690
},
692691
},
693692
{
694-
name: "subnet with cluster tag does not get precedence, with SubnetsClusterTagCheck disabled",
693+
name: "subnets tagged for some other clusters get ignored, with SubnetsClusterTagCheck enabled",
695694
fields: fields{
696695
vpcID: "vpc-1",
697696
clusterName: "kube-cluster",
@@ -712,14 +711,44 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
712711
output: []*ec2sdk.Subnet{
713712
{
714713
SubnetId: awssdk.String("subnet-1"),
714+
AvailabilityZone: awssdk.String("us-west-2a"),
715+
AvailabilityZoneId: awssdk.String("usw2-az1"),
716+
VpcId: awssdk.String("vpc-1"),
717+
Tags: []*ec2sdk.Tag{
718+
{
719+
Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"),
720+
Value: awssdk.String("owned"),
721+
},
722+
},
723+
},
724+
{
725+
SubnetId: awssdk.String("subnet-3"),
726+
AvailabilityZone: awssdk.String("us-west-2a"),
727+
AvailabilityZoneId: awssdk.String("usw2-az1"),
728+
VpcId: awssdk.String("vpc-1"),
729+
Tags: []*ec2sdk.Tag{
730+
{
731+
Key: awssdk.String("kubernetes.io/cluster/kube-cluster"),
732+
Value: awssdk.String("owned"),
733+
},
734+
},
735+
},
736+
{
737+
SubnetId: awssdk.String("subnet-4"),
715738
AvailabilityZone: awssdk.String("us-west-2b"),
716739
AvailabilityZoneId: awssdk.String("usw2-az2"),
717740
VpcId: awssdk.String("vpc-1"),
741+
Tags: []*ec2sdk.Tag{
742+
{
743+
Key: awssdk.String("kubernetes.io/cluster/no-cluster"),
744+
Value: awssdk.String("owned"),
745+
},
746+
},
718747
},
719748
{
720749
SubnetId: awssdk.String("subnet-2"),
721-
AvailabilityZone: awssdk.String("us-west-2b"),
722-
AvailabilityZoneId: awssdk.String("usw2-az2"),
750+
AvailabilityZone: awssdk.String("us-west-2a"),
751+
AvailabilityZoneId: awssdk.String("usw2-az1"),
723752
VpcId: awssdk.String("vpc-1"),
724753
Tags: []*ec2sdk.Tag{
725754
{
@@ -728,15 +757,30 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
728757
},
729758
},
730759
},
760+
{
761+
SubnetId: awssdk.String("subnet-5"),
762+
AvailabilityZone: awssdk.String("us-west-2c"),
763+
AvailabilityZoneId: awssdk.String("usw2-az3"),
764+
VpcId: awssdk.String("vpc-1"),
765+
},
731766
},
732767
},
733768
},
734769
fetchAZInfosCalls: []fetchAZInfosCall{
735770
{
736-
availabilityZoneIDs: []string{"usw2-az2"},
771+
availabilityZoneIDs: []string{"usw2-az1"},
737772
azInfoByAZID: map[string]ec2sdk.AvailabilityZone{
738-
"usw2-az2": {
739-
ZoneId: awssdk.String("usw2-az2"),
773+
"usw2-az1": {
774+
ZoneId: awssdk.String("usw2-az1"),
775+
ZoneType: awssdk.String("availability-zone"),
776+
},
777+
},
778+
},
779+
{
780+
availabilityZoneIDs: []string{"usw2-az3"},
781+
azInfoByAZID: map[string]ec2sdk.AvailabilityZone{
782+
"usw2-az3": {
783+
ZoneId: awssdk.String("usw2-az3"),
740784
ZoneType: awssdk.String("availability-zone"),
741785
},
742786
},
@@ -747,20 +791,32 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
747791
opts: []SubnetsResolveOption{
748792
WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
749793
WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing),
750-
WithSubnetsClusterTagCheck(false),
794+
WithSubnetsClusterTagCheck(defaultSubnetsClusterTagCheck),
751795
},
752796
},
753797
want: []*ec2sdk.Subnet{
754798
{
755-
SubnetId: awssdk.String("subnet-1"),
756-
AvailabilityZone: awssdk.String("us-west-2b"),
757-
AvailabilityZoneId: awssdk.String("usw2-az2"),
799+
SubnetId: awssdk.String("subnet-2"),
800+
AvailabilityZone: awssdk.String("us-west-2a"),
801+
AvailabilityZoneId: awssdk.String("usw2-az1"),
802+
VpcId: awssdk.String("vpc-1"),
803+
Tags: []*ec2sdk.Tag{
804+
{
805+
Key: awssdk.String("kubernetes.io/cluster/kube-cluster"),
806+
Value: awssdk.String("owned"),
807+
},
808+
},
809+
},
810+
{
811+
SubnetId: awssdk.String("subnet-5"),
812+
AvailabilityZone: awssdk.String("us-west-2c"),
813+
AvailabilityZoneId: awssdk.String("usw2-az3"),
758814
VpcId: awssdk.String("vpc-1"),
759815
},
760816
},
761817
},
762818
{
763-
name: "subnets tagged for some other clusters get ignored",
819+
name: "subnets tagged for some other clusters doesn't get ignored, with SubnetsClusterTagCheck disabled",
764820
fields: fields{
765821
vpcID: "vpc-1",
766822
clusterName: "kube-cluster",
@@ -780,9 +836,9 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
780836
},
781837
output: []*ec2sdk.Subnet{
782838
{
783-
SubnetId: awssdk.String("subnet-1"),
784-
AvailabilityZone: awssdk.String("us-west-2a"),
785-
AvailabilityZoneId: awssdk.String("usw2-az1"),
839+
SubnetId: awssdk.String("subnet-3"),
840+
AvailabilityZone: awssdk.String("us-west-2c"),
841+
AvailabilityZoneId: awssdk.String("usw2-az3"),
786842
VpcId: awssdk.String("vpc-1"),
787843
Tags: []*ec2sdk.Tag{
788844
{
@@ -792,33 +848,21 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
792848
},
793849
},
794850
{
795-
SubnetId: awssdk.String("subnet-3"),
851+
SubnetId: awssdk.String("subnet-1"),
796852
AvailabilityZone: awssdk.String("us-west-2a"),
797853
AvailabilityZoneId: awssdk.String("usw2-az1"),
798854
VpcId: awssdk.String("vpc-1"),
799855
Tags: []*ec2sdk.Tag{
800856
{
801-
Key: awssdk.String("kubernetes.io/cluster/kube-cluster"),
802-
Value: awssdk.String("owned"),
803-
},
804-
},
805-
},
806-
{
807-
SubnetId: awssdk.String("subnet-4"),
808-
AvailabilityZone: awssdk.String("us-west-2b"),
809-
AvailabilityZoneId: awssdk.String("usw2-az2"),
810-
VpcId: awssdk.String("vpc-1"),
811-
Tags: []*ec2sdk.Tag{
812-
{
813-
Key: awssdk.String("kubernetes.io/cluster/no-cluster"),
857+
Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"),
814858
Value: awssdk.String("owned"),
815859
},
816860
},
817861
},
818862
{
819863
SubnetId: awssdk.String("subnet-2"),
820-
AvailabilityZone: awssdk.String("us-west-2a"),
821-
AvailabilityZoneId: awssdk.String("usw2-az1"),
864+
AvailabilityZone: awssdk.String("us-west-2c"),
865+
AvailabilityZoneId: awssdk.String("usw2-az3"),
822866
VpcId: awssdk.String("vpc-1"),
823867
Tags: []*ec2sdk.Tag{
824868
{
@@ -827,12 +871,6 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
827871
},
828872
},
829873
},
830-
{
831-
SubnetId: awssdk.String("subnet-5"),
832-
AvailabilityZone: awssdk.String("us-west-2c"),
833-
AvailabilityZoneId: awssdk.String("usw2-az3"),
834-
VpcId: awssdk.String("vpc-1"),
835-
},
836874
},
837875
},
838876
},
@@ -861,26 +899,33 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
861899
opts: []SubnetsResolveOption{
862900
WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeNetwork),
863901
WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing),
902+
WithSubnetsClusterTagCheck(false),
864903
},
865904
},
866905
want: []*ec2sdk.Subnet{
867906
{
868-
SubnetId: awssdk.String("subnet-2"),
907+
SubnetId: awssdk.String("subnet-1"),
869908
AvailabilityZone: awssdk.String("us-west-2a"),
870909
AvailabilityZoneId: awssdk.String("usw2-az1"),
871910
VpcId: awssdk.String("vpc-1"),
872911
Tags: []*ec2sdk.Tag{
873912
{
874-
Key: awssdk.String("kubernetes.io/cluster/kube-cluster"),
913+
Key: awssdk.String("kubernetes.io/cluster/some-other-cluster"),
875914
Value: awssdk.String("owned"),
876915
},
877916
},
878917
},
879918
{
880-
SubnetId: awssdk.String("subnet-5"),
919+
SubnetId: awssdk.String("subnet-2"),
881920
AvailabilityZone: awssdk.String("us-west-2c"),
882921
AvailabilityZoneId: awssdk.String("usw2-az3"),
883922
VpcId: awssdk.String("vpc-1"),
923+
Tags: []*ec2sdk.Tag{
924+
{
925+
Key: awssdk.String("kubernetes.io/cluster/kube-cluster"),
926+
Value: awssdk.String("owned"),
927+
},
928+
},
884929
},
885930
},
886931
},

0 commit comments

Comments
 (0)