github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-24 04:06:03 +00:00
Code Issues Projects Releases Wiki Activity

Avoid checking the entire backend service URL for FR equality.

Browse Source 
Check only the resource name, since the path could have differences like
"www.googleapis.com" vs "compute.googleapis.com"

Add a log message for what changed in the ILB forwarding rule.

updated BUILD

Follow up comments and unit tests
This commit is contained in:
Pavithra Ramesh 2020-11-11 16:40:47 -08:00
parent 5cfce4e5cb
commit a65675f1b7
3 changed files with 33 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
staging/src/k8s.io/legacy-cloud-providers/gce/BUILD
View File

@ -75,6 +75,7 @@ go_library(
"//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/filter:go_default_library",
"//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta:go_default_library",
"//vendor/github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock:go_default_library",
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
"//vendor/golang.org/x/oauth2:go_default_library",
"//vendor/golang.org/x/oauth2/google:go_default_library",
"//vendor/google.golang.org/api/compute/v0.alpha:go_default_library",

14
staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go
View File

@ -28,6 +28,7 @@ import (
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"github.com/google/go-cmp/cmp"
compute "google.golang.org/api/compute/v1"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
@ -200,7 +201,8 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
// 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)
frDiff := cmp.Diff(existingFwdRule, newFwdRule)
klog.V(2).Infof("ensureInternalLoadBalancer(%v): forwarding rule changed - Existing - %+v\n, New - %+v\n, Diff(-existing, +new) - %s\n. Deleting existing forwarding rule.", loadBalancerName, existingFwdRule, newFwdRule, frDiff)
if err = ignoreNotFound(g.DeleteRegionForwardingRule(loadBalancerName, g.region)); err != nil {
return nil, err
}
@ -214,7 +216,8 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
}
if fwdRuleDeleted || existingFwdRule == nil {
if err := g.ensureInternalForwardingRule(existingFwdRule, newFwdRule); err != nil {
// existing rule has been deleted, pass in nil
if err := g.ensureInternalForwardingRule(nil, newFwdRule); err != nil {
return nil, err
}
}
@ -979,11 +982,16 @@ 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)
newResourceID, err := cloud.ParseResourceURL(new.BackendService)
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.BackendService == new.BackendService &&
oldResourceID.Equal(newResourceID) &&
old.AllowGlobalAccess == new.AllowGlobalAccess &&
old.Subnetwork == new.Subnetwork
}

23
staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go
View File

@ -32,7 +32,7 @@ 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"
v1 "k8s.io/api/core/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"
@ -328,7 +328,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) {
}
gce.CreateRegionBackendService(existingBS, gce.region)
existingFwdRule.BackendService = existingBS.Name
existingFwdRule.BackendService = cloud.SelfLink(meta.VersionGA, vals.ProjectID, "backendServices", meta.RegionalKey(existingBS.Name, gce.region))
_, err = createInternalLoadBalancer(gce, svc, existingFwdRule, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName)
assert.NoError(t, err)
@ -1310,6 +1310,7 @@ func TestForwardingRulesEqual(t *testing.T) {
Ports: []string{"123"},
IPProtocol: "TCP",
LoadBalancingScheme: string(cloud.SchemeInternal),
BackendService: "http://www.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
{
Name: "tcp-fwd-rule",
@ -1317,6 +1318,7 @@ func TestForwardingRulesEqual(t *testing.T) {
Ports: []string{"123"},
IPProtocol: "TCP",
LoadBalancingScheme: string(cloud.SchemeInternal),
BackendService: "http://www.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
{
Name: "udp-fwd-rule",
@ -1324,6 +1326,7 @@ func TestForwardingRulesEqual(t *testing.T) {
Ports: []string{"123"},
IPProtocol: "UDP",
LoadBalancingScheme: string(cloud.SchemeInternal),
BackendService: "http://www.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
{
Name: "global-access-fwd-rule",
@ -1332,6 +1335,16 @@ func TestForwardingRulesEqual(t *testing.T) {
IPProtocol: "TCP",
LoadBalancingScheme: string(cloud.SchemeInternal),
AllowGlobalAccess: true,
BackendService: "http://www.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
{
Name: "global-access-fwd-rule",
IPAddress: "10.0.0.0",
Ports: []string{"123"},
IPProtocol: "TCP",
LoadBalancingScheme: string(cloud.SchemeInternal),
AllowGlobalAccess: true,
BackendService: "http://compute.googleapis.com/projects/test/regions/us-central1/backendServices/bs1",
},
}
@ -1365,6 +1378,12 @@ func TestForwardingRulesEqual(t *testing.T) {
newFwdRule: fwdRules[3],
expect: true,
},
{
desc: "same forwarding rule, different basepath",
oldFwdRule: fwdRules[3],
newFwdRule: fwdRules[4],
expect: true,
},
} {
t.Run(tc.desc, func(t *testing.T) {
got := forwardingRulesEqual(tc.oldFwdRule, tc.newFwdRule)