diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index d5f287e28e3..9aa599dca6a 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/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,44 @@ 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] + + tags := sets.NewString() + 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 { + tags.Insert(longest_tag) + } else if len(tags) > 0 { + return nil, fmt.Errorf("Some, but not all, instances have prefix tags (%s is missing)", instance.Name) + } + } + + if len(tags) == 0 { + glog.V(2).Info("No instances had tags, creating rule without target tags") + } + + return tags.List(), nil } // UpdateTCPLoadBalancer is an implementation of TCPLoadBalancer.UpdateTCPLoadBalancer. diff --git a/pkg/cloudprovider/providers/gce/gce_test.go b/pkg/cloudprovider/providers/gce/gce_test.go index 3f916848135..34ff3b3382e 100644 --- a/pkg/cloudprovider/providers/gce/gce_test.go +++ b/pkg/cloudprovider/providers/gce/gce_test.go @@ -37,34 +37,6 @@ func TestGetRegion(t *testing.T) { } } -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", - }, - { - host: "kubernetes-minion-559o.c.PROJECT_NAME.internal", - expected: "kubernetes-minion", - }, - } - - 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