Skip to content

Commit e85be6b

Browse files
nrbfad3t
nrb
authored and
fad3t
committed
Remove incorrect optimization and fixup tests
When the optimization was enabled, we were missing a bunch of creation calls that need to get defined. Add some partial matcher support in order to allow for generated names. Signed-off-by: Nolan Brubaker <[email protected]>
1 parent 5cb0f2c commit e85be6b

File tree

5 files changed

+272
-24
lines changed

5 files changed

+272
-24
lines changed

‎controllers/awscluster_controller_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,10 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
304304
mockedDescribeInstanceCall(m)
305305
mockedDescribeAvailabilityZones(m, []string{"us-east-1c", "us-east-1a"})
306306
mockedDescribeTargetGroupsCall(t, e)
307+
mockedCreateTargetGroupCall(t, e)
308+
mockedModifyTargetGroupAttributes(t, e)
307309
mockedDescribeListenersCall(t, e)
310+
mockedCreateListenerCall(t, e)
308311
}
309312

310313
expect(ec2Mock.EXPECT(), elbv2Mock.EXPECT())

‎controllers/helpers_test.go

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929

3030
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
3131
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
32+
"sigs.k8s.io/cluster-api-provider-aws/v2/test/helpers"
3233
"sigs.k8s.io/cluster-api-provider-aws/v2/test/mocks"
3334
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3435
"sigs.k8s.io/cluster-api/util/conditions"
@@ -39,6 +40,7 @@ const DNSName = "www.google.com"
3940
var (
4041
lbName = aws.String("test-cluster-apiserver")
4142
lbArn = aws.String("loadbalancer::arn")
43+
tgArn = aws.String("arn::target-group")
4244
describeLBInput = &elb.DescribeLoadBalancersInput{
4345
LoadBalancerNames: aws.StringSlice([]string{"test-cluster-apiserver"}),
4446
}
@@ -313,7 +315,7 @@ func mockedDescribeTargetGroupsCall(t *testing.T, m *mocks.MockELBV2APIMockRecor
313315
Port: new(int64),
314316
Protocol: new(string),
315317
ProtocolVersion: new(string),
316-
TargetGroupArn: aws.String("arn::targetgroup"),
318+
TargetGroupArn: tgArn,
317319
TargetGroupName: new(string),
318320
TargetType: new(string),
319321
UnhealthyThresholdCount: new(int64),
@@ -322,6 +324,68 @@ func mockedDescribeTargetGroupsCall(t *testing.T, m *mocks.MockELBV2APIMockRecor
322324
}, nil)
323325
}
324326

327+
func mockedCreateTargetGroupCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
328+
t.Helper()
329+
m.CreateTargetGroup(helpers.PartialMatchCreateTargetGroupInput(t, &elbv2.CreateTargetGroupInput{
330+
HealthCheckEnabled: aws.Bool(true),
331+
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
332+
HealthCheckPort: aws.String(infrav1.DefaultAPIServerPortString),
333+
HealthCheckProtocol: aws.String("TCP"),
334+
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
335+
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
336+
// Note: this is treated as a prefix with the partial matcher.
337+
Name: aws.String("apiserver-target"),
338+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
339+
Protocol: aws.String("TCP"),
340+
Tags: []*elbv2.Tag{
341+
{
342+
Key: aws.String("Name"),
343+
Value: aws.String("bar-apiserver"),
344+
},
345+
{
346+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
347+
Value: aws.String("owned"),
348+
},
349+
{
350+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
351+
Value: aws.String("apiserver"),
352+
},
353+
},
354+
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
355+
VpcId: aws.String("vpc-exists"),
356+
})).Return(&elbv2.CreateTargetGroupOutput{
357+
TargetGroups: []*elbv2.TargetGroup{{
358+
HealthCheckEnabled: aws.Bool(true),
359+
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
360+
HealthCheckPort: aws.String(infrav1.DefaultAPIServerPortString),
361+
HealthCheckProtocol: aws.String("TCP"),
362+
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
363+
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
364+
LoadBalancerArns: []*string{lbArn},
365+
Matcher: &elbv2.Matcher{},
366+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
367+
Protocol: aws.String("TCP"),
368+
TargetGroupArn: tgArn,
369+
TargetGroupName: aws.String("apiserver-target"),
370+
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
371+
VpcId: aws.String("vpc-exists"),
372+
}},
373+
}, nil)
374+
}
375+
376+
func mockedModifyTargetGroupAttributes(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
377+
t.Helper()
378+
m.ModifyTargetGroupAttributes(gomock.Eq(&elbv2.ModifyTargetGroupAttributesInput{
379+
TargetGroupArn: tgArn,
380+
Attributes: []*elbv2.TargetGroupAttribute{
381+
{
382+
Key: aws.String(infrav1.TargetGroupAttributeEnablePreserveClientIP),
383+
Value: aws.String("false"),
384+
},
385+
},
386+
})).Return(nil, nil)
387+
}
388+
325389
func mockedDescribeListenersCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
326390
t.Helper()
327391
m.DescribeListeners(gomock.Eq(&elbv2.DescribeListenersInput{
@@ -330,14 +394,56 @@ func mockedDescribeListenersCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder
330394
Return(&elbv2.DescribeListenersOutput{
331395
Listeners: []*elbv2.Listener{{
332396
DefaultActions: []*elbv2.Action{{
333-
TargetGroupArn: aws.String("arn::targetgroup"),
397+
TargetGroupArn: aws.String("arn::targetgroup-not-found"),
334398
}},
335399
ListenerArn: aws.String("arn::listener"),
336400
LoadBalancerArn: lbArn,
337401
}},
338402
}, nil)
339403
}
340404

405+
func mockedCreateListenerCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
406+
t.Helper()
407+
m.CreateListener(gomock.Eq(&elbv2.CreateListenerInput{
408+
DefaultActions: []*elbv2.Action{
409+
{
410+
TargetGroupArn: tgArn,
411+
Type: aws.String(elbv2.ActionTypeEnumForward),
412+
},
413+
},
414+
LoadBalancerArn: lbArn,
415+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
416+
Protocol: aws.String("TCP"),
417+
Tags: []*elbv2.Tag{
418+
{
419+
Key: aws.String("Name"),
420+
Value: aws.String("test-cluster-apiserver"),
421+
},
422+
{
423+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
424+
Value: aws.String("owned"),
425+
},
426+
{
427+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
428+
Value: aws.String("apiserver"),
429+
},
430+
},
431+
})).Return(&elbv2.CreateListenerOutput{
432+
Listeners: []*elbv2.Listener{
433+
{
434+
DefaultActions: []*elbv2.Action{
435+
{
436+
TargetGroupArn: tgArn,
437+
Type: aws.String(elbv2.ActionTypeEnumForward),
438+
},
439+
},
440+
ListenerArn: aws.String("listener::arn"),
441+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
442+
Protocol: aws.String("TCP"),
443+
},
444+
}}, nil)
445+
}
446+
341447
func mockedDeleteLBCalls(expectV2Call bool, mv2 *mocks.MockELBV2APIMockRecorder, m *mocks.MockELBAPIMockRecorder) {
342448
if expectV2Call {
343449
mv2.DescribeLoadBalancers(gomock.Any()).Return(describeLBOutputV2, nil)

‎pkg/cloud/services/elb/loadbalancer.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,10 +1550,6 @@ func (s *Service) reconcileTargetGroupsAndListeners(lbARN string, spec *infrav1.
15501550
s.scope.Error(err, "could not describe listeners for load balancer", "arn", lbARN)
15511551
}
15521552

1553-
if len(existingTargetGroups.TargetGroups) == len(existingListeners.Listeners) && len(existingListeners.Listeners) == len(spec.ELBListeners) {
1554-
return existingTargetGroups.TargetGroups, existingListeners.Listeners, nil
1555-
}
1556-
15571553
createdTargetGroups := make([]*elbv2.TargetGroup, 0, len(spec.ELBListeners))
15581554
createdListeners := make([]*elbv2.Listener, 0, len(spec.ELBListeners))
15591555

‎pkg/cloud/services/elb/loadbalancer_test.go

Lines changed: 95 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040

4141
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
4242
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
43+
"sigs.k8s.io/cluster-api-provider-aws/v2/test/helpers"
4344
"sigs.k8s.io/cluster-api-provider-aws/v2/test/mocks"
4445
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
4546
"sigs.k8s.io/cluster-api/util/conditions"
@@ -2211,6 +2212,7 @@ func TestReconcileV2LB(t *testing.T) {
22112212
clusterSubnetID = "subnet-1"
22122213
elbName = "bar-apiserver"
22132214
elbArn = "arn::apiserver"
2215+
tgArn = "arn::target-group"
22142216
vpcID = "vpc-id"
22152217
az = "us-west-1a"
22162218
)
@@ -2322,24 +2324,11 @@ func TestReconcileV2LB(t *testing.T) {
23222324
NextMarker: new(string),
23232325
TargetGroups: []*elbv2.TargetGroup{
23242326
{
2325-
HealthCheckEnabled: aws.Bool(true),
2326-
HealthCheckIntervalSeconds: new(int64),
2327-
HealthCheckPath: new(string),
2328-
HealthCheckPort: new(string),
2329-
HealthCheckProtocol: new(string),
2330-
HealthCheckTimeoutSeconds: new(int64),
2331-
HealthyThresholdCount: new(int64),
2332-
IpAddressType: new(string),
2333-
LoadBalancerArns: []*string{aws.String(elbArn)},
2334-
Matcher: &elbv2.Matcher{},
2335-
Port: new(int64),
2336-
Protocol: new(string),
2337-
ProtocolVersion: new(string),
2338-
TargetGroupArn: aws.String("arn::targetgroup"),
2339-
TargetGroupName: new(string),
2340-
TargetType: new(string),
2341-
UnhealthyThresholdCount: new(int64),
2342-
VpcId: new(string),
2327+
HealthCheckEnabled: aws.Bool(true),
2328+
LoadBalancerArns: []*string{aws.String(elbArn)},
2329+
Matcher: &elbv2.Matcher{},
2330+
TargetGroupArn: aws.String(tgArn),
2331+
TargetGroupName: aws.String("targetGroup"),
23432332
}},
23442333
}, nil)
23452334
m.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{
@@ -2352,6 +2341,56 @@ func TestReconcileV2LB(t *testing.T) {
23522341
}}).
23532342
Return(&elbv2.ModifyLoadBalancerAttributesOutput{}, nil)
23542343

2344+
m.CreateTargetGroup(helpers.PartialMatchCreateTargetGroupInput(t, &elbv2.CreateTargetGroupInput{
2345+
HealthCheckEnabled: aws.Bool(true),
2346+
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
2347+
HealthCheckPort: aws.String(infrav1.DefaultAPIServerPortString),
2348+
HealthCheckProtocol: aws.String("TCP"),
2349+
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
2350+
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
2351+
// Note: this is treated as a prefix with the partial matcher.
2352+
Name: aws.String("apiserver-target"),
2353+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
2354+
Protocol: aws.String("TCP"),
2355+
Tags: []*elbv2.Tag{
2356+
{
2357+
Key: aws.String("Name"),
2358+
Value: aws.String("bar-apiserver"),
2359+
},
2360+
{
2361+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/bar"),
2362+
Value: aws.String("owned"),
2363+
},
2364+
{
2365+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
2366+
Value: aws.String("apiserver"),
2367+
},
2368+
},
2369+
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
2370+
VpcId: aws.String(vpcID),
2371+
})).Return(&elbv2.CreateTargetGroupOutput{
2372+
TargetGroups: []*elbv2.TargetGroup{
2373+
{
2374+
TargetGroupArn: aws.String(tgArn),
2375+
VpcId: aws.String(vpcID),
2376+
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
2377+
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
2378+
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
2379+
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
2380+
},
2381+
},
2382+
}, nil)
2383+
2384+
m.ModifyTargetGroupAttributes(gomock.Eq(&elbv2.ModifyTargetGroupAttributesInput{
2385+
TargetGroupArn: aws.String(tgArn),
2386+
Attributes: []*elbv2.TargetGroupAttribute{
2387+
{
2388+
Key: aws.String(infrav1.TargetGroupAttributeEnablePreserveClientIP),
2389+
Value: aws.String("false"),
2390+
},
2391+
},
2392+
})).Return(nil, nil)
2393+
23552394
m.DescribeListeners(gomock.Eq(&elbv2.DescribeListenersInput{
23562395
LoadBalancerArn: aws.String(elbArn),
23572396
})).
@@ -2364,6 +2403,44 @@ func TestReconcileV2LB(t *testing.T) {
23642403
LoadBalancerArn: aws.String(elbArn),
23652404
}},
23662405
}, nil)
2406+
m.CreateListener(gomock.Eq(&elbv2.CreateListenerInput{
2407+
DefaultActions: []*elbv2.Action{
2408+
{
2409+
TargetGroupArn: aws.String(tgArn),
2410+
Type: aws.String(elbv2.ActionTypeEnumForward),
2411+
},
2412+
},
2413+
LoadBalancerArn: aws.String(elbArn),
2414+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
2415+
Protocol: aws.String("TCP"),
2416+
Tags: []*elbv2.Tag{
2417+
{
2418+
Key: aws.String("Name"),
2419+
Value: aws.String("bar-apiserver"),
2420+
},
2421+
{
2422+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/bar"),
2423+
Value: aws.String("owned"),
2424+
},
2425+
{
2426+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
2427+
Value: aws.String("apiserver"),
2428+
},
2429+
},
2430+
})).Return(&elbv2.CreateListenerOutput{
2431+
Listeners: []*elbv2.Listener{
2432+
{
2433+
DefaultActions: []*elbv2.Action{
2434+
{
2435+
TargetGroupArn: aws.String(tgArn),
2436+
Type: aws.String(elbv2.ActionTypeEnumForward),
2437+
},
2438+
},
2439+
ListenerArn: aws.String("listener::arn"),
2440+
Port: aws.Int64(infrav1.DefaultAPIServerPort),
2441+
Protocol: aws.String("TCP"),
2442+
},
2443+
}}, nil)
23672444
m.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: aws.String(elbArn)}).Return(
23682445
&elbv2.DescribeLoadBalancerAttributesOutput{
23692446
Attributes: []*elbv2.LoadBalancerAttribute{

‎test/helpers/matchers.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package helpers
18+
19+
import (
20+
"fmt"
21+
"strings"
22+
"testing"
23+
24+
"github.com/aws/aws-sdk-go/service/elbv2"
25+
"github.com/golang/mock/gomock"
26+
)
27+
28+
// PartialMatchCreateTargetGroupInput matches a partial CreateTargetGroupInput struct based on fuzzy matching rules.
29+
func PartialMatchCreateTargetGroupInput(t *testing.T, i *elbv2.CreateTargetGroupInput) gomock.Matcher {
30+
t.Helper()
31+
return &createTargetGroupInputPartialMatcher{
32+
in: i,
33+
t: t,
34+
}
35+
}
36+
37+
// createTargetGroupInputPartialMatcher conforms to the gomock.Matcher interface in order to implement a match against a partial
38+
// CreateTargetGroupInput expected value.
39+
// In particular, the TargetGroupName expected value is used as a prefix, in order to support generated names.
40+
type createTargetGroupInputPartialMatcher struct {
41+
in *elbv2.CreateTargetGroupInput
42+
t *testing.T
43+
}
44+
45+
func (m *createTargetGroupInputPartialMatcher) Matches(x interface{}) bool {
46+
actual, ok := x.(*elbv2.CreateTargetGroupInput)
47+
if !ok {
48+
return false
49+
}
50+
51+
// Check for a perfect match across all fields first.
52+
eq := gomock.Eq(m.in).Matches(actual)
53+
54+
if !eq && (actual.Name != nil && m.in.Name != nil) {
55+
// If the actual name is prefixed with the expected value, then it matches
56+
if (*actual.Name != *m.in.Name) && strings.HasPrefix(*actual.Name, *m.in.Name) {
57+
return true
58+
}
59+
}
60+
61+
return eq
62+
}
63+
64+
func (m *createTargetGroupInputPartialMatcher) String() string {
65+
return fmt.Sprintf("%v (%T)", m.in, m.in)
66+
}

0 commit comments

Comments
 (0)