Skip to content

Commit

Permalink
try to fix the in_tree_volumes cases: refactor the projectBasePath logic
Browse files Browse the repository at this point in the history
  • Loading branch information
pacoxu committed Jul 15, 2021
1 parent 7a36a5b commit 2d3323d
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 45 deletions.
35 changes: 13 additions & 22 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ type Cloud struct {
s *cloud.Service

metricsCollector loadbalancerMetricsCollector

// the compute API endpoint with the `projects/` element.
projectsBasePath string
}

// ConfigGlobal is the in memory representation of the gce.conf config data
Expand Down Expand Up @@ -431,14 +434,10 @@ func CreateGCECloud(config *CloudConfig) (*Cloud, error) {
}
serviceAlpha.UserAgent = userAgent

// Expect override api endpoint to always be v1 api and follows the same pattern as prod.
// Generate alpha and beta api endpoints based on override v1 api endpoint.
// For example,
// staging API endpoint: https://www.googleapis.com/compute/staging_v1/
if config.APIEndpoint != "" {
service.BasePath = fmt.Sprintf("%sprojects/", config.APIEndpoint)
serviceBeta.BasePath = fmt.Sprintf("%sprojects/", strings.Replace(config.APIEndpoint, "v1", "beta", -1))
serviceAlpha.BasePath = fmt.Sprintf("%sprojects/", strings.Replace(config.APIEndpoint, "v1", "alpha", -1))
service.BasePath = config.APIEndpoint
serviceBeta.BasePath = strings.Replace(config.APIEndpoint, "v1", "beta", -1)
serviceAlpha.BasePath = strings.Replace(config.APIEndpoint, "v1", "alpha", -1)
}

containerService, err := container.NewService(context.Background(), option.WithTokenSource(config.TokenSource))
Expand Down Expand Up @@ -524,6 +523,7 @@ func CreateGCECloud(config *CloudConfig) (*Cloud, error) {
AlphaFeatureGate: config.AlphaFeatureGate,
nodeZones: map[string]sets.String{},
metricsCollector: newLoadBalancerMetrics(),
projectsBasePath: getProjectsBasePath(service.BasePath),
}

gce.manager = &gceServiceManager{gce}
Expand Down Expand Up @@ -793,23 +793,14 @@ func (g *Cloud) HasClusterID() bool {
return true
}

// GetBasePath returns the compute API endpoint with the `projects/` element
// compute API v0.36 changed basepath and dropped the `projects/` suffix, therefore suffix
// must be added back when generating compute resource urls.
func (g *Cloud) GetBasePath() string {
basePath := g.ComputeServices().GA.BasePath
return GetProjectBasePath(basePath)
}

func GetProjectBasePath(basePath string) string {
// getProjectsBasePath returns the compute API endpoint with the `projects/` element.
// The suffix must be added when generating compute resource urls.
func getProjectsBasePath(basePath string) string {
if !strings.HasSuffix(basePath, "/") {
basePath += "/"
}
// Trim the trailing /, so that split will not consider the last element as empty
elements := strings.Split(strings.TrimSuffix(basePath, "/"), "/")

if elements[len(elements)-1] != "projects" {
return fmt.Sprintf("%sprojects/", basePath)
if !strings.HasSuffix(basePath, "/projects/") {
basePath += "projects/"
}
return basePath
}
Expand Down Expand Up @@ -957,7 +948,7 @@ func newOauthClient(tokenSource oauth2.TokenSource) (*http.Client, error) {
func (manager *gceServiceManager) getProjectsAPIEndpoint() string {
projectsAPIEndpoint := gceComputeAPIEndpoint + "projects/"
if manager.gce.service != nil {
projectsAPIEndpoint = manager.gce.service.BasePath
projectsAPIEndpoint = manager.gce.projectsBasePath
}

return projectsAPIEndpoint
Expand Down
1 change: 1 addition & 0 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func NewFakeGCECloud(vals TestClusterValues) *Cloud {
ClusterID: fakeClusterID(vals.ClusterID),
onXPN: vals.OnXPN,
metricsCollector: newLoadBalancerMetrics(),
projectsBasePath: getProjectsBasePath(service.BasePath),
}
c := cloud.NewMockGCE(&gceProjectRouter{gce})
gce.c = c
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func makeHostURL(projectsAPIEndpoint, projectID, zone, host string) string {
// ToInstanceReferences returns instance references by links
func (g *Cloud) ToInstanceReferences(zone string, instanceNames []string) (refs []*compute.InstanceReference) {
for _, ins := range instanceNames {
instanceLink := makeHostURL(g.GetBasePath(), g.projectID, zone, ins)
instanceLink := makeHostURL(g.projectsBasePath, g.projectID, zone, ins)
refs = append(refs, &compute.InstanceReference{Instance: instanceLink})
}
return refs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ func (g *Cloud) updateTargetPool(loadBalancerName string, hosts []*gceInstance)
}

func (g *Cloud) targetPoolURL(name string) string {
return g.GetBasePath() + strings.Join([]string{g.projectID, "regions", g.region, "targetPools", name}, "/")
return g.projectsBasePath + strings.Join([]string{g.projectID, "regions", g.region, "targetPools", name}, "/")
}

func makeHTTPHealthCheck(name, path string, port int32) *compute.HttpHealthCheck {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ func getPortRanges(ports []int) (ranges []string) {
}

func (g *Cloud) getBackendServiceLink(name string) string {
return g.service.BasePath + strings.Join([]string{g.projectID, "regions", g.region, "backendServices", name}, "/")
return g.projectsBasePath + strings.Join([]string{g.projectID, "regions", g.region, "backendServices", name}, "/")
}

func getNameFromLink(link string) string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,16 @@ func TestEnsureLoadBalancerDeletedDeletesInternalLb(t *testing.T) {
assertInternalLbResourcesDeleted(t, gce, apiService, vals, true)
}

func TestBasePath(t *testing.T) {
func TestProjectsBasePath(t *testing.T) {
t.Parallel()
vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
// Loadbalancer controller code expects basepath to contain the projects string.
expectBasePath := "https://compute.googleapis.com/compute/v1/projects/"
expectProjectsBasePath := "https://compute.googleapis.com/compute/v1/projects/"
// See https://github.com/kubernetes/kubernetes/issues/102757, the endpoint can have mtls in some cases.
expectMtlsBasePath := "https://compute.mtls.googleapis.com/compute/v1/projects/"
expectMtlsProjectsBasePath := "https://compute.mtls.googleapis.com/compute/v1/projects/"
require.NoError(t, err)
if gce.GetBasePath() != expectBasePath && gce.GetBasePath() != expectMtlsBasePath {
t.Errorf("Compute basePath has changed. Got %q, want %q or %q", gce.GetBasePath(), expectBasePath, expectMtlsBasePath)
if gce.projectsBasePath != expectProjectsBasePath && gce.projectsBasePath != expectMtlsProjectsBasePath {
t.Errorf("Compute projectsBasePath has changed. Got %q, want %q or %q", gce.projectsBasePath, expectProjectsBasePath, expectMtlsProjectsBasePath)
}
}
28 changes: 14 additions & 14 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,37 +603,37 @@ func TestLastComponent(t *testing.T) {
}
}

func TestGetProjectBasePath(t *testing.T) {
func TestGetProjectsBasePath(t *testing.T) {
testCases := []struct {
basePath string
expectProjectBasePath string
basePath string
expectProjectsBasePath string
}{
// basepath ends in `/projects/`
{
basePath: "path/to/api/projects/",
expectProjectBasePath: "path/to/api/projects/",
basePath: "path/to/api/projects/",
expectProjectsBasePath: "path/to/api/projects/",
},
// basepath ends in `/projects`, without trailing /
{
basePath: "path/to/api/projects",
expectProjectBasePath: "path/to/api/projects/",
basePath: "path/to/api/projects",
expectProjectsBasePath: "path/to/api/projects/",
},
// basepath does not end in `/projects/`
{
basePath: "path/to/api/",
expectProjectBasePath: "path/to/api/projects/",
basePath: "path/to/api/",
expectProjectsBasePath: "path/to/api/projects/",
},
// basepath does not end in `/projects/` and does not have trailing /
{
basePath: "path/to/api",
expectProjectBasePath: "path/to/api/projects/",
basePath: "path/to/api",
expectProjectsBasePath: "path/to/api/projects/",
},
}

for _, tc := range testCases {
projectBasePath := GetProjectBasePath(tc.basePath)
if projectBasePath != tc.expectProjectBasePath {
t.Errorf("Expected project base path %s; but got %s", tc.expectProjectBasePath, projectBasePath)
projectsBasePath := getProjectsBasePath(tc.basePath)
if projectsBasePath != tc.expectProjectsBasePath {
t.Errorf("Expected projects base path %s; but got %s", tc.expectProjectsBasePath, projectsBasePath)
}
}
}
2 changes: 1 addition & 1 deletion staging/src/k8s.io/legacy-cloud-providers/gce/gce_zones.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,5 @@ func (g *Cloud) ListZonesInRegion(region string) ([]*compute.Zone, error) {
}

func (g *Cloud) getRegionLink(region string) string {
return g.GetBasePath() + strings.Join([]string{g.projectID, "regions", region}, "/")
return g.projectsBasePath + strings.Join([]string{g.projectID, "regions", region}, "/")
}

0 comments on commit 2d3323d

Please sign in to comment.