Skip to content

Commit

Permalink
Support > 5 ports in L4 ILB.
Browse files Browse the repository at this point in the history
Added logic to set AllPorts field if more than 5 ports are specified.
  • Loading branch information
prameshj committed Mar 18, 2021
1 parent 7e48dab commit 55fcfd7
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const (
ILBFinalizerV2 = "gke.networking.io/l4-ilb-v2"
// maxInstancesPerInstanceGroup defines maximum number of VMs per InstanceGroup.
maxInstancesPerInstanceGroup = 1000
// maxL4ILBPorts is the maximum number of ports that can be specified in an L4 ILB Forwarding Rule. Beyond this, "AllPorts" field should be used.
maxL4ILBPorts = 5
)

func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
Expand Down Expand Up @@ -201,6 +203,10 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
if options.AllowGlobalAccess {
newFwdRule.AllowGlobalAccess = options.AllowGlobalAccess
}
if len(ports) > maxL4ILBPorts {
newFwdRule.Ports = nil
newFwdRule.AllPorts = true
}

fwdRuleDeleted := false
if existingFwdRule != nil && !forwardingRulesEqual(existingFwdRule, newFwdRule) {
Expand Down Expand Up @@ -994,6 +1000,7 @@ func forwardingRulesEqual(old, new *compute.ForwardingRule) bool {
old.IPProtocol == new.IPProtocol &&
old.LoadBalancingScheme == new.LoadBalancingScheme &&
equalStringSets(old.Ports, new.Ports) &&
old.AllPorts == new.AllPorts &&
oldResourceID.Equal(newResourceID) &&
old.AllowGlobalAccess == new.AllowGlobalAccess &&
old.Subnetwork == new.Subnetwork
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,15 @@ func TestForwardingRulesEqual(t *testing.T) {
AllowGlobalAccess: true,
BackendService: "http://compute.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
{
Name: "udp-fwd-rule-allports",
IPAddress: "10.0.0.0",
Ports: []string{"123"},
AllPorts: true,
IPProtocol: "UDP",
LoadBalancingScheme: string(cloud.SchemeInternal),
BackendService: "http://www.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
}

for _, tc := range []struct {
Expand Down Expand Up @@ -1389,6 +1398,12 @@ func TestForwardingRulesEqual(t *testing.T) {
newFwdRule: fwdRules[4],
expect: true,
},
{
desc: "same forwarding rule, one uses AllPorts",
oldFwdRule: fwdRules[2],
newFwdRule: fwdRules[5],
expect: false,
},
} {
t.Run(tc.desc, func(t *testing.T) {
got := forwardingRulesEqual(tc.oldFwdRule, tc.newFwdRule)
Expand Down Expand Up @@ -1723,3 +1738,86 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) {
}
assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
}

func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) {
t.Parallel()

vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
require.NoError(t, err)
nodeNames := []string{"test-node-1"}
nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
require.NoError(t, err)
svc := fakeLoadbalancerService(string(LBTypeInternal))
svc, err = gce.client.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
require.NoError(t, err)
lbName := gce.GetLoadBalancerName(context.TODO(), "", svc)
status, err := createInternalLoadBalancer(gce, svc, nil, nodeNames, vals.ClusterName, vals.ClusterID, vals.ZoneName)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
assert.NotEmpty(t, status.Ingress)
fwdRule, err := gce.GetRegionForwardingRule(lbName, gce.region)
if err != nil {
t.Errorf("gce.GetRegionForwardingRule(%q, %q) = %v, want nil", lbName, gce.region, err)
}
if fwdRule.Ports[0] != "123" {
t.Errorf("Unexpected port value %v, expected [123]", fwdRule.Ports)
}

// Change service spec to use more than 5 ports
svc.Spec.Ports = []v1.ServicePort{
{Name: "testport", Port: int32(8080), Protocol: "TCP"},
{Name: "testport", Port: int32(8090), Protocol: "TCP"},
{Name: "testport", Port: int32(8100), Protocol: "TCP"},
{Name: "testport", Port: int32(8200), Protocol: "TCP"},
{Name: "testport", Port: int32(8300), Protocol: "TCP"},
{Name: "testport", Port: int32(8400), Protocol: "TCP"},
}
status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, svc, nodes)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
assert.NotEmpty(t, status.Ingress)
fwdRule, err = gce.GetRegionForwardingRule(lbName, gce.region)
if err != nil {
t.Errorf("gce.GetRegionForwardingRule(%q, %q) = %v, want nil", lbName, gce.region, err)
}
if !fwdRule.AllPorts {
t.Errorf("Unexpected AllPorts false value, expected true, FR - %v", fwdRule)
}
if len(fwdRule.Ports) != 0 {
t.Errorf("Unexpected port value %v, expected empty list", fwdRule.Ports)
}

// Change service spec back to use < 5 ports
svc.Spec.Ports = []v1.ServicePort{
{Name: "testport", Port: int32(8090), Protocol: "TCP"},
{Name: "testport", Port: int32(8100), Protocol: "TCP"},
{Name: "testport", Port: int32(8300), Protocol: "TCP"},
{Name: "testport", Port: int32(8400), Protocol: "TCP"},
}
expectPorts := []string{"8090", "8100", "8300", "8400"}
status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, svc, nodes)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
assert.NotEmpty(t, status.Ingress)
fwdRule, err = gce.GetRegionForwardingRule(lbName, gce.region)
if err != nil {
t.Errorf("gce.GetRegionForwardingRule(%q, %q) = %v, want nil", lbName, gce.region, err)
}
if fwdRule.AllPorts {
t.Errorf("Unexpected AllPorts true value, expected false, FR - %v", fwdRule)
}
if !equalStringSets(fwdRule.Ports, expectPorts) {
t.Errorf("Unexpected port value %v, expected %v", fwdRule.Ports, expectPorts)
}

// Delete the service
err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, svc)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
assertInternalLbResourcesDeleted(t, gce, svc, vals, true)
}

0 comments on commit 55fcfd7

Please sign in to comment.