Skip to content

Commit

Permalink
Remove incorrect optimization and fixup tests
Browse files Browse the repository at this point in the history
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 <nolan@nbrubaker.com>
  • Loading branch information
nrb committed Jun 10, 2024
1 parent 34edeed commit cc2cfe4
Show file tree
Hide file tree
Showing 5 changed files with 272 additions and 24 deletions.
3 changes: 3 additions & 0 deletions controllers/awscluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,10 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
mockedDescribeInstanceCall(m)
mockedDescribeAvailabilityZones(m, []string{"us-east-1c", "us-east-1a"})
mockedDescribeTargetGroupsCall(t, e)
mockedCreateTargetGroupCall(t, e)
mockedModifyTargetGroupAttributes(t, e)
mockedDescribeListenersCall(t, e)
mockedCreateListenerCall(t, e)
}

expect(ec2Mock.EXPECT(), elbv2Mock.EXPECT())
Expand Down
110 changes: 108 additions & 2 deletions controllers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
"sigs.k8s.io/cluster-api-provider-aws/v2/test/helpers"
"sigs.k8s.io/cluster-api-provider-aws/v2/test/mocks"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
Expand All @@ -39,6 +40,7 @@ const DNSName = "www.google.com"
var (
lbName = aws.String("test-cluster-apiserver")
lbArn = aws.String("loadbalancer::arn")
tgArn = aws.String("arn::target-group")
describeLBInput = &elb.DescribeLoadBalancersInput{
LoadBalancerNames: aws.StringSlice([]string{"test-cluster-apiserver"}),
}
Expand Down Expand Up @@ -313,7 +315,7 @@ func mockedDescribeTargetGroupsCall(t *testing.T, m *mocks.MockELBV2APIMockRecor
Port: new(int64),
Protocol: new(string),
ProtocolVersion: new(string),
TargetGroupArn: aws.String("arn::targetgroup"),
TargetGroupArn: tgArn,
TargetGroupName: new(string),
TargetType: new(string),
UnhealthyThresholdCount: new(int64),
Expand All @@ -322,6 +324,68 @@ func mockedDescribeTargetGroupsCall(t *testing.T, m *mocks.MockELBV2APIMockRecor
}, nil)
}

func mockedCreateTargetGroupCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
t.Helper()
m.CreateTargetGroup(helpers.PartialMatchCreateTargetGroupInput(t, &elbv2.CreateTargetGroupInput{
HealthCheckEnabled: aws.Bool(true),
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
HealthCheckPort: aws.String(infrav1.DefaultAPIServerPortString),
HealthCheckProtocol: aws.String("TCP"),
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
// Note: this is treated as a prefix with the partial matcher.
Name: aws.String("apiserver-target"),
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
Tags: []*elbv2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("bar-apiserver"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("apiserver"),
},
},
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
VpcId: aws.String("vpc-exists"),
})).Return(&elbv2.CreateTargetGroupOutput{
TargetGroups: []*elbv2.TargetGroup{{
HealthCheckEnabled: aws.Bool(true),
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
HealthCheckPort: aws.String(infrav1.DefaultAPIServerPortString),
HealthCheckProtocol: aws.String("TCP"),
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
LoadBalancerArns: []*string{lbArn},
Matcher: &elbv2.Matcher{},
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
TargetGroupArn: tgArn,
TargetGroupName: aws.String("apiserver-target"),
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
VpcId: aws.String("vpc-exists"),
}},
}, nil)
}

func mockedModifyTargetGroupAttributes(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
t.Helper()
m.ModifyTargetGroupAttributes(gomock.Eq(&elbv2.ModifyTargetGroupAttributesInput{
TargetGroupArn: tgArn,
Attributes: []*elbv2.TargetGroupAttribute{
{
Key: aws.String(infrav1.TargetGroupAttributeEnablePreserveClientIP),
Value: aws.String("false"),
},
},
})).Return(nil, nil)
}

func mockedDescribeListenersCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
t.Helper()
m.DescribeListeners(gomock.Eq(&elbv2.DescribeListenersInput{
Expand All @@ -330,14 +394,56 @@ func mockedDescribeListenersCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder
Return(&elbv2.DescribeListenersOutput{
Listeners: []*elbv2.Listener{{
DefaultActions: []*elbv2.Action{{
TargetGroupArn: aws.String("arn::targetgroup"),
TargetGroupArn: aws.String("arn::targetgroup-not-found"),
}},
ListenerArn: aws.String("arn::listener"),
LoadBalancerArn: lbArn,
}},
}, nil)
}

func mockedCreateListenerCall(t *testing.T, m *mocks.MockELBV2APIMockRecorder) {
t.Helper()
m.CreateListener(gomock.Eq(&elbv2.CreateListenerInput{
DefaultActions: []*elbv2.Action{
{
TargetGroupArn: tgArn,
Type: aws.String(elbv2.ActionTypeEnumForward),
},
},
LoadBalancerArn: lbArn,
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
Tags: []*elbv2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-apiserver"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("apiserver"),
},
},
})).Return(&elbv2.CreateListenerOutput{
Listeners: []*elbv2.Listener{
{
DefaultActions: []*elbv2.Action{
{
TargetGroupArn: tgArn,
Type: aws.String(elbv2.ActionTypeEnumForward),
},
},
ListenerArn: aws.String("listener::arn"),
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
},
}}, nil)
}

func mockedDeleteLBCalls(expectV2Call bool, mv2 *mocks.MockELBV2APIMockRecorder, m *mocks.MockELBAPIMockRecorder) {
if expectV2Call {
mv2.DescribeLoadBalancers(gomock.Any()).Return(describeLBOutputV2, nil)
Expand Down
4 changes: 0 additions & 4 deletions pkg/cloud/services/elb/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1550,10 +1550,6 @@ func (s *Service) reconcileTargetGroupsAndListeners(lbARN string, spec *infrav1.
s.scope.Error(err, "could not describe listeners for load balancer", "arn", lbARN)
}

if len(existingTargetGroups.TargetGroups) == len(existingListeners.Listeners) && len(existingListeners.Listeners) == len(spec.ELBListeners) {
return existingTargetGroups.TargetGroups, existingListeners.Listeners, nil
}

createdTargetGroups := make([]*elbv2.TargetGroup, 0, len(spec.ELBListeners))
createdListeners := make([]*elbv2.Listener, 0, len(spec.ELBListeners))

Expand Down
113 changes: 95 additions & 18 deletions pkg/cloud/services/elb/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
"sigs.k8s.io/cluster-api-provider-aws/v2/test/helpers"
"sigs.k8s.io/cluster-api-provider-aws/v2/test/mocks"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
Expand Down Expand Up @@ -2211,6 +2212,7 @@ func TestReconcileV2LB(t *testing.T) {
clusterSubnetID = "subnet-1"
elbName = "bar-apiserver"
elbArn = "arn::apiserver"
tgArn = "arn::target-group"
vpcID = "vpc-id"
az = "us-west-1a"
)
Expand Down Expand Up @@ -2322,24 +2324,11 @@ func TestReconcileV2LB(t *testing.T) {
NextMarker: new(string),
TargetGroups: []*elbv2.TargetGroup{
{
HealthCheckEnabled: aws.Bool(true),
HealthCheckIntervalSeconds: new(int64),
HealthCheckPath: new(string),
HealthCheckPort: new(string),
HealthCheckProtocol: new(string),
HealthCheckTimeoutSeconds: new(int64),
HealthyThresholdCount: new(int64),
IpAddressType: new(string),
LoadBalancerArns: []*string{aws.String(elbArn)},
Matcher: &elbv2.Matcher{},
Port: new(int64),
Protocol: new(string),
ProtocolVersion: new(string),
TargetGroupArn: aws.String("arn::targetgroup"),
TargetGroupName: new(string),
TargetType: new(string),
UnhealthyThresholdCount: new(int64),
VpcId: new(string),
HealthCheckEnabled: aws.Bool(true),
LoadBalancerArns: []*string{aws.String(elbArn)},
Matcher: &elbv2.Matcher{},
TargetGroupArn: aws.String(tgArn),
TargetGroupName: aws.String("targetGroup"),
}},
}, nil)
m.ModifyLoadBalancerAttributes(&elbv2.ModifyLoadBalancerAttributesInput{
Expand All @@ -2352,6 +2341,56 @@ func TestReconcileV2LB(t *testing.T) {
}}).
Return(&elbv2.ModifyLoadBalancerAttributesOutput{}, nil)

m.CreateTargetGroup(helpers.PartialMatchCreateTargetGroupInput(t, &elbv2.CreateTargetGroupInput{
HealthCheckEnabled: aws.Bool(true),
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
HealthCheckPort: aws.String(infrav1.DefaultAPIServerPortString),
HealthCheckProtocol: aws.String("TCP"),
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
// Note: this is treated as a prefix with the partial matcher.
Name: aws.String("apiserver-target"),
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
Tags: []*elbv2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("bar-apiserver"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/bar"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("apiserver"),
},
},
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
VpcId: aws.String(vpcID),
})).Return(&elbv2.CreateTargetGroupOutput{
TargetGroups: []*elbv2.TargetGroup{
{
TargetGroupArn: aws.String(tgArn),
VpcId: aws.String(vpcID),
HealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerHealthThresholdCount),
UnhealthyThresholdCount: aws.Int64(infrav1.DefaultAPIServerUnhealthThresholdCount),
HealthCheckIntervalSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckIntervalSec),
HealthCheckTimeoutSeconds: aws.Int64(infrav1.DefaultAPIServerHealthCheckTimeoutSec),
},
},
}, nil)

m.ModifyTargetGroupAttributes(gomock.Eq(&elbv2.ModifyTargetGroupAttributesInput{
TargetGroupArn: aws.String(tgArn),
Attributes: []*elbv2.TargetGroupAttribute{
{
Key: aws.String(infrav1.TargetGroupAttributeEnablePreserveClientIP),
Value: aws.String("false"),
},
},
})).Return(nil, nil)

m.DescribeListeners(gomock.Eq(&elbv2.DescribeListenersInput{
LoadBalancerArn: aws.String(elbArn),
})).
Expand All @@ -2364,6 +2403,44 @@ func TestReconcileV2LB(t *testing.T) {
LoadBalancerArn: aws.String(elbArn),
}},
}, nil)
m.CreateListener(gomock.Eq(&elbv2.CreateListenerInput{
DefaultActions: []*elbv2.Action{
{
TargetGroupArn: aws.String(tgArn),
Type: aws.String(elbv2.ActionTypeEnumForward),
},
},
LoadBalancerArn: aws.String(elbArn),
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
Tags: []*elbv2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("bar-apiserver"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/bar"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("apiserver"),
},
},
})).Return(&elbv2.CreateListenerOutput{
Listeners: []*elbv2.Listener{
{
DefaultActions: []*elbv2.Action{
{
TargetGroupArn: aws.String(tgArn),
Type: aws.String(elbv2.ActionTypeEnumForward),
},
},
ListenerArn: aws.String("listener::arn"),
Port: aws.Int64(infrav1.DefaultAPIServerPort),
Protocol: aws.String("TCP"),
},
}}, nil)
m.DescribeLoadBalancerAttributes(&elbv2.DescribeLoadBalancerAttributesInput{LoadBalancerArn: aws.String(elbArn)}).Return(
&elbv2.DescribeLoadBalancerAttributesOutput{
Attributes: []*elbv2.LoadBalancerAttribute{
Expand Down
66 changes: 66 additions & 0 deletions test/helpers/matchers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package helpers

import (
"fmt"
"strings"
"testing"

"github.com/aws/aws-sdk-go/service/elbv2"
"github.com/golang/mock/gomock"
)

// PartialMatchCreateTargetGroupInput matches a partial CreateTargetGroupInput struct based on fuzzy matching rules.
func PartialMatchCreateTargetGroupInput(t *testing.T, i *elbv2.CreateTargetGroupInput) gomock.Matcher {
t.Helper()
return &createTargetGroupInputPartialMatcher{
in: i,
t: t,
}
}

// createTargetGroupInputPartialMatcher conforms to the gomock.Matcher interface in order to implement a match against a partial
// CreateTargetGroupInput expected value.
// In particular, the TargetGroupName expected value is used as a prefix, in order to support generated names.
type createTargetGroupInputPartialMatcher struct {
in *elbv2.CreateTargetGroupInput
t *testing.T
}

func (m *createTargetGroupInputPartialMatcher) Matches(x interface{}) bool {
actual, ok := x.(*elbv2.CreateTargetGroupInput)
if !ok {
return false
}

// Check for a perfect match across all fields first.
eq := gomock.Eq(m.in).Matches(actual)

if !eq && (actual.Name != nil && m.in.Name != nil) {
// If the actual name is prefixed with the expected value, then it matches
if (*actual.Name != *m.in.Name) && strings.HasPrefix(*actual.Name, *m.in.Name) {
return true
}
}

return eq
}

func (m *createTargetGroupInputPartialMatcher) String() string {
return fmt.Sprintf("%v (%T)", m.in, m.in)
}

0 comments on commit cc2cfe4

Please sign in to comment.