Merge pull request #93423 from prameshj/ilb-fix

Fix GCE ILB code to support protocol change
This commit is contained in:
Kubernetes Prow Robot 2020-09-02 02:37:21 -07:00 committed by GitHub
commit eaa0fe2381
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 85 additions and 9 deletions

View File

@ -173,12 +173,6 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
return nil, err
}
bsDescription := makeBackendServiceDescription(nm, sharedBackend)
err = g.ensureInternalBackendService(backendServiceName, bsDescription, svc.Spec.SessionAffinity, scheme, protocol, igLinks, hc.SelfLink)
if err != nil {
return nil, err
}
fwdRuleDescription := &forwardingRuleDescription{ServiceName: nm.String()}
fwdRuleDescriptionString, err := fwdRuleDescription.marshal()
if err != nil {
@ -200,10 +194,31 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
if options.AllowGlobalAccess {
newFwdRule.AllowGlobalAccess = options.AllowGlobalAccess
}
if err := g.ensureInternalForwardingRule(existingFwdRule, newFwdRule); err != nil {
fwdRuleDeleted := false
if existingFwdRule != nil && !forwardingRulesEqual(existingFwdRule, newFwdRule) {
// Delete existing forwarding rule before making changes to the backend service. For example - changing protocol
// of backend service without first deleting forwarding rule will throw an error since the linked forwarding
// rule would show the old protocol.
klog.V(2).Infof("ensureInternalLoadBalancer(%v): deleting existing forwarding rule with IP address %v", loadBalancerName, existingFwdRule.IPAddress)
if err = ignoreNotFound(g.DeleteRegionForwardingRule(loadBalancerName, g.region)); err != nil {
return nil, err
}
fwdRuleDeleted = true
}
bsDescription := makeBackendServiceDescription(nm, sharedBackend)
err = g.ensureInternalBackendService(backendServiceName, bsDescription, svc.Spec.SessionAffinity, scheme, protocol, igLinks, hc.SelfLink)
if err != nil {
return nil, err
}
if fwdRuleDeleted || existingFwdRule == nil {
if err := g.ensureInternalForwardingRule(existingFwdRule, newFwdRule); err != nil {
return nil, err
}
}
// Delete the previous internal load balancer resources if necessary
if existingBackendService != nil {
g.clearPreviousInternalResources(svc, loadBalancerName, existingBackendService, backendServiceName, hcName)

View File

@ -32,11 +32,11 @@ import (
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock"
"google.golang.org/api/compute/v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"k8s.io/cloud-provider"
cloudprovider "k8s.io/cloud-provider"
servicehelper "k8s.io/cloud-provider/service/helpers"
)
@ -1638,3 +1638,64 @@ func TestEnsureLoadBalancerPartialDelete(t *testing.T) {
require.NoError(t, err)
assert.False(t, exists)
}
func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) {
t.Parallel()
vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
require.NoError(t, err)
c := gce.c.(*cloud.MockGCE)
c.MockRegionBackendServices.UpdateHook = func(ctx context.Context, key *meta.Key, be *compute.BackendService, m *cloud.MockRegionBackendServices) error {
// Same key can be used since FR will have the same name.
fr, err := c.MockForwardingRules.Get(ctx, key)
if err != nil && !isNotFound(err) {
return err
}
if fr != nil && fr.IPProtocol != be.Protocol {
return fmt.Errorf("Protocol mismatch between Forwarding Rule value %q and Backend service value %q", fr.IPProtocol, be.Protocol)
}
return mock.UpdateRegionBackendServiceHook(ctx, key, be, m)
}
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.IPProtocol != "TCP" {
t.Errorf("Unexpected protocol value %s, expected TCP", fwdRule.IPProtocol)
}
// change the protocol to UDP
svc.Spec.Ports[0].Protocol = v1.ProtocolUDP
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.IPProtocol != "UDP" {
t.Errorf("Unexpected protocol value %s, expected UDP", fwdRule.IPProtocol)
}
// 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)
}