Skip to content

Add compute-endpoint flag to pdcsi driver #1077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/gce-pd-csi-driver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
var (
cloudConfigFilePath = flag.String("cloud-config", "", "Path to GCE cloud provider config")
endpoint = flag.String("endpoint", "unix:/tmp/csi.sock", "CSI endpoint")
computeEndpoint = flag.String("compute-endpoint", "", "If set, used as the endpoint for the GCE API.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For safety, should we always fall back/default to the public GCE endpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no endpoint is passed into the computebeta.NewService constructor, the default "https://compute.googleapis.com/compute/beta/" is used. This value is being used in the current implementation and the new.

[1]: see "basePath" in https://raw.githubusercontent.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/7592dc19d3bd000f34cf950454c3355ae35f1475/vendor/google.golang.org/api/compute/v0.beta/compute-gen.go

runControllerService = flag.Bool("run-controller-service", true, "If set to false then the CSI driver does not activate its controller service (default: true)")
runNodeService = flag.Bool("run-node-service", true, "If set to false then the CSI driver does not activate its node service (default: true)")
httpEndpoint = flag.String("http-endpoint", "", "The TCP network address where the prometheus metrics endpoint will listen (example: `:8080`). The default is empty string, which means metrics endpoint is disabled.")
Expand Down Expand Up @@ -117,7 +118,7 @@ func handle() {
//Initialize requirements for the controller service
var controllerServer *driver.GCEControllerServer
if *runControllerService {
cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath)
cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, *computeEndpoint)
if err != nil {
klog.Fatalf("Failed to get cloud provider: %v", err)
}
Expand Down
32 changes: 19 additions & 13 deletions pkg/gce-cloud-provider/compute/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,6 @@ const (

regionURITemplate = "projects/%s/regions/%s"

GCEComputeAPIEndpoint = "https://www.googleapis.com/compute/v1/"
GCEComputeBetaAPIEndpoint = "https://www.googleapis.com/compute/beta/"
GCEComputeAlphaAPIEndpoint = "https://www.googleapis.com/compute/alpha/"

replicaZoneURITemplateSingleZone = "projects/%s/zones/%s" // {gce.projectID}/zones/{disk.Zone}
)

Expand All @@ -73,7 +69,7 @@ type ConfigGlobal struct {
Zone string `gcfg:"zone"`
}

func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string) (*CloudProvider, error) {
func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint string) (*CloudProvider, error) {
configFile, err := readConfig(configPath)
if err != nil {
return nil, err
Expand All @@ -88,12 +84,12 @@ func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath s
return nil, err
}

svc, err := createCloudService(ctx, vendorVersion, tokenSource)
svc, err := createCloudService(ctx, vendorVersion, tokenSource, computeEndpoint)
if err != nil {
return nil, err
}

betasvc, err := createBetaCloudService(ctx, vendorVersion, tokenSource)
betasvc, err := createBetaCloudService(ctx, vendorVersion, tokenSource, computeEndpoint)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -159,30 +155,40 @@ func readConfig(configPath string) (*ConfigFile, error) {
return cfg, nil
}

func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource) (*computebeta.Service, error) {
func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string) (*computebeta.Service, error) {
client, err := newOauthClient(ctx, tokenSource)
if err != nil {
return nil, err
}
service, err := computebeta.NewService(ctx, option.WithHTTPClient(client))

computeOpts := []option.ClientOption{option.WithHTTPClient(client)}
if computeEndpoint != "" {
computeOpts = append(computeOpts, option.WithEndpoint(computeEndpoint))
}
service, err := computebeta.NewService(ctx, computeOpts...)
if err != nil {
return nil, err
}
service.UserAgent = fmt.Sprintf("GCE CSI Driver/%s (%s %s)", vendorVersion, runtime.GOOS, runtime.GOARCH)
return service, nil
}

func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource) (*compute.Service, error) {
svc, err := createCloudServiceWithDefaultServiceAccount(ctx, vendorVersion, tokenSource)
func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string) (*compute.Service, error) {
svc, err := createCloudServiceWithDefaultServiceAccount(ctx, vendorVersion, tokenSource, computeEndpoint)
return svc, err
}

func createCloudServiceWithDefaultServiceAccount(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource) (*compute.Service, error) {
func createCloudServiceWithDefaultServiceAccount(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string) (*compute.Service, error) {
client, err := newOauthClient(ctx, tokenSource)
if err != nil {
return nil, err
}
service, err := compute.New(client)

computeOpts := []option.ClientOption{option.WithHTTPClient(client)}
if computeEndpoint != "" {
computeOpts = append(computeOpts, option.WithEndpoint(computeEndpoint))
}
service, err := compute.NewService(ctx, computeOpts...)
if err != nil {
return nil, err
}
Expand Down
7 changes: 3 additions & 4 deletions pkg/gce-pd-csi-driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
"context"
"fmt"
"math/rand"
"regexp"
"sort"
"strings"
"time"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
Expand Down Expand Up @@ -1526,9 +1526,8 @@ func generateCreateVolumeResponse(disk *gce.CloudDisk, zones []string) *csi.Crea
}

func cleanSelfLink(selfLink string) string {
temp := strings.TrimPrefix(selfLink, gce.GCEComputeAPIEndpoint)
temp = strings.TrimPrefix(temp, gce.GCEComputeBetaAPIEndpoint)
return strings.TrimPrefix(temp, gce.GCEComputeAlphaAPIEndpoint)
r, _ := regexp.Compile("https:\\/\\/www.*apis.com\\/.*(v1|beta|alpha)\\/")
return r.ReplaceAllString(selfLink, "")
}

func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name string, zones []string, params common.DiskParameters, capacityRange *csi.CapacityRange, capBytes int64, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool) (*gce.CloudDisk, error) {
Expand Down
65 changes: 65 additions & 0 deletions pkg/gce-pd-csi-driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2294,6 +2294,71 @@ func TestControllerPublishBackoffMissingInstance(t *testing.T) {
})
}

func TestCleanSelfLink(t *testing.T) {
testCases := []struct {
name string
in string
want string
}{
{
name: "v1 full standard w/ endpoint prefix",
in: "https://www.googleapis.com/compute/v1/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
{
name: "beta full standard w/ endpoint prefix",
in: "https://www.googleapis.com/compute/beta/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
{
name: "alpha full standard w/ endpoint prefix",
in: "https://www.googleapis.com/compute/alpha/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
{
name: "no prefix",
in: "projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},

{
name: "no prefix + project omitted",
in: "zones/zone/disks/disk",
want: "zones/zone/disks/disk",
},
{
name: "Compute prefix, google api",
in: "https://www.compute.googleapis.com/compute/v1/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
{
name: "Compute prefix, partner api",
in: "https://www.compute.PARTNERapis.com/compute/v1/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
{
name: "Partner beta api",
in: "https://www.PARTNERapis.com/compute/beta/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
{
name: "Partner alpha api",
in: "https://www.partnerapis.com/compute/alpha/projects/project/zones/zone/disks/disk",
want: "projects/project/zones/zone/disks/disk",
},
}

// Run test cases
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
got := cleanSelfLink(tc.in)
if got != tc.want {
t.Errorf("Expected cleaned self link: %v, got: %v", tc.want, got)
}
})
}
}

func backoffTesterForPublish(t *testing.T, config *backoffTesterConfig) {
readyToExecute := make(chan chan gce.Signal)
cloudDisks := []*gce.CloudDisk{
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/tests/setup_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ var _ = BeforeSuite(func() {

klog.Infof("Creating new driver and client for node %s\n", i.GetName())
// Create new driver and client
testContext, err := testutils.GCEClientAndDriverSetup(i)
testContext, err := testutils.GCEClientAndDriverSetup(i, "")
if err != nil {
klog.Fatalf("Failed to set up Test Context for instance %v: %v", i.GetName(), err)
}
Expand Down
40 changes: 37 additions & 3 deletions test/e2e/tests/single_zone_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"path/filepath"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -1123,6 +1124,40 @@ var _ = Describe("GCE PD CSI Driver", func() {
Expect(gce.IsGCEError(err, "notFound")).To(BeTrue(), "Expected disk to not be found")
}()
})

It("Should pass/fail if valid/invalid compute endpoint is passed in", func() {
// gets instance set up w/o compute-endpoint set from test setup
_, err := getRandomTestContext().Client.ListVolumes()
Expect(err).To(BeNil(), "no error expected when passed valid compute url")

zone := "us-central1-c"
nodeID := fmt.Sprintf("gce-pd-csi-e2e-%s", zone)
i, err := remote.SetupInstance(*project, *architecture, zone, nodeID, *machineType, *serviceAccount, *imageURL, computeService)

if err != nil {
klog.Fatalf("Failed to setup instance %v: %v", nodeID, err)
}

klog.Infof("Creating new driver and client for node %s\n", i.GetName())

// Create new driver and client w/ invalid endpoint
tcInvalid, err := testutils.GCEClientAndDriverSetup(i, "invalid-string")
if err != nil {
klog.Fatalf("Failed to set up Test Context for instance %v: %v", i.GetName(), err)
}

_, err = tcInvalid.Client.ListVolumes()
Expect(err.Error()).To(ContainSubstring("no such host"), "expected error when passed invalid compute url")

// Create new driver and client w/ valid, passed-in endpoint
tcValid, err := testutils.GCEClientAndDriverSetup(i, "https://compute.googleapis.com/compute/v1/")
if err != nil {
klog.Fatalf("Failed to set up Test Context for instance %v: %v", i.GetName(), err)
}
_, err = tcValid.Client.ListVolumes()

Expect(err).To(BeNil(), "no error expected when passed valid compute url")
})
})

func equalWithinEpsilon(a, b, epsiolon int64) bool {
Expand Down Expand Up @@ -1204,7 +1239,6 @@ func createAndValidateUniqueZonalMultiWriterDisk(client *remote.CsiClient, proje
}

func cleanSelfLink(selfLink string) string {
temp := strings.TrimPrefix(selfLink, gce.GCEComputeAPIEndpoint)
temp = strings.TrimPrefix(temp, gce.GCEComputeBetaAPIEndpoint)
return strings.TrimPrefix(temp, gce.GCEComputeAlphaAPIEndpoint)
r, _ := regexp.Compile("https:\\/\\/www.*apis.com\\/.*(v1|beta|alpha)\\/")
return r.ReplaceAllString(selfLink, "")
}
10 changes: 7 additions & 3 deletions test/e2e/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var (
boskos, _ = boskosclient.NewClient(os.Getenv("JOB_NAME"), "http://boskos", "", "")
)

func GCEClientAndDriverSetup(instance *remote.InstanceInfo) (*remote.TestContext, error) {
func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint string) (*remote.TestContext, error) {
port := fmt.Sprintf("%v", 1024+rand.Intn(10000))
goPath, ok := os.LookupEnv("GOPATH")
if !ok {
Expand All @@ -53,10 +53,14 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo) (*remote.TestContext
binPath := path.Join(pkgPath, "bin/gce-pd-csi-driver")

endpoint := fmt.Sprintf("tcp://localhost:%s", port)
computeFlag := ""
if computeEndpoint != "" {
computeFlag = fmt.Sprintf("--compute-endpoint %s", computeEndpoint)
}

workspace := remote.NewWorkspaceDir("gce-pd-e2e-")
driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver -v=4 --endpoint=%s --extra-labels=%s=%s 2> %s/prog.out < /dev/null > /dev/null &'",
workspace, endpoint, DiskLabelKey, DiskLabelValue, workspace)
driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver -v=4 --endpoint=%s %s --extra-labels=%s=%s 2> %s/prog.out < /dev/null > /dev/null &'",
workspace, endpoint, computeFlag, DiskLabelKey, DiskLabelValue, workspace)

config := &remote.ClientConfig{
PkgPath: pkgPath,
Expand Down