diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/BUILD b/staging/src/k8s.io/legacy-cloud-providers/gce/BUILD index 5da7a3d6307..d48e4a4b9c2 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/BUILD @@ -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", diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go index cf35e1a3426..cb141bd8b05 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go @@ -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 } diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go index d36b83e1439..9d50ea1a074 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go @@ -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)