From 14df6a49468c36225d60b745c1fa251df1d0f8fa Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Thu, 1 Feb 2024 23:28:16 +0000 Subject: [PATCH 01/11] update driver to support staging compute --- cmd/gce-pd-csi-driver/main.go | 3 +- pkg/gce-cloud-provider/compute/gce.go | 92 +++++++++++++++++---------- pkg/gce-pd-csi-driver/controller.go | 2 +- 3 files changed, 60 insertions(+), 37 deletions(-) diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go index dcf94f41a..92defd22c 100644 --- a/cmd/gce-pd-csi-driver/main.go +++ b/cmd/gce-pd-csi-driver/main.go @@ -67,6 +67,7 @@ var ( maxConcurrentFormatAndMount = flag.Int("max-concurrent-format-and-mount", 1, "If set then format and mount operations are serialized on each node. This is stronger than max-concurrent-format as it includes fsck and other mount operations") formatAndMountTimeout = flag.Duration("format-and-mount-timeout", 1*time.Minute, "The maximum duration of a format and mount operation before another such operation will be started. Used only if --serialize-format-and-mount") + computeEnvironment = flag.String("compute-environment", "prod", "Sets the compute environment") fallbackRequisiteZonesFlag = flag.String("fallback-requisite-zones", "", "Comma separated list of requisite zones that will be used if there are not sufficient zones present in requisite topologies when provisioning a disk") @@ -156,7 +157,7 @@ func handle() { // Initialize requirements for the controller service var controllerServer *driver.GCEControllerServer if *runControllerService { - cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, *computeEndpoint) + cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, *computeEndpoint, *computeEnvironment) if err != nil { klog.Fatalf("Failed to get cloud provider: %v", err.Error()) } diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index e2a998b16..96b616331 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "os" "runtime" "time" @@ -47,8 +48,27 @@ const ( regionURITemplate = "projects/%s/regions/%s" replicaZoneURITemplateSingleZone = "projects/%s/zones/%s" // {gce.projectID}/zones/{disk.Zone} + versionV1 = "v1" + versionBeta = "beta" + versionAlpha = "alpha" + googleEnv = "googleapis" ) +var computeVersionMap = map[string]map[string]map[string]string{ + googleEnv: { + "prod": { + versionV1: "compute/v1/", + versionBeta: "compute/beta/", + versionAlpha: "compute/alpha/", + }, + "staging": { + versionV1: "compute/staging_v1/", + versionBeta: "compute/staging_beta/", + versionAlpha: "compute/staging_alpha/", + }, + }, +} + type CloudProvider struct { service *compute.Service betaService *computebeta.Service @@ -72,7 +92,7 @@ type ConfigGlobal struct { Zone string `gcfg:"zone"` } -func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint string) (*CloudProvider, error) { +func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint string, computeEnvironment string) (*CloudProvider, error) { configFile, err := readConfig(configPath) if err != nil { return nil, err @@ -87,20 +107,23 @@ func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath s return nil, err } - svc, err := createCloudService(ctx, vendorVersion, tokenSource, computeEndpoint) + svc, err := createCloudService(ctx, vendorVersion, tokenSource, computeEndpoint, computeEnvironment) if err != nil { return nil, err } + klog.Infof("Compute endpoint for V1 version: %s", svc.BasePath) - betasvc, err := createBetaCloudService(ctx, vendorVersion, tokenSource, computeEndpoint) + betasvc, err := createBetaCloudService(ctx, vendorVersion, tokenSource, computeEndpoint, computeEnvironment) if err != nil { return nil, err } + klog.Infof("Compute endpoint for Beta version: %s", betasvc.BasePath) - alphasvc, err := createAlphaCloudService(ctx, vendorVersion, tokenSource, computeEndpoint) + alphasvc, err := createAlphaCloudService(ctx, vendorVersion, tokenSource, computeEndpoint, computeEnvironment) if err != nil { return nil, err } + klog.Infof("Compute endpoint for Alpha version: %s", alphasvc.BasePath) project, zone, err := getProjectAndZone(configFile) if err != nil { @@ -164,16 +187,23 @@ func readConfig(configPath string) (*ConfigFile, error) { return cfg, nil } -func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string) (*computebeta.Service, error) { - client, err := newOauthClient(ctx, tokenSource) +func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string) (*computealpha.Service, error) { + computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionAlpha) + if err != nil { + klog.Errorf("Failed to get compute endpoint: %s", err) + } + service, err := computealpha.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 +} - computeOpts := []option.ClientOption{option.WithHTTPClient(client)} - if computeEndpoint != "" { - betaEndpoint := fmt.Sprintf("%s/compute/beta/", computeEndpoint) - computeOpts = append(computeOpts, option.WithEndpoint(betaEndpoint)) +func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string) (*computebeta.Service, error) { + computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionBeta) + if err != nil { + klog.Errorf("Failed to get compute endpoint: %s", err) } service, err := computebeta.NewService(ctx, computeOpts...) if err != nil { @@ -183,18 +213,12 @@ func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSour return service, nil } -func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string) (*computealpha.Service, error) { - client, err := newOauthClient(ctx, tokenSource) +func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string) (*compute.Service, error) { + computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionV1) if err != nil { - return nil, err - } - - computeOpts := []option.ClientOption{option.WithHTTPClient(client)} - if computeEndpoint != "" { - alphaEndpoint := fmt.Sprintf("%s/compute/alpha/", computeEndpoint) - computeOpts = append(computeOpts, option.WithEndpoint(alphaEndpoint)) + klog.Errorf("Failed to get compute endpoint: %s", err) } - service, err := computealpha.NewService(ctx, computeOpts...) + service, err := compute.NewService(ctx, computeOpts...) if err != nil { return nil, err } @@ -202,28 +226,26 @@ func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSou return service, nil } -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, computeEndpoint string) (*compute.Service, error) { +func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string, computeVersion string) ([]option.ClientOption, error) { client, err := newOauthClient(ctx, tokenSource) if err != nil { return nil, err } - + computeEnvironmentSuffix, ok := computeVersionMap[googleEnv][computeEnvironment][computeVersion] + if !ok { + return nil, errors.New("Unable to fetch compute endpoint") + } computeOpts := []option.ClientOption{option.WithHTTPClient(client)} if computeEndpoint != "" { - v1Endpoint := fmt.Sprintf("%s/compute/v1/", computeEndpoint) - computeOpts = append(computeOpts, option.WithEndpoint(v1Endpoint)) - } - service, err := compute.NewService(ctx, computeOpts...) - if err != nil { - return nil, err + endpoint := fmt.Sprintf("%s%s", computeEndpoint, computeEnvironmentSuffix) + klog.Infof("Got compute endpoint %s", endpoint) + _, err := url.ParseRequestURI(endpoint) + if err != nil { + klog.Fatalf("Error parsing compute endpoint %s", endpoint) + } + computeOpts = append(computeOpts, option.WithEndpoint(endpoint)) } - service.UserAgent = fmt.Sprintf("GCE CSI Driver/%s (%s %s)", vendorVersion, runtime.GOOS, runtime.GOARCH) - return service, nil + return computeOpts, nil } func newOauthClient(ctx context.Context, tokenSource oauth2.TokenSource) (*http.Client, error) { diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index fc8aeae66..fb4745015 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -159,7 +159,7 @@ const ( ) var ( - validResourceApiVersions = map[string]bool{"v1": true, "alpha": true, "beta": true} + validResourceApiVersions = map[string]bool{"v1": true, "alpha": true, "beta": true, "staging_v1": true, "staging_beta": true, "staging_alpha": true} ) func isDiskReady(disk *gce.CloudDisk) (bool, error) { From 609ad4ae2a4bcc12644f4fcac1a56331e8cb91e6 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Tue, 6 Feb 2024 18:50:40 +0000 Subject: [PATCH 02/11] add logs for testing --- test/e2e/utils/utils.go | 3 ++- test/remote/runner.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index 97364bed4..b819bad57 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -53,6 +53,7 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint stri binPath := path.Join(pkgPath, "bin/gce-pd-csi-driver") endpoint := fmt.Sprintf("tcp://localhost:%s", port) + klog.Infof("ENDPOINT: %s", endpoint) extra_flags := []string{ fmt.Sprintf("--extra-labels=%s=%s", DiskLabelKey, DiskLabelValue), "--max-concurrent-format-and-mount=20", // otherwise the serialization times out the e2e test. @@ -65,7 +66,7 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint stri // useful to see what's happening when debugging tests. driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver -v=6 --endpoint=%s %s 2> %s/prog.out < /dev/null > /dev/null &'", workspace, endpoint, strings.Join(extra_flags, " "), workspace) - + klog.Infof("DRIVER COMMAND %s", driverRunCmd) config := &remote.ClientConfig{ PkgPath: pkgPath, BinPath: binPath, diff --git a/test/remote/runner.go b/test/remote/runner.go index 2fb8d8c93..d4f4135b0 100644 --- a/test/remote/runner.go +++ b/test/remote/runner.go @@ -59,6 +59,7 @@ func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd s klog.V(4).Infof("Starting driver on %q", i.name) // When the process is killed the driver should close the TCP endpoint, then we want to download the logs + klog.Infof("DRIVER RUN COMMAND: %s", driverRunCmd) output, err := i.SSH(driverRunCmd) if err != nil { // Exit failure with the error From bd5b16d5be172ee79b117a2b585ee0eb920be7bc Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Tue, 6 Feb 2024 21:30:48 +0000 Subject: [PATCH 03/11] add logs for PID --- test/remote/runner.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/remote/runner.go b/test/remote/runner.go index d4f4135b0..d505a656c 100644 --- a/test/remote/runner.go +++ b/test/remote/runner.go @@ -76,6 +76,7 @@ func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd s //`awk "{print \$2}"`, ) driverPIDString, err := i.SSHNoSudo("sh", "-c", driverPIDCmd) + klog.Infof("DRIVER PID STRING %s: COMMAND: %s", driverPIDString, driverPIDCmd) if err != nil { // Exit failure with the error return -1, fmt.Errorf("failed to get PID of driver, got output: %v, error: %v", output, err.Error()) From 0f4f85668b773eaf1afd2d15ac07f98545bc51c2 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Fri, 9 Feb 2024 23:32:57 +0000 Subject: [PATCH 04/11] Add checks for compute environment flags and url checks --- cmd/gce-pd-csi-driver/main.go | 30 ++++++++++--- pkg/gce-cloud-provider/compute/gce.go | 61 ++++++++++++-------------- test/e2e/tests/single_zone_e2e_test.go | 9 ---- 3 files changed, 52 insertions(+), 48 deletions(-) diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go index 92defd22c..2c2ebe37a 100644 --- a/cmd/gce-pd-csi-driver/main.go +++ b/cmd/gce-pd-csi-driver/main.go @@ -17,7 +17,9 @@ package main import ( "context" + "errors" "flag" + "fmt" "math/rand" "os" "runtime" @@ -67,13 +69,12 @@ var ( maxConcurrentFormatAndMount = flag.Int("max-concurrent-format-and-mount", 1, "If set then format and mount operations are serialized on each node. This is stronger than max-concurrent-format as it includes fsck and other mount operations") formatAndMountTimeout = flag.Duration("format-and-mount-timeout", 1*time.Minute, "The maximum duration of a format and mount operation before another such operation will be started. Used only if --serialize-format-and-mount") - computeEnvironment = flag.String("compute-environment", "prod", "Sets the compute environment") + fallbackRequisiteZonesFlag = flag.String("fallback-requisite-zones", "", "Comma separated list of requisite zones that will be used if there are not sufficient zones present in requisite topologies when provisioning a disk") - fallbackRequisiteZonesFlag = flag.String("fallback-requisite-zones", "", "Comma separated list of requisite zones that will be used if there are not sufficient zones present in requisite topologies when provisioning a disk") - - enableStoragePoolsFlag = flag.Bool("enable-storage-pools", false, "If set to true, the CSI Driver will allow volumes to be provisioned in Storage Pools") - - version string + enableStoragePoolsFlag = flag.Bool("enable-storage-pools", false, "If set to true, the CSI Driver will allow volumes to be provisioned in Storage Pools") + computeEnvironment gce.Environment = "production" + version string + allowedComputeEnvironment = []string{"staging", "production"} ) const ( @@ -86,6 +87,7 @@ func init() { // Use V(4) for general debug information logging // Use V(5) for GCE Cloud Provider Call informational logging // Use V(6) for extra repeated/polling information + enumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment") klog.InitFlags(flag.CommandLine) flag.Set("logtostderr", "true") } @@ -157,7 +159,7 @@ func handle() { // Initialize requirements for the controller service var controllerServer *driver.GCEControllerServer if *runControllerService { - cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, *computeEndpoint, *computeEnvironment) + cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, *computeEndpoint, computeEnvironment) if err != nil { klog.Fatalf("Failed to get cloud provider: %v", err.Error()) } @@ -206,3 +208,17 @@ func handle() { gceDriver.Run(*endpoint, *grpcLogCharCap, *enableOtelTracing) } + +func enumFlag(target *gce.Environment, name string, allowedComputeEnvironment []string, usage string) { + flag.Func(name, usage, func(flagValue string) error { + for _, allowedValue := range allowedComputeEnvironment { + if flagValue == allowedValue { + *target = gce.Environment(flagValue) + return nil + } + } + errMsg := fmt.Sprintf(`must be one of %v`, allowedComputeEnvironment) + return errors.New(errMsg) + }) + +} diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index 96b616331..cfdebaab9 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -38,6 +38,9 @@ import ( "k8s.io/klog/v2" ) +type Environment string +type Version string + const ( TokenURL = "https://accounts.google.com/o/oauth2/token" diskSourceURITemplateSingleZone = "projects/%s/zones/%s/disks/%s" // {gce.projectID}/zones/{disk.Zone}/disks/{disk.Name}" @@ -47,28 +50,13 @@ const ( regionURITemplate = "projects/%s/regions/%s" - replicaZoneURITemplateSingleZone = "projects/%s/zones/%s" // {gce.projectID}/zones/{disk.Zone} - versionV1 = "v1" - versionBeta = "beta" - versionAlpha = "alpha" - googleEnv = "googleapis" + replicaZoneURITemplateSingleZone = "projects/%s/zones/%s" // {gce.projectID}/zones/{disk.Zone} + versionV1 Version = "v1" + versionBeta Version = "beta" + versionAlpha Version = "alpha" + environmentStaging Environment = "staging" ) -var computeVersionMap = map[string]map[string]map[string]string{ - googleEnv: { - "prod": { - versionV1: "compute/v1/", - versionBeta: "compute/beta/", - versionAlpha: "compute/alpha/", - }, - "staging": { - versionV1: "compute/staging_v1/", - versionBeta: "compute/staging_beta/", - versionAlpha: "compute/staging_alpha/", - }, - }, -} - type CloudProvider struct { service *compute.Service betaService *computebeta.Service @@ -92,7 +80,7 @@ type ConfigGlobal struct { Zone string `gcfg:"zone"` } -func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint string, computeEnvironment string) (*CloudProvider, error) { +func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint string, computeEnvironment Environment) (*CloudProvider, error) { configFile, err := readConfig(configPath) if err != nil { return nil, err @@ -187,7 +175,7 @@ func readConfig(configPath string) (*ConfigFile, error) { return cfg, nil } -func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string) (*computealpha.Service, error) { +func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment) (*computealpha.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionAlpha) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -200,7 +188,7 @@ func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSou return service, nil } -func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string) (*computebeta.Service, error) { +func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment) (*computebeta.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionBeta) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -213,7 +201,7 @@ func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSour return service, nil } -func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string) (*compute.Service, error) { +func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment) (*compute.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionV1) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -226,20 +214,21 @@ func createCloudService(ctx context.Context, vendorVersion string, tokenSource o return service, nil } -func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment string, computeVersion string) ([]option.ClientOption, error) { +func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment, computeVersion Version) ([]option.ClientOption, error) { client, err := newOauthClient(ctx, tokenSource) if err != nil { return nil, err } - computeEnvironmentSuffix, ok := computeVersionMap[googleEnv][computeEnvironment][computeVersion] - if !ok { - return nil, errors.New("Unable to fetch compute endpoint") - } + computeEnvironmentSuffix := getPath(computeEnvironment, computeVersion) computeOpts := []option.ClientOption{option.WithHTTPClient(client)} + if computeEndpoint != "" { - endpoint := fmt.Sprintf("%s%s", computeEndpoint, computeEnvironmentSuffix) - klog.Infof("Got compute endpoint %s", endpoint) - _, err := url.ParseRequestURI(endpoint) + computeURL, err := url.ParseRequestURI(computeEndpoint) + if err != nil { + return nil, err + } + endpoint := computeURL.JoinPath(computeEnvironmentSuffix).String() + _, err = url.ParseRequestURI(endpoint) if err != nil { klog.Fatalf("Error parsing compute endpoint %s", endpoint) } @@ -248,6 +237,14 @@ func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, comp return computeOpts, nil } +func getPath(env Environment, version Version) string { + prefix := "" + if env == environmentStaging { + prefix = fmt.Sprintf("%s_", env) + } + return fmt.Sprintf("compute/%s%s/", prefix, version) +} + func newOauthClient(ctx context.Context, tokenSource oauth2.TokenSource) (*http.Client, error) { if err := wait.PollImmediate(5*time.Second, 30*time.Second, func() (bool, error) { if _, err := tokenSource.Token(); err != nil { diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index 933b7551a..1bec2d998 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -1295,15 +1295,6 @@ var _ = Describe("GCE PD CSI Driver", func() { 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: %w", 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") if err != nil { From ef11d6b0bf06187882e0159350bb19fcda5ea563 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Wed, 14 Feb 2024 21:36:53 +0000 Subject: [PATCH 05/11] add unit test --- pkg/gce-cloud-provider/compute/gce_test.go | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/pkg/gce-cloud-provider/compute/gce_test.go b/pkg/gce-cloud-provider/compute/gce_test.go index 5bb2aed89..c7005e8e1 100644 --- a/pkg/gce-cloud-provider/compute/gce_test.go +++ b/pkg/gce-cloud-provider/compute/gce_test.go @@ -18,14 +18,28 @@ limitations under the License. package gcecloudprovider import ( + "context" "errors" "fmt" "net/http" "testing" + "time" + + "golang.org/x/oauth2" "google.golang.org/api/googleapi" ) +type mockTokenSource struct{} + +func (*mockTokenSource) Token() (*oauth2.Token, error) { + return &oauth2.Token{ + AccessToken: "access", + TokenType: "Bearer", + RefreshToken: "refresh", + Expiry: time.Now().Add(1 * time.Hour), + }, nil +} func TestIsGCEError(t *testing.T) { testCases := []struct { name string @@ -84,3 +98,55 @@ func TestIsGCEError(t *testing.T) { } } } + +func TestGetComputeVersion(t *testing.T) { + testCases := []struct { + name string + computeEndpoint string + computeEnvironment Environment + computeVersion Version + expectedEndpoint string + expectError bool + }{ + + { + name: "check for production environment", + computeEndpoint: "https://compute.googleapis.com", + computeEnvironment: "production", + computeVersion: "v1", + expectedEndpoint: "https://compute.googleapis.com/compute/v1/", + expectError: false, + }, + { + name: "check for incorrect endpoint", + computeEndpoint: "https://compute.googleapis", + computeEnvironment: "prod", + computeVersion: "v1", + expectError: true, + }, + { + name: "check for staging environment", + computeEndpoint: "https://compute.googleapis.com", + computeEnvironment: environmentStaging, + computeVersion: "v1", + expectedEndpoint: "compute/staging_v1/", + expectError: false, + }, + { + name: "check for random string as endpoint", + computeEndpoint: "compute-googleapis", + computeEnvironment: "prod", + computeVersion: "v1", + expectedEndpoint: "compute/v1/", + expectError: true, + }, + } + for _, tc := range testCases { + ctx := context.Background() + _, err := getComputeVersion(ctx, &mockTokenSource{}, tc.computeEndpoint, tc.computeEnvironment, tc.computeVersion) + if err != nil && !tc.expectError { + t.Fatalf("Got error %v, expected endpoint %s", err, tc.expectedEndpoint) + } + } + +} From 9c77fb463e8033299d060422e2fcef2365982fac Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Wed, 14 Feb 2024 22:18:37 +0000 Subject: [PATCH 06/11] Cleaning up some added debug log --- test/e2e/utils/utils.go | 2 -- test/remote/runner.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/test/e2e/utils/utils.go b/test/e2e/utils/utils.go index b819bad57..c9ee708ff 100644 --- a/test/e2e/utils/utils.go +++ b/test/e2e/utils/utils.go @@ -53,7 +53,6 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint stri binPath := path.Join(pkgPath, "bin/gce-pd-csi-driver") endpoint := fmt.Sprintf("tcp://localhost:%s", port) - klog.Infof("ENDPOINT: %s", endpoint) extra_flags := []string{ fmt.Sprintf("--extra-labels=%s=%s", DiskLabelKey, DiskLabelValue), "--max-concurrent-format-and-mount=20", // otherwise the serialization times out the e2e test. @@ -66,7 +65,6 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint stri // useful to see what's happening when debugging tests. driverRunCmd := fmt.Sprintf("sh -c '/usr/bin/nohup %s/gce-pd-csi-driver -v=6 --endpoint=%s %s 2> %s/prog.out < /dev/null > /dev/null &'", workspace, endpoint, strings.Join(extra_flags, " "), workspace) - klog.Infof("DRIVER COMMAND %s", driverRunCmd) config := &remote.ClientConfig{ PkgPath: pkgPath, BinPath: binPath, diff --git a/test/remote/runner.go b/test/remote/runner.go index d505a656c..2fb8d8c93 100644 --- a/test/remote/runner.go +++ b/test/remote/runner.go @@ -59,7 +59,6 @@ func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd s klog.V(4).Infof("Starting driver on %q", i.name) // When the process is killed the driver should close the TCP endpoint, then we want to download the logs - klog.Infof("DRIVER RUN COMMAND: %s", driverRunCmd) output, err := i.SSH(driverRunCmd) if err != nil { // Exit failure with the error @@ -76,7 +75,6 @@ func (i *InstanceInfo) UploadAndRun(archivePath, remoteWorkspace, driverRunCmd s //`awk "{print \$2}"`, ) driverPIDString, err := i.SSHNoSudo("sh", "-c", driverPIDCmd) - klog.Infof("DRIVER PID STRING %s: COMMAND: %s", driverPIDString, driverPIDCmd) if err != nil { // Exit failure with the error return -1, fmt.Errorf("failed to get PID of driver, got output: %v, error: %v", output, err.Error()) From e95a078cff027bf3110fcbac026053da8adb7864 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Wed, 14 Feb 2024 23:20:32 +0000 Subject: [PATCH 07/11] fail on incorrect endpoint --- pkg/gce-cloud-provider/compute/gce.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index cfdebaab9..d314721dc 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -231,6 +231,7 @@ func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, comp _, err = url.ParseRequestURI(endpoint) if err != nil { klog.Fatalf("Error parsing compute endpoint %s", endpoint) + return nil, err } computeOpts = append(computeOpts, option.WithEndpoint(endpoint)) } From 3c04c77f3fed270174595733705d315b8dd2365b Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Thu, 15 Feb 2024 00:18:11 +0000 Subject: [PATCH 08/11] remove invalid compute endpoint testcase as updated logic causes crash on invalid endpoint --- pkg/gce-cloud-provider/compute/gce.go | 1 - test/e2e/tests/single_zone_e2e_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index d314721dc..cfdebaab9 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -231,7 +231,6 @@ func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, comp _, err = url.ParseRequestURI(endpoint) if err != nil { klog.Fatalf("Error parsing compute endpoint %s", endpoint) - return nil, err } computeOpts = append(computeOpts, option.WithEndpoint(endpoint)) } diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index 1bec2d998..58911cd34 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -1280,7 +1280,7 @@ var _ = Describe("GCE PD CSI Driver", func() { }() }) - It("Should pass/fail if valid/invalid compute endpoint is passed in", func() { + It("Should pass if valid 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") From 5750bdf887c46459b846d7bd2c619bccdbda5bc5 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Thu, 15 Feb 2024 23:25:38 +0000 Subject: [PATCH 09/11] Add url check during initialization --- cmd/gce-pd-csi-driver/main.go | 25 +++++++++--- pkg/gce-cloud-provider/compute/gce.go | 32 ++++++--------- pkg/gce-cloud-provider/compute/gce_test.go | 46 +++++++++++++--------- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go index 2c2ebe37a..c0bfaf27a 100644 --- a/cmd/gce-pd-csi-driver/main.go +++ b/cmd/gce-pd-csi-driver/main.go @@ -21,6 +21,7 @@ import ( "flag" "fmt" "math/rand" + "net/url" "os" "runtime" "strings" @@ -40,7 +41,6 @@ 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.") 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.") @@ -72,9 +72,10 @@ var ( fallbackRequisiteZonesFlag = flag.String("fallback-requisite-zones", "", "Comma separated list of requisite zones that will be used if there are not sufficient zones present in requisite topologies when provisioning a disk") enableStoragePoolsFlag = flag.Bool("enable-storage-pools", false, "If set to true, the CSI Driver will allow volumes to be provisioned in Storage Pools") - computeEnvironment gce.Environment = "production" + computeEnvironment gce.Environment = gce.EnvironmentProduction + computeEndpoint url.URL version string - allowedComputeEnvironment = []string{"staging", "production"} + allowedComputeEnvironment = []gce.Environment{gce.EnvironmentStaging, gce.EnvironmentProduction} ) const ( @@ -88,6 +89,7 @@ func init() { // Use V(5) for GCE Cloud Provider Call informational logging // Use V(6) for extra repeated/polling information enumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment") + urlFlag(&computeEndpoint, "compute-endpoint", "Compute endpoint") klog.InitFlags(flag.CommandLine) flag.Set("logtostderr", "true") } @@ -159,7 +161,7 @@ func handle() { // Initialize requirements for the controller service var controllerServer *driver.GCEControllerServer if *runControllerService { - cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, *computeEndpoint, computeEnvironment) + cloudProvider, err := gce.CreateCloudProvider(ctx, version, *cloudConfigFilePath, computeEndpoint, computeEnvironment) if err != nil { klog.Fatalf("Failed to get cloud provider: %v", err.Error()) } @@ -209,10 +211,10 @@ func handle() { gceDriver.Run(*endpoint, *grpcLogCharCap, *enableOtelTracing) } -func enumFlag(target *gce.Environment, name string, allowedComputeEnvironment []string, usage string) { +func enumFlag(target *gce.Environment, name string, allowedComputeEnvironment []gce.Environment, usage string) { flag.Func(name, usage, func(flagValue string) error { for _, allowedValue := range allowedComputeEnvironment { - if flagValue == allowedValue { + if gce.Environment(flagValue) == allowedValue { *target = gce.Environment(flagValue) return nil } @@ -222,3 +224,14 @@ func enumFlag(target *gce.Environment, name string, allowedComputeEnvironment [] }) } + +func urlFlag(target *url.URL, name string, usage string) { + flag.Func(name, usage, func(flagValue string) error { + computeURL, err := url.ParseRequestURI(flagValue) + if err == nil { + *target = *computeURL + return nil + } + return err + }) +} diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index cfdebaab9..cc5d9b7a5 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -54,7 +54,8 @@ const ( versionV1 Version = "v1" versionBeta Version = "beta" versionAlpha Version = "alpha" - environmentStaging Environment = "staging" + EnvironmentStaging Environment = "staging" + EnvironmentProduction Environment = "production" ) type CloudProvider struct { @@ -80,7 +81,7 @@ type ConfigGlobal struct { Zone string `gcfg:"zone"` } -func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint string, computeEnvironment Environment) (*CloudProvider, error) { +func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint url.URL, computeEnvironment Environment) (*CloudProvider, error) { configFile, err := readConfig(configPath) if err != nil { return nil, err @@ -175,7 +176,7 @@ func readConfig(configPath string) (*ConfigFile, error) { return cfg, nil } -func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment) (*computealpha.Service, error) { +func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment) (*computealpha.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionAlpha) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -188,7 +189,7 @@ func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSou return service, nil } -func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment) (*computebeta.Service, error) { +func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment) (*computebeta.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionBeta) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -201,7 +202,7 @@ func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSour return service, nil } -func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment) (*compute.Service, error) { +func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment) (*compute.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionV1) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -214,32 +215,25 @@ func createCloudService(ctx context.Context, vendorVersion string, tokenSource o return service, nil } -func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint string, computeEnvironment Environment, computeVersion Version) ([]option.ClientOption, error) { +func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment, computeVersion Version) ([]option.ClientOption, error) { client, err := newOauthClient(ctx, tokenSource) if err != nil { return nil, err } - computeEnvironmentSuffix := getPath(computeEnvironment, computeVersion) computeOpts := []option.ClientOption{option.WithHTTPClient(client)} - if computeEndpoint != "" { - computeURL, err := url.ParseRequestURI(computeEndpoint) - if err != nil { - return nil, err - } - endpoint := computeURL.JoinPath(computeEnvironmentSuffix).String() - _, err = url.ParseRequestURI(endpoint) - if err != nil { - klog.Fatalf("Error parsing compute endpoint %s", endpoint) - } + if computeEndpoint.String() != "" { + computeEnvironmentSuffix := constructComputeEndpointPath(computeEnvironment, computeVersion) + computeEndpoint.Path = computeEnvironmentSuffix + endpoint := computeEndpoint.String() computeOpts = append(computeOpts, option.WithEndpoint(endpoint)) } return computeOpts, nil } -func getPath(env Environment, version Version) string { +func constructComputeEndpointPath(env Environment, version Version) string { prefix := "" - if env == environmentStaging { + if env == EnvironmentStaging { prefix = fmt.Sprintf("%s_", env) } return fmt.Sprintf("compute/%s%s/", prefix, version) diff --git a/pkg/gce-cloud-provider/compute/gce_test.go b/pkg/gce-cloud-provider/compute/gce_test.go index c7005e8e1..824ad22c5 100644 --- a/pkg/gce-cloud-provider/compute/gce_test.go +++ b/pkg/gce-cloud-provider/compute/gce_test.go @@ -22,11 +22,13 @@ import ( "errors" "fmt" "net/http" + "net/url" "testing" "time" "golang.org/x/oauth2" + "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" ) @@ -102,7 +104,7 @@ func TestIsGCEError(t *testing.T) { func TestGetComputeVersion(t *testing.T) { testCases := []struct { name string - computeEndpoint string + computeEndpoint url.URL computeEnvironment Environment computeVersion Version expectedEndpoint string @@ -111,30 +113,23 @@ func TestGetComputeVersion(t *testing.T) { { name: "check for production environment", - computeEndpoint: "https://compute.googleapis.com", - computeEnvironment: "production", - computeVersion: "v1", - expectedEndpoint: "https://compute.googleapis.com/compute/v1/", + computeEndpoint: convertStringToURL("https://compute.googleapis.com"), + computeEnvironment: EnvironmentProduction, + computeVersion: versionBeta, + expectedEndpoint: "https://compute.googleapis.com/compute/beta/", expectError: false, }, - { - name: "check for incorrect endpoint", - computeEndpoint: "https://compute.googleapis", - computeEnvironment: "prod", - computeVersion: "v1", - expectError: true, - }, { name: "check for staging environment", - computeEndpoint: "https://compute.googleapis.com", - computeEnvironment: environmentStaging, - computeVersion: "v1", - expectedEndpoint: "compute/staging_v1/", + computeEndpoint: convertStringToURL("https://compute.googleapis.com"), + computeEnvironment: EnvironmentStaging, + computeVersion: versionV1, + expectedEndpoint: "https://compute.googleapis.com/compute/staging_v1/", expectError: false, }, { name: "check for random string as endpoint", - computeEndpoint: "compute-googleapis", + computeEndpoint: url.URL{}, computeEnvironment: "prod", computeVersion: "v1", expectedEndpoint: "compute/v1/", @@ -143,10 +138,23 @@ func TestGetComputeVersion(t *testing.T) { } for _, tc := range testCases { ctx := context.Background() - _, err := getComputeVersion(ctx, &mockTokenSource{}, tc.computeEndpoint, tc.computeEnvironment, tc.computeVersion) + computeOpts, err := getComputeVersion(ctx, &mockTokenSource{}, tc.computeEndpoint, tc.computeEnvironment, tc.computeVersion) + service, _ := compute.NewService(ctx, computeOpts...) + gotEndpoint := service.BasePath if err != nil && !tc.expectError { - t.Fatalf("Got error %v, expected endpoint %s", err, tc.expectedEndpoint) + t.Fatalf("Got error %v", err) + } + if gotEndpoint != tc.expectedEndpoint && !tc.expectError { + t.Fatalf("expected endpoint %s, got endpoint %s", tc.expectedEndpoint, gotEndpoint) } } } + +func convertStringToURL(urlString string) url.URL { + parsedURL, err := url.ParseRequestURI(urlString) + if err != nil { + return url.URL{} + } + return *parsedURL +} From da30cb84f0167a3476974a74a06c9e96a069494f Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Thu, 15 Feb 2024 23:51:01 +0000 Subject: [PATCH 10/11] Update flag to use pointers --- cmd/gce-pd-csi-driver/main.go | 6 +++--- pkg/gce-cloud-provider/compute/gce.go | 12 ++++++------ pkg/gce-cloud-provider/compute/gce_test.go | 10 +++++----- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go index c0bfaf27a..41f9f4669 100644 --- a/cmd/gce-pd-csi-driver/main.go +++ b/cmd/gce-pd-csi-driver/main.go @@ -73,7 +73,7 @@ var ( enableStoragePoolsFlag = flag.Bool("enable-storage-pools", false, "If set to true, the CSI Driver will allow volumes to be provisioned in Storage Pools") computeEnvironment gce.Environment = gce.EnvironmentProduction - computeEndpoint url.URL + computeEndpoint *url.URL version string allowedComputeEnvironment = []gce.Environment{gce.EnvironmentStaging, gce.EnvironmentProduction} ) @@ -89,7 +89,7 @@ func init() { // Use V(5) for GCE Cloud Provider Call informational logging // Use V(6) for extra repeated/polling information enumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment") - urlFlag(&computeEndpoint, "compute-endpoint", "Compute endpoint") + urlFlag(computeEndpoint, "compute-endpoint", "Compute endpoint") klog.InitFlags(flag.CommandLine) flag.Set("logtostderr", "true") } @@ -229,7 +229,7 @@ func urlFlag(target *url.URL, name string, usage string) { flag.Func(name, usage, func(flagValue string) error { computeURL, err := url.ParseRequestURI(flagValue) if err == nil { - *target = *computeURL + target = computeURL return nil } return err diff --git a/pkg/gce-cloud-provider/compute/gce.go b/pkg/gce-cloud-provider/compute/gce.go index cc5d9b7a5..2c769cedc 100644 --- a/pkg/gce-cloud-provider/compute/gce.go +++ b/pkg/gce-cloud-provider/compute/gce.go @@ -81,7 +81,7 @@ type ConfigGlobal struct { Zone string `gcfg:"zone"` } -func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint url.URL, computeEnvironment Environment) (*CloudProvider, error) { +func CreateCloudProvider(ctx context.Context, vendorVersion string, configPath string, computeEndpoint *url.URL, computeEnvironment Environment) (*CloudProvider, error) { configFile, err := readConfig(configPath) if err != nil { return nil, err @@ -176,7 +176,7 @@ func readConfig(configPath string) (*ConfigFile, error) { return cfg, nil } -func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment) (*computealpha.Service, error) { +func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint *url.URL, computeEnvironment Environment) (*computealpha.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionAlpha) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -189,7 +189,7 @@ func createAlphaCloudService(ctx context.Context, vendorVersion string, tokenSou return service, nil } -func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment) (*computebeta.Service, error) { +func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint *url.URL, computeEnvironment Environment) (*computebeta.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionBeta) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -202,7 +202,7 @@ func createBetaCloudService(ctx context.Context, vendorVersion string, tokenSour return service, nil } -func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment) (*compute.Service, error) { +func createCloudService(ctx context.Context, vendorVersion string, tokenSource oauth2.TokenSource, computeEndpoint *url.URL, computeEnvironment Environment) (*compute.Service, error) { computeOpts, err := getComputeVersion(ctx, tokenSource, computeEndpoint, computeEnvironment, versionV1) if err != nil { klog.Errorf("Failed to get compute endpoint: %s", err) @@ -215,14 +215,14 @@ func createCloudService(ctx context.Context, vendorVersion string, tokenSource o return service, nil } -func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint url.URL, computeEnvironment Environment, computeVersion Version) ([]option.ClientOption, error) { +func getComputeVersion(ctx context.Context, tokenSource oauth2.TokenSource, computeEndpoint *url.URL, computeEnvironment Environment, computeVersion Version) ([]option.ClientOption, error) { client, err := newOauthClient(ctx, tokenSource) if err != nil { return nil, err } computeOpts := []option.ClientOption{option.WithHTTPClient(client)} - if computeEndpoint.String() != "" { + if computeEndpoint != nil { computeEnvironmentSuffix := constructComputeEndpointPath(computeEnvironment, computeVersion) computeEndpoint.Path = computeEnvironmentSuffix endpoint := computeEndpoint.String() diff --git a/pkg/gce-cloud-provider/compute/gce_test.go b/pkg/gce-cloud-provider/compute/gce_test.go index 824ad22c5..49f85221d 100644 --- a/pkg/gce-cloud-provider/compute/gce_test.go +++ b/pkg/gce-cloud-provider/compute/gce_test.go @@ -104,7 +104,7 @@ func TestIsGCEError(t *testing.T) { func TestGetComputeVersion(t *testing.T) { testCases := []struct { name string - computeEndpoint url.URL + computeEndpoint *url.URL computeEnvironment Environment computeVersion Version expectedEndpoint string @@ -129,7 +129,7 @@ func TestGetComputeVersion(t *testing.T) { }, { name: "check for random string as endpoint", - computeEndpoint: url.URL{}, + computeEndpoint: convertStringToURL(""), computeEnvironment: "prod", computeVersion: "v1", expectedEndpoint: "compute/v1/", @@ -151,10 +151,10 @@ func TestGetComputeVersion(t *testing.T) { } -func convertStringToURL(urlString string) url.URL { +func convertStringToURL(urlString string) *url.URL { parsedURL, err := url.ParseRequestURI(urlString) if err != nil { - return url.URL{} + return nil } - return *parsedURL + return parsedURL } From 9f5ca1bd47d1ca8a83ffb4f427b48d2bb8c52726 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Fri, 16 Feb 2024 23:39:00 +0000 Subject: [PATCH 11/11] fix pointer issue for GCE staging support --- cmd/gce-pd-csi-driver/main.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/gce-pd-csi-driver/main.go b/cmd/gce-pd-csi-driver/main.go index 41f9f4669..8a122ee64 100644 --- a/cmd/gce-pd-csi-driver/main.go +++ b/cmd/gce-pd-csi-driver/main.go @@ -89,7 +89,7 @@ func init() { // Use V(5) for GCE Cloud Provider Call informational logging // Use V(6) for extra repeated/polling information enumFlag(&computeEnvironment, "compute-environment", allowedComputeEnvironment, "Operating compute environment") - urlFlag(computeEndpoint, "compute-endpoint", "Compute endpoint") + urlFlag(&computeEndpoint, "compute-endpoint", "Compute endpoint") klog.InitFlags(flag.CommandLine) flag.Set("logtostderr", "true") } @@ -97,6 +97,7 @@ func init() { func main() { flag.Parse() rand.Seed(time.Now().UnixNano()) + klog.Infof("Operating compute environment set to: %s and computeEndpoint is set to: %v", computeEnvironment, computeEndpoint) handle() os.Exit(0) } @@ -225,13 +226,14 @@ func enumFlag(target *gce.Environment, name string, allowedComputeEnvironment [] } -func urlFlag(target *url.URL, name string, usage string) { +func urlFlag(target **url.URL, name string, usage string) { flag.Func(name, usage, func(flagValue string) error { computeURL, err := url.ParseRequestURI(flagValue) if err == nil { - target = computeURL + *target = computeURL return nil } + klog.Infof("Error parsing endpoint compute endpoint %v", err) return err }) }