Updated GCE FW target tag logic to use the set of longest tag prefixes from all instances

pkg/cloudprovider/providers/gce/gce.go

@@ -423,14 +423,17 @@ func (gce *GCECloud) EnsureTCPLoadBalancer(name, region string, loadBalancerIP n
 		allowedPorts[ix] = strconv.Itoa(ports[ix].Port)
 	}
 
-	hostTag := gce.computeHostTag(hosts[0])
+	hostTags, err := gce.computeHostTags(hosts)
+	if err != nil {
+		return nil, err
+	}
 
 	firewall := &compute.Firewall{
 		Name:         makeFirewallName(name),
 		Description:  fmt.Sprintf("KubernetesAutoGenerated_OnlyAllowTrafficForDestinationIP_%s", fwd.IPAddress),
 		Network:      gce.networkURL,
 		SourceRanges: []string{"0.0.0.0/0"},
-		TargetTags:   []string{hostTag},
+		TargetTags:   hostTags,
 		Allowed: []*compute.FirewallAllowed{
 			{
 				IPProtocol: "tcp",
@@ -450,16 +453,52 @@ func (gce *GCECloud) EnsureTCPLoadBalancer(name, region string, loadBalancerIP n
 	return status, nil
 }
 
-// This is kind of hacky, but the managed instance group adds 4 random chars and a hyphen
-// to the base name. Older naming schemes put a hyphen and an incrementing index after
-// the base name. Thus we pull off the characters after the final dash to support both.
-func (gce *GCECloud) computeHostTag(host string) string {
-	host = strings.SplitN(host, ".", 2)[0]
-	lastHyphen := strings.LastIndex(host, "-")
-	if lastHyphen == -1 {
-		return host
+// We grab all tags from all instances being added to the pool.  
+// * The longest tag that is a prefix of the instance name is used
+// * If any instance has a prefix tag, all instances must
+// * If no instances have a prefix tag, no tags are used
+func (gce *GCECloud) computeHostTags(hosts []string) ([]string, error) {
+	listCall := gce.service.Instances.List(gce.projectID, gce.zone)
+
+    // Add the filter for hosts
+    listCall = listCall.Filter("name eq (" + strings.Join(hosts, "|") + ")")
+
+    // Add the fields we want
+    listCall = listCall.Fields("items(name,tags)")
+
+	res, err := listCall.Do()
+	if err != nil {
+		return nil, err
 	}
-	return host[:lastHyphen]
+
+    // Set symantics
+    // * This will be faster for large numbers of hosts (over iterating for "contains")
+    set := make(map[string]bool)
+    for _, instance := range res.Items {
+        longest_tag := ""
+        for  _, tag := range instance.Tags.Items {
+            if strings.HasPrefix(instance.Name, tag) && len(tag) > len(longest_tag) {
+                longest_tag = tag
+            }
+        }
+        if len(longest_tag) > 0 {
+           set[longest_tag] = true
+        } else if len(set) > 0 {
+		    return nil, fmt.Errorf("Some, but not all, instances have prefix tags (%s is missing)", instance.Name)
+        }
+    }
+
+    // Get the values
+    tags := make([]string, 0, len(set))
+    for tag := range set {
+        tags = append(tags, tag)
+    }
+
+    if len(tags) == 0 {
+        glog.V(2).Info("No instances had tags, creating rule without target tags")
+    }
+
+	return tags, nil
 }
 
 // UpdateTCPLoadBalancer is an implementation of TCPLoadBalancer.UpdateTCPLoadBalancer.