From d07dc0c10e99e4ae44ca4db391f9760dae17d3d5 Mon Sep 17 00:00:00 2001
From: Alex Robinson <arob@google.com>
Date: Wed, 15 Jul 2015 18:58:54 +0000
Subject: [PATCH] Fix issue of comparing instance URLs with different project
 ID representations in GCE target pools.

---
 pkg/cloudprovider/gce/gce.go      | 24 +++++++--
 pkg/cloudprovider/gce/gce_test.go | 86 +++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go
index e18b4ca5902..796d53e6b00 100644
--- a/pkg/cloudprovider/gce/gce.go
+++ b/pkg/cloudprovider/gce/gce.go
@@ -219,12 +219,25 @@ func (gce *GCECloud) Routes() (cloudprovider.Routes, bool) {
 	return gce, true
 }
 
-func makeHostLink(projectID, zone, host string) string {
+func makeHostURL(projectID, zone, host string) string {
 	host = canonicalizeInstanceName(host)
 	return fmt.Sprintf("https://www.googleapis.com/compute/v1/projects/%s/zones/%s/instances/%s",
 		projectID, zone, host)
 }
 
+func makeComparableHostPath(zone, host string) string {
+	host = canonicalizeInstanceName(host)
+	return fmt.Sprintf("/zones/%s/instances/%s", zone, host)
+}
+
+func hostURLToComparablePath(hostURL string) string {
+	idx := strings.Index(hostURL, "/zones/")
+	if idx < 0 {
+		return ""
+	}
+	return hostURL[idx:]
+}
+
 // Session Affinity Type string
 type GCEAffinityType string
 
@@ -240,7 +253,7 @@ const (
 func (gce *GCECloud) makeTargetPool(name, region string, hosts []string, affinityType GCEAffinityType) error {
 	var instances []string
 	for _, host := range hosts {
-		instances = append(instances, makeHostLink(gce.projectID, gce.zone, host))
+		instances = append(instances, makeHostURL(gce.projectID, gce.zone, host))
 	}
 	pool := &compute.TargetPool{
 		Name:            name,
@@ -390,12 +403,15 @@ func (gce *GCECloud) UpdateTCPLoadBalancer(name, region string, hosts []string)
 	if err != nil {
 		return err
 	}
-	existing := util.NewStringSet(pool.Instances...)
+	existing := util.NewStringSet()
+	for _, instance := range pool.Instances {
+		existing.Insert(hostURLToComparablePath(instance))
+	}
 
 	var toAdd []*compute.InstanceReference
 	var toRemove []*compute.InstanceReference
 	for _, host := range hosts {
-		link := makeHostLink(gce.projectID, gce.zone, host)
+		link := makeComparableHostPath(gce.zone, host)
 		if !existing.Has(link) {
 			toAdd = append(toAdd, &compute.InstanceReference{link})
 		}
diff --git a/pkg/cloudprovider/gce/gce_test.go b/pkg/cloudprovider/gce/gce_test.go
index 798175d9a0b..bc2ae1a4a5c 100644
--- a/pkg/cloudprovider/gce/gce_test.go
+++ b/pkg/cloudprovider/gce/gce_test.go
@@ -36,3 +36,89 @@ func TestGetRegion(t *testing.T) {
 		t.Errorf("Unexpected region: %s", zone.Region)
 	}
 }
+
+func TestGetHostTag(t *testing.T) {
+	tests := []struct {
+		host     string
+		expected string
+	}{
+		{
+			host:     "kubernetes-minion-559o",
+			expected: "kubernetes-minion",
+		},
+		{
+			host:     "gke-test-ea6e8c80-node-8ytk",
+			expected: "gke-test-ea6e8c80-node",
+		},
+	}
+
+	gce := &GCECloud{}
+	for _, test := range tests {
+		hostTag := gce.computeHostTag(test.host)
+		if hostTag != test.expected {
+			t.Errorf("expected: %s, saw: %s for %s", test.expected, hostTag, test.host)
+		}
+	}
+}
+
+func TestComparingHostURLs(t *testing.T) {
+	tests := []struct {
+		host1       string
+		zone        string
+		name        string
+		expectEqual bool
+	}{
+		{
+			host1:       "https://www.googleapis.com/compute/v1/projects/1234567/zones/us-central1-f/instances/kubernetes-node-fhx1",
+			zone:        "us-central1-f",
+			name:        "kubernetes-node-fhx1",
+			expectEqual: true,
+		},
+		{
+			host1:       "https://www.googleapis.com/compute/v1/projects/cool-project/zones/us-central1-f/instances/kubernetes-node-fhx1",
+			zone:        "us-central1-f",
+			name:        "kubernetes-node-fhx1",
+			expectEqual: true,
+		},
+		{
+			host1:       "https://www.googleapis.com/compute/v23/projects/1234567/zones/us-central1-f/instances/kubernetes-node-fhx1",
+			zone:        "us-central1-f",
+			name:        "kubernetes-node-fhx1",
+			expectEqual: true,
+		},
+		{
+			host1:       "https://www.googleapis.com/compute/v24/projects/1234567/regions/us-central1/zones/us-central1-f/instances/kubernetes-node-fhx1",
+			zone:        "us-central1-f",
+			name:        "kubernetes-node-fhx1",
+			expectEqual: true,
+		},
+		{
+			host1:       "https://www.googleapis.com/compute/v1/projects/1234567/zones/us-central1-f/instances/kubernetes-node-fhx1",
+			zone:        "us-central1-c",
+			name:        "kubernetes-node-fhx1",
+			expectEqual: false,
+		},
+		{
+			host1:       "https://www.googleapis.com/compute/v1/projects/1234567/zones/us-central1-f/instances/kubernetes-node-fhx",
+			zone:        "us-central1-f",
+			name:        "kubernetes-node-fhx1",
+			expectEqual: false,
+		},
+		{
+			host1:       "https://www.googleapis.com/compute/v1/projects/1234567/zones/us-central1-f/instances/kubernetes-node-fhx1",
+			zone:        "us-central1-f",
+			name:        "kubernetes-node-fhx",
+			expectEqual: false,
+		},
+	}
+
+	for _, test := range tests {
+		link1 := hostURLToComparablePath(test.host1)
+		link2 := makeComparableHostPath(test.zone, test.name)
+		if test.expectEqual && link1 != link2 {
+			t.Errorf("expected link1 and link2 to be equal, got %s and %s", link1, link2)
+		} else if !test.expectEqual && link1 == link2 {
+			t.Errorf("expected link1 and link2 not to be equal, got %s and %s", link1, link2)
+		}
+	}
+}