diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index 6e817dae6ce..1c77a4f79a4 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -114,6 +114,7 @@ type GCECloud struct { localZone string // The zone in which we are running managedZones []string // List of zones we are spanning (for multi-AZ clusters, primarily when running on master) networkURL string + isLegacyNetwork bool subnetworkURL string secondaryRangeName string networkProjectID string @@ -397,31 +398,49 @@ func CreateGCECloud(config *CloudConfig) (*GCECloud, error) { // ProjectID and.NetworkProjectID may be project number or name. projID, netProjID := tryConvertToProjectNames(config.ProjectID, config.NetworkProjectID, service) - onXPN := projID != netProjID var networkURL string var subnetURL string + var isLegacyNetwork bool - if config.NetworkName == "" && config.NetworkURL == "" { - // TODO: Stop using this call and return an error. - // This function returns the first network in a list of networks for a project. The project - // should be set via configuration instead of randomly taking the first. - networkName, err := getNetworkNameViaAPICall(service, config.NetworkProjectID) - if err != nil { - return nil, err - } - networkURL = gceNetworkURL(config.ApiEndpoint, netProjID, networkName) - } else if config.NetworkURL != "" { + if config.NetworkURL != "" { networkURL = config.NetworkURL - } else { + } else if config.NetworkName != "" { networkURL = gceNetworkURL(config.ApiEndpoint, netProjID, config.NetworkName) + } else { + // Other consumers may use the cloudprovider without utilizing the wrapped GCE API functions + // or functions requiring network/subnetwork URLs (e.g. Kubelet). + glog.Warningf("No network name or URL specified.") } if config.SubnetworkURL != "" { subnetURL = config.SubnetworkURL } else if config.SubnetworkName != "" { subnetURL = gceSubnetworkURL(config.ApiEndpoint, netProjID, config.Region, config.SubnetworkName) + } else { + // Attempt to determine the subnetwork in case it's an automatic network. + // Legacy networks will not have a subnetwork, so subnetworkURL should remain empty. + if networkName := lastComponent(networkURL); networkName != "" { + if n, err := getNetwork(service, netProjID, networkName); err != nil { + // Gracefully fail because kubelet calls CreateGCECloud without any config, and API calls will fail coming from minions. + glog.Warningf("Could not retrieve network %q in attempt to determine if legacy network or see list of subnets, err %v", networkURL, err) + } else { + // Legacy networks have a non-empty IPv4Range + if len(n.IPv4Range) > 0 { + glog.Infof("Determined network %q is type legacy", networkURL) + isLegacyNetwork = true + } else { + // Try to find the subnet in the list of subnets + subnetURL = findSubnetForRegion(n.Subnetworks, config.Region) + if len(subnetURL) > 0 { + glog.Infof("Using subnet %q within network %q & region %q because none was specified.", subnetURL, n.Name, config.Region) + } else { + glog.Warningf("Could not find any subnet in region %q within list %v.", config.Region, n.Subnetworks) + } + } + } + } } if len(config.ManagedZones) == 0 { @@ -449,6 +468,7 @@ func CreateGCECloud(config *CloudConfig) (*GCECloud, error) { localZone: config.Zone, managedZones: config.ManagedZones, networkURL: networkURL, + isLegacyNetwork: isLegacyNetwork, subnetworkURL: subnetURL, secondaryRangeName: config.SecondaryRangeName, nodeTags: config.NodeTags, @@ -572,6 +592,10 @@ func (gce *GCECloud) SubnetworkURL() string { return gce.subnetworkURL } +func (gce *GCECloud) IsLegacyNetwork() bool { + return gce.isLegacyNetwork +} + // Known-useless DNS search path. var uselessDNSSearchRE = regexp.MustCompile(`^[0-9]+.google.internal.$`) @@ -615,7 +639,7 @@ func gceSubnetworkURL(apiEndpoint, project, region, subnetwork string) string { return apiEndpoint + strings.Join([]string{"projects", project, "regions", region, "subnetworks", subnetwork}, "/") } -// getProjectIDInURL parses typical full resource URLS and shorter URLS +// getProjectIDInURL parses full resource URLS and shorter URLS // https://www.googleapis.com/compute/v1/projects/myproject/global/networks/mycustom // projects/myproject/global/networks/mycustom // All return "myproject" @@ -629,6 +653,20 @@ func getProjectIDInURL(urlStr string) (string, error) { return "", fmt.Errorf("could not find project field in url: %v", urlStr) } +// getRegionInURL parses full resource URLS and shorter URLS +// https://www.googleapis.com/compute/v1/projects/myproject/regions/us-central1/subnetworks/a +// projects/myproject/regions/us-central1/subnetworks/a +// All return "us-central1" +func getRegionInURL(urlStr string) string { + fields := strings.Split(urlStr, "/") + for i, v := range fields { + if v == "regions" && i < len(fields)-1 { + return fields[i+1] + } + } + return "" +} + func getNetworkNameViaMetadata() (string, error) { result, err := metadata.Get("instance/network-interfaces/0/network") if err != nil { @@ -641,18 +679,9 @@ func getNetworkNameViaMetadata() (string, error) { return parts[3], nil } -func getNetworkNameViaAPICall(svc *compute.Service, projectID string) (string, error) { - // TODO: use PageToken to list all not just the first 500 - networkList, err := svc.Networks.List(projectID).Do() - if err != nil { - return "", err - } - - if networkList == nil || len(networkList.Items) <= 0 { - return "", fmt.Errorf("GCE Network List call returned no networks for project %q", projectID) - } - - return networkList.Items[0].Name, nil +// getNetwork returns a GCP network +func getNetwork(svc *compute.Service, networkProjectID, networkID string) (*compute.Network, error) { + return svc.Networks.Get(networkProjectID, networkID).Do() } // getProjectID returns the project's string ID given a project number or string @@ -687,6 +716,15 @@ func getZonesForRegion(svc *compute.Service, projectID, region string) ([]string return zones, nil } +func findSubnetForRegion(subnetURLs []string, region string) string { + for _, url := range subnetURLs { + if thisRegion := getRegionInURL(url); thisRegion == region { + return url + } + } + return "" +} + func newOauthClient(tokenSource oauth2.TokenSource) (*http.Client, error) { if tokenSource == nil { var err error diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go index 31d770c849a..5a5f72bf6a5 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go +++ b/pkg/cloudprovider/providers/gce/gce_loadbalancer_internal.go @@ -80,12 +80,18 @@ func (gce *GCECloud) ensureInternalLoadBalancer(clusterName, clusterID string, s // Determine IP which will be used for this LB. If no forwarding rule has been established // or specified in the Service spec, then requestedIP = "". requestedIP := determineRequestedIP(svc, existingFwdRule) - addrMgr := newAddressManager(gce, nm.String(), gce.Region(), gce.getInternalSubnetURL(), loadBalancerName, requestedIP, schemeInternal) - ipToUse, err := addrMgr.HoldAddress() - if err != nil { - return nil, err + ipToUse := requestedIP + + var addrMgr *addressManager + // If the network is not a legacy network, use the address manager + if !gce.IsLegacyNetwork() { + addrMgr = newAddressManager(gce, nm.String(), gce.Region(), gce.SubnetworkURL(), loadBalancerName, requestedIP, schemeInternal) + ipToUse, err = addrMgr.HoldAddress() + if err != nil { + return nil, err + } + glog.V(2).Infof("ensureInternalLoadBalancer(%v): reserved IP %q for the forwarding rule", loadBalancerName, ipToUse) } - glog.V(2).Infof("ensureInternalLoadBalancer(%v): reserved IP %q for the forwarding rule", loadBalancerName, ipToUse) // Ensure firewall rules if necessary if err = gce.ensureInternalFirewalls(loadBalancerName, ipToUse, clusterID, nm, svc, strconv.Itoa(int(hcPort)), sharedHealthCheck, nodes); err != nil { @@ -102,7 +108,7 @@ func (gce *GCECloud) ensureInternalLoadBalancer(clusterName, clusterID string, s LoadBalancingScheme: string(scheme), } - // Specify subnetwork if network type is manual + // Specify subnetwork if known if len(gce.subnetworkURL) > 0 { expectedFwdRule.Subnetwork = gce.subnetworkURL } else { @@ -138,13 +144,21 @@ func (gce *GCECloud) ensureInternalLoadBalancer(clusterName, clusterID string, s gce.clearPreviousInternalResources(svc, loadBalancerName, existingBackendService, backendServiceName, hcName) } - // Now that the controller knows the forwarding rule exists, we can release the address. - if err := addrMgr.ReleaseAddress(); err != nil { - glog.Errorf("ensureInternalLoadBalancer: failed to release address reservation, possibly causing an orphan: %v", err) + if addrMgr != nil { + // Now that the controller knows the forwarding rule exists, we can release the address. + if err := addrMgr.ReleaseAddress(); err != nil { + glog.Errorf("ensureInternalLoadBalancer: failed to release address reservation, possibly causing an orphan: %v", err) + } + } + + // Get the most recent forwarding rule for the address. + updatedFwdRule, err := gce.GetRegionForwardingRule(loadBalancerName, gce.region) + if err != nil { + return nil, err } status := &v1.LoadBalancerStatus{} - status.Ingress = []v1.LoadBalancerIngress{{IP: ipToUse}} + status.Ingress = []v1.LoadBalancerIngress{{IP: updatedFwdRule.IPAddress}} return status, nil } @@ -662,20 +676,6 @@ func (gce *GCECloud) getBackendServiceLink(name string) string { return gce.service.BasePath + strings.Join([]string{gce.projectID, "regions", gce.region, "backendServices", name}, "/") } -// getInternalSubnetURL first attempts to return the configured SubnetURL. -// If subnetwork-name was not specified, then a best-effort generation is made. -// Note subnet names might not be the network name for some auto networks. -func (gce *GCECloud) getInternalSubnetURL() string { - if gce.SubnetworkURL() != "" { - return gce.SubnetworkURL() - } - - networkName := getNameFromLink(gce.NetworkURL()) - v := gceSubnetworkURL("", gce.NetworkProjectID(), gce.Region(), networkName) - glog.Warningf("Generating subnetwork URL based off network since subnet name/URL was not configured: %q", v) - return v -} - func getNameFromLink(link string) string { if link == "" { return "" diff --git a/pkg/cloudprovider/providers/gce/gce_test.go b/pkg/cloudprovider/providers/gce/gce_test.go index 14d4e703418..e3020acb7db 100644 --- a/pkg/cloudprovider/providers/gce/gce_test.go +++ b/pkg/cloudprovider/providers/gce/gce_test.go @@ -626,3 +626,63 @@ func TestNewAlphaFeatureGate(t *testing.T) { delete(knownAlphaFeatures, "foo") delete(knownAlphaFeatures, "bar") } + +func TestGetRegionInURL(t *testing.T) { + cases := map[string]string{ + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/a": "us-central1", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-west2/subnetworks/b": "us-west2", + "projects/my-project/regions/asia-central1/subnetworks/c": "asia-central1", + "regions/europe-north2": "europe-north2", + "my-url": "", + "": "", + } + for input, output := range cases { + result := getRegionInURL(input) + if result != output { + t.Errorf("Actual result %q does not match expected result %q for input: %q", result, output, input) + } + } +} + +func TestFindSubnetForRegion(t *testing.T) { + s := []string{ + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/default-38b01f54907a15a7", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-west1/subnetworks/default", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-east1/subnetworks/default-277eec3815f742b6", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-east4/subnetworks/default", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/asia-northeast1/subnetworks/default", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/asia-east1/subnetworks/default-8e020b4b8b244809", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/australia-southeast1/subnetworks/default", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/southamerica-east1/subnetworks/default", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/europe-west3/subnetworks/default", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/asia-southeast1/subnetworks/default", + "", + } + actual := findSubnetForRegion(s, "asia-east1") + expectedResult := "https://www.googleapis.com/compute/v1/projects/my-project/regions/asia-east1/subnetworks/default-8e020b4b8b244809" + if actual != expectedResult { + t.Errorf("Actual result %q does not match expected result %q", actual, expectedResult) + } + + var nilSlice []string + res := findSubnetForRegion(nilSlice, "us-central1") + if res != "" { + t.Errorf("expected an empty result, got %v", res) + } +} + +func TestLastComponent(t *testing.T) { + cases := map[string]string{ + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/a": "a", + "https://www.googleapis.com/compute/v1/projects/my-project/regions/us-central1/subnetworks/b": "b", + "projects/my-project/regions/us-central1/subnetworks/c": "c", + "d": "d", + "": "", + } + for input, output := range cases { + result := lastComponent(input) + if result != output { + t.Errorf("Actual result %q does not match expected result %q for input: %q", result, output, input) + } + } +}