Merge pull request #99595 from prameshj/ports-fix

Support specifying more than 5 ports in L4 ILB service
This commit is contained in:
Kubernetes Prow Robot 2021-03-08 01:21:45 -08:00 committed by GitHub
commit 763514f438
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 111 additions and 2 deletions

View File

@ -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) {
@ -187,6 +189,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) {
@ -969,13 +975,18 @@ func (g *Cloud) ensureInternalForwardingRule(existingFwdRule, newFwdRule *comput
func forwardingRulesEqual(old, new *compute.ForwardingRule) bool {
// basepath could have differences like compute.googleapis.com vs www.googleapis.com, compare resourceIDs
oldResourceID, err := cloud.ParseResourceURL(old.BackendService)
klog.Errorf("forwardingRulesEqual(): failed to parse backend resource URL from existing FR, err - %v", err)
if err != nil {
klog.Errorf("forwardingRulesEqual(): failed to parse backend resource URL from existing FR, err - %v", err)
}
newResourceID, err := cloud.ParseResourceURL(new.BackendService)
klog.Errorf("forwardingRulesEqual(): failed to parse resource URL from new FR, err - %v", err)
if err != nil {
klog.Errorf("forwardingRulesEqual(): failed to parse resource URL from new FR, err - %v", err)
}
return (old.IPAddress == "" || new.IPAddress == "" || old.IPAddress == new.IPAddress) &&
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

View File

@ -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 {
@ -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)
@ -1722,3 +1737,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)
}