Skip to content

Commit 4685eea

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#76343 from frankgreco/ensureLoadBalancer-unit-tests
Begin Adding Unit Tests for EnsureLoadBalancer
2 parents 8d38f3f + 91cdcce commit 4685eea

File tree

2 files changed

+199
-49
lines changed

2 files changed

+199
-49
lines changed

‎pkg/cloudprovider/providers/aws/aws_loadbalancer.go‎

Lines changed: 65 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,59 +1098,14 @@ func (c *Cloud) ensureLoadBalancer(namespacedName types.NamespacedName, loadBala
10981098
}
10991099

11001100
{
1101-
// Sync listeners
1102-
listenerDescriptions := loadBalancer.ListenerDescriptions
1103-
1104-
foundSet := make(map[int]bool)
1105-
removals := []*int64{}
1106-
for _, listenerDescription := range listenerDescriptions {
1107-
actual := listenerDescription.Listener
1108-
if actual == nil {
1109-
klog.Warning("Ignoring empty listener in AWS loadbalancer: ", loadBalancerName)
1110-
continue
1111-
}
1112-
1113-
found := -1
1114-
for i, expected := range listeners {
1115-
if !elbProtocolsAreEqual(actual.Protocol, expected.Protocol) {
1116-
continue
1117-
}
1118-
if !elbProtocolsAreEqual(actual.InstanceProtocol, expected.InstanceProtocol) {
1119-
continue
1120-
}
1121-
if aws.Int64Value(actual.InstancePort) != aws.Int64Value(expected.InstancePort) {
1122-
continue
1123-
}
1124-
if aws.Int64Value(actual.LoadBalancerPort) != aws.Int64Value(expected.LoadBalancerPort) {
1125-
continue
1126-
}
1127-
if !awsArnEquals(actual.SSLCertificateId, expected.SSLCertificateId) {
1128-
continue
1129-
}
1130-
found = i
1131-
}
1132-
if found != -1 {
1133-
foundSet[found] = true
1134-
} else {
1135-
removals = append(removals, actual.LoadBalancerPort)
1136-
}
1137-
}
1138-
1139-
additions := []*elb.Listener{}
1140-
for i := range listeners {
1141-
if foundSet[i] {
1142-
continue
1143-
}
1144-
additions = append(additions, listeners[i])
1145-
}
1101+
additions, removals := syncElbListeners(loadBalancerName, listeners, loadBalancer.ListenerDescriptions)
11461102

11471103
if len(removals) != 0 {
11481104
request := &elb.DeleteLoadBalancerListenersInput{}
11491105
request.LoadBalancerName = aws.String(loadBalancerName)
11501106
request.LoadBalancerPorts = removals
11511107
klog.V(2).Info("Deleting removed load balancer listeners")
1152-
_, err := c.elb.DeleteLoadBalancerListeners(request)
1153-
if err != nil {
1108+
if _, err := c.elb.DeleteLoadBalancerListeners(request); err != nil {
11541109
return nil, fmt.Errorf("error deleting AWS loadbalancer listeners: %q", err)
11551110
}
11561111
dirty = true
@@ -1161,8 +1116,7 @@ func (c *Cloud) ensureLoadBalancer(namespacedName types.NamespacedName, loadBala
11611116
request.LoadBalancerName = aws.String(loadBalancerName)
11621117
request.Listeners = additions
11631118
klog.V(2).Info("Creating added load balancer listeners")
1164-
_, err := c.elb.CreateLoadBalancerListeners(request)
1165-
if err != nil {
1119+
if _, err := c.elb.CreateLoadBalancerListeners(request); err != nil {
11661120
return nil, fmt.Errorf("error creating AWS loadbalancer listeners: %q", err)
11671121
}
11681122
dirty = true
@@ -1289,6 +1243,68 @@ func (c *Cloud) ensureLoadBalancer(namespacedName types.NamespacedName, loadBala
12891243
return loadBalancer, nil
12901244
}
12911245

1246+
// syncElbListeners computes a plan to reconcile the desired vs actual state of the listeners on an ELB
1247+
// NOTE: there exists an O(nlgn) implementation for this function. However, as the default limit of
1248+
// listeners per elb is 100, this implementation is reduced from O(m*n) => O(n).
1249+
func syncElbListeners(loadBalancerName string, listeners []*elb.Listener, listenerDescriptions []*elb.ListenerDescription) ([]*elb.Listener, []*int64) {
1250+
foundSet := make(map[int]bool)
1251+
removals := []*int64{}
1252+
additions := []*elb.Listener{}
1253+
1254+
for _, listenerDescription := range listenerDescriptions {
1255+
actual := listenerDescription.Listener
1256+
if actual == nil {
1257+
klog.Warning("Ignoring empty listener in AWS loadbalancer: ", loadBalancerName)
1258+
continue
1259+
}
1260+
1261+
found := false
1262+
for i, expected := range listeners {
1263+
if expected == nil {
1264+
klog.Warning("Ignoring empty desired listener for loadbalancer: ", loadBalancerName)
1265+
continue
1266+
}
1267+
if elbListenersAreEqual(actual, expected) {
1268+
// The current listener on the actual
1269+
// elb is in the set of desired listeners.
1270+
foundSet[i] = true
1271+
found = true
1272+
break
1273+
}
1274+
}
1275+
if !found {
1276+
removals = append(removals, actual.LoadBalancerPort)
1277+
}
1278+
}
1279+
1280+
for i := range listeners {
1281+
if !foundSet[i] {
1282+
additions = append(additions, listeners[i])
1283+
}
1284+
}
1285+
1286+
return additions, removals
1287+
}
1288+
1289+
func elbListenersAreEqual(actual, expected *elb.Listener) bool {
1290+
if !elbProtocolsAreEqual(actual.Protocol, expected.Protocol) {
1291+
return false
1292+
}
1293+
if !elbProtocolsAreEqual(actual.InstanceProtocol, expected.InstanceProtocol) {
1294+
return false
1295+
}
1296+
if aws.Int64Value(actual.InstancePort) != aws.Int64Value(expected.InstancePort) {
1297+
return false
1298+
}
1299+
if aws.Int64Value(actual.LoadBalancerPort) != aws.Int64Value(expected.LoadBalancerPort) {
1300+
return false
1301+
}
1302+
if !awsArnEquals(actual.SSLCertificateId, expected.SSLCertificateId) {
1303+
return false
1304+
}
1305+
return true
1306+
}
1307+
12921308
func createSubnetMappings(subnetIDs []string) []*elbv2.SubnetMapping {
12931309
response := []*elbv2.SubnetMapping{}
12941310

‎pkg/cloudprovider/providers/aws/aws_loadbalancer_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222

2323
"github.com/aws/aws-sdk-go/aws"
2424
"github.com/aws/aws-sdk-go/service/ec2"
25+
"github.com/aws/aws-sdk-go/service/elb"
26+
"github.com/stretchr/testify/assert"
2527
)
2628

2729
func TestElbProtocolsAreEqual(t *testing.T) {
@@ -222,3 +224,135 @@ func TestSecurityGroupFiltering(t *testing.T) {
222224
}
223225

224226
}
227+
228+
func TestSyncElbListeners(t *testing.T) {
229+
tests := []struct {
230+
name string
231+
loadBalancerName string
232+
listeners []*elb.Listener
233+
listenerDescriptions []*elb.ListenerDescription
234+
toCreate []*elb.Listener
235+
toDelete []*int64
236+
}{
237+
{
238+
name: "no edge cases",
239+
loadBalancerName: "lb_one",
240+
listeners: []*elb.Listener{
241+
{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")},
242+
{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP"), SSLCertificateId: aws.String("def-456")},
243+
{InstancePort: aws.Int64(8443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(8443), Protocol: aws.String("TCP"), SSLCertificateId: aws.String("def-456")},
244+
},
245+
listenerDescriptions: []*elb.ListenerDescription{
246+
{Listener: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")}},
247+
{Listener: &elb.Listener{InstancePort: aws.Int64(8443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(8443), Protocol: aws.String("TCP"), SSLCertificateId: aws.String("def-456")}},
248+
},
249+
toDelete: []*int64{
250+
aws.Int64(80),
251+
},
252+
toCreate: []*elb.Listener{
253+
{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")},
254+
{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP"), SSLCertificateId: aws.String("def-456")},
255+
},
256+
},
257+
{
258+
name: "no listeners to delete",
259+
loadBalancerName: "lb_two",
260+
listeners: []*elb.Listener{
261+
{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")},
262+
{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP"), SSLCertificateId: aws.String("def-456")},
263+
},
264+
listenerDescriptions: []*elb.ListenerDescription{
265+
{Listener: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")}},
266+
},
267+
toCreate: []*elb.Listener{
268+
{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP"), SSLCertificateId: aws.String("def-456")},
269+
},
270+
toDelete: []*int64{},
271+
},
272+
{
273+
name: "no listeners to create",
274+
loadBalancerName: "lb_three",
275+
listeners: []*elb.Listener{
276+
{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")},
277+
},
278+
listenerDescriptions: []*elb.ListenerDescription{
279+
{Listener: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")}},
280+
{Listener: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")}},
281+
},
282+
toDelete: []*int64{
283+
aws.Int64(80),
284+
},
285+
toCreate: []*elb.Listener{},
286+
},
287+
{
288+
name: "nil actual listener",
289+
loadBalancerName: "lb_four",
290+
listeners: []*elb.Listener{
291+
{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP")},
292+
},
293+
listenerDescriptions: []*elb.ListenerDescription{
294+
{Listener: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP"), SSLCertificateId: aws.String("abc-123")}},
295+
{Listener: nil},
296+
},
297+
toDelete: []*int64{
298+
aws.Int64(443),
299+
},
300+
toCreate: []*elb.Listener{
301+
{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("HTTP")},
302+
},
303+
},
304+
}
305+
306+
for _, test := range tests {
307+
t.Run(test.name, func(t *testing.T) {
308+
additions, removals := syncElbListeners(test.loadBalancerName, test.listeners, test.listenerDescriptions)
309+
assert.Equal(t, additions, test.toCreate)
310+
assert.Equal(t, removals, test.toDelete)
311+
})
312+
}
313+
}
314+
315+
func TestElbListenersAreEqual(t *testing.T) {
316+
tests := []struct {
317+
name string
318+
expected, actual *elb.Listener
319+
equal bool
320+
}{
321+
{
322+
name: "should be equal",
323+
expected: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
324+
actual: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
325+
equal: true,
326+
},
327+
{
328+
name: "instance port should be different",
329+
expected: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
330+
actual: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
331+
equal: false,
332+
},
333+
{
334+
name: "instance protocol should be different",
335+
expected: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("HTTP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
336+
actual: &elb.Listener{InstancePort: aws.Int64(80), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
337+
equal: false,
338+
},
339+
{
340+
name: "load balancer port should be different",
341+
expected: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(443), Protocol: aws.String("TCP")},
342+
actual: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
343+
equal: false,
344+
},
345+
{
346+
name: "protocol should be different",
347+
expected: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("TCP")},
348+
actual: &elb.Listener{InstancePort: aws.Int64(443), InstanceProtocol: aws.String("TCP"), LoadBalancerPort: aws.Int64(80), Protocol: aws.String("HTTP")},
349+
equal: false,
350+
},
351+
}
352+
353+
for _, test := range tests {
354+
t.Run(test.name, func(t *testing.T) {
355+
assert.Equal(t, test.equal, elbListenersAreEqual(test.expected, test.actual))
356+
})
357+
}
358+
}

0 commit comments

Comments
 (0)