Merge pull request #96491 from prameshj/ilblog

Check Backend service by name instead of path in GCE LB code.

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",

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"
@@ -186,7 +187,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
 		}
@@ -200,7 +202,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
 		}
 	}
@@ -965,11 +968,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
 }

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)