From 5789a076f72723fda662497caa30f7b04e54c0ea Mon Sep 17 00:00:00 2001 From: karkunpavan Date: Fri, 10 Jan 2025 12:56:38 +0000 Subject: [PATCH 1/9] changes to support hyperdisk multi-writer mode --- pkg/common/parameters.go | 4 ++++ pkg/gce-cloud-provider/compute/cloud-disk.go | 2 ++ pkg/gce-cloud-provider/compute/gce-compute.go | 20 +++++-------------- pkg/gce-pd-csi-driver/controller.go | 2 +- test/e2e/tests/single_zone_e2e_test.go | 12 +++++------ 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index 872837193..837b52cee 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -106,6 +106,9 @@ type DiskParameters struct { // Values: {bool} // Default: false MultiZoneProvisioning bool + // Values: READ_WRITE_SINGLE, READ_ONLY_MANY, READ_WRITE_MANY + // Default: READ_WRITE_SINGLE + AccessMode string } // SnapshotParameters contains normalized and defaulted parameters for snapshots @@ -148,6 +151,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string] Tags: make(map[string]string), // Default Labels: make(map[string]string), // Default ResourceTags: make(map[string]string), // Default + AccessMode: "READ_WRITE_SINGLE", // Default } for k, v := range extraVolumeLabels { diff --git a/pkg/gce-cloud-provider/compute/cloud-disk.go b/pkg/gce-cloud-provider/compute/cloud-disk.go index 6790992c4..71ed6490b 100644 --- a/pkg/gce-cloud-provider/compute/cloud-disk.go +++ b/pkg/gce-cloud-provider/compute/cloud-disk.go @@ -217,6 +217,8 @@ func (d *CloudDisk) GetMultiWriter() bool { switch { case d.disk != nil: return false + case d.disk != nil && d.disk.AccessMode == "READ_WRITE_MANY": + return true case d.betaDisk != nil: return d.betaDisk.MultiWriter default: diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index c2c2b7603..940d9d86f 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -324,8 +324,6 @@ func (cloud *CloudProvider) ListSnapshots(ctx context.Context, filter string) ([ func (cloud *CloudProvider) GetDisk(ctx context.Context, project string, key *meta.Key, gceAPIVersion GCEAPIVersion) (*CloudDisk, error) { klog.V(5).Infof("Getting disk %v", key) - // Override GCEAPIVersion as hyperdisk is only available in beta and we cannot get the disk-type with get disk call. - gceAPIVersion = GCEAPIVersionBeta switch key.Type() { case meta.Zonal: if gceAPIVersion == GCEAPIVersionBeta { @@ -407,11 +405,6 @@ func (cloud *CloudProvider) ValidateExistingDisk(ctx context.Context, resp *Clou reqBytes, common.GbToBytes(resp.GetSizeGb()), limBytes) } - // We are assuming here that a multiWriter disk could be used as non-multiWriter - if multiWriter && !resp.GetMultiWriter() { - return fmt.Errorf("disk already exists with incompatible capability. Need MultiWriter. Got non-MultiWriter") - } - return ValidateDiskParameters(resp, params) } @@ -553,9 +546,6 @@ func convertV1DiskToBetaDisk(v1Disk *computev1.Disk) *computebeta.Disk { AccessMode: v1Disk.AccessMode, } - // Hyperdisk doesn't currently support multiWriter (https://cloud.google.com/compute/docs/disks/hyperdisks#limitations), - // but if multiWriter + hyperdisk is supported in the future, we want the PDCSI driver to support this feature without - // any additional code change. if v1Disk.ProvisionedIops > 0 { betaDisk.ProvisionedIops = v1Disk.ProvisionedIops } @@ -619,9 +609,6 @@ func convertBetaDiskToV1Disk(betaDisk *computebeta.Disk) *computev1.Disk { AccessMode: betaDisk.AccessMode, } - // Hyperdisk doesn't currently support multiWriter (https://cloud.google.com/compute/docs/disks/hyperdisks#limitations), - // but if multiWriter + hyperdisk is supported in the future, we want the PDCSI driver to support this feature without - // any additional code change. if betaDisk.ProvisionedIops > 0 { v1Disk.ProvisionedIops = betaDisk.ProvisionedIops } @@ -651,7 +638,8 @@ func (cloud *CloudProvider) insertRegionalDisk( gceAPIVersion = GCEAPIVersionV1 ) - if multiWriter { + // Use beta API for non-hyperdisk types in multi-writer mode. + if multiWriter && !strings.Contains(params.DiskType, "hyperdisk") { gceAPIVersion = GCEAPIVersionBeta } @@ -778,7 +766,9 @@ func (cloud *CloudProvider) insertZonalDisk( opName string gceAPIVersion = GCEAPIVersionV1 ) - if multiWriter { + + // Use beta API for non-hyperdisk types in multi-writer mode. + if multiWriter && !strings.Contains(params.DiskType, "hyperdisk") { gceAPIVersion = GCEAPIVersionBeta } diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index decac55a0..b096305dc 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -596,7 +596,7 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi capBytes, _ := getRequestCapacity(capacityRange) multiWriter, _ := getMultiWriterFromCapabilities(req.GetVolumeCapabilities()) readonly, _ := getReadOnlyFromCapabilities(req.GetVolumeCapabilities()) - accessMode := "" + accessMode := params.AccessMode if readonly && slices.Contains(disksWithModifiableAccessMode, params.DiskType) { accessMode = readOnlyManyAccessMode } diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index c77c7b911..65ad912cc 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -904,8 +904,7 @@ var _ = Describe("GCE PD CSI Driver", func() { Expect(err).To(BeNil(), "Failed to go through volume lifecycle") }) - // Pending while multi-writer feature is in Alpha - PIt("Should create and delete multi-writer disk", func() { + It("Should create and delete multi-writer disk", func() { Expect(testContexts).ToNot(BeEmpty()) testContext := getRandomTestContext() @@ -916,7 +915,7 @@ var _ = Describe("GCE PD CSI Driver", func() { zone := "us-east1-a" // Create and Validate Disk - volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, zone, standardDiskType) + volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, zone, hdbDiskType) defer func() { // Delete Disk @@ -929,8 +928,7 @@ var _ = Describe("GCE PD CSI Driver", func() { }() }) - // Pending while multi-writer feature is in Alpha - PIt("Should complete entire disk lifecycle with multi-writer disk", func() { + It("Should complete entire disk lifecycle with multi-writer disk", func() { testContext := getRandomTestContext() p, z, _ := testContext.Instance.GetIdentity() @@ -938,7 +936,7 @@ var _ = Describe("GCE PD CSI Driver", func() { instance := testContext.Instance // Create and Validate Disk - volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, z, standardDiskType) + volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, z, hdbDiskType) defer func() { // Delete Disk @@ -1710,6 +1708,8 @@ func deleteVolumeOrError(client *remote.CsiClient, volID string) { func createAndValidateUniqueZonalMultiWriterDisk(client *remote.CsiClient, project, zone string, diskType string) (string, string) { // Create Disk disk := typeToDisk[diskType] + disk.params.AccessMode = "READ_WRITE_MANY" + volName := testNamePrefix + string(uuid.NewUUID()) volume, err := client.CreateVolumeWithCaps(volName, disk.params, defaultMwSizeGb, &csi.TopologyRequirement{ From c85f509207f89231dcf1332b4de78adfbbb2608d Mon Sep 17 00:00:00 2001 From: Sam Serdlow Date: Fri, 10 Jan 2025 15:29:49 +0000 Subject: [PATCH 2/9] Added fixes for test key handling. --- pkg/common/parameters.go | 8 +++++++- test/e2e/tests/single_zone_e2e_test.go | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index 837b52cee..bae7b65f4 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -22,6 +22,9 @@ import ( ) const ( + // Disk Params + ParameterAccessMode = "access-mode" + // Parameters for StorageClass ParameterKeyType = "type" ParameterKeyReplicationType = "replication-type" @@ -151,7 +154,6 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string] Tags: make(map[string]string), // Default Labels: make(map[string]string), // Default ResourceTags: make(map[string]string), // Default - AccessMode: "READ_WRITE_SINGLE", // Default } for k, v := range extraVolumeLabels { @@ -254,6 +256,10 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string] if paramEnableMultiZoneProvisioning { p.Labels[MultiZoneLabel] = "true" } + case ParameterAccessMode: + if v != "" { + p.AccessMode = v + } default: return p, fmt.Errorf("parameters contains invalid option %q", k) } diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index 65ad912cc..802099150 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -1708,7 +1708,8 @@ func deleteVolumeOrError(client *remote.CsiClient, volID string) { func createAndValidateUniqueZonalMultiWriterDisk(client *remote.CsiClient, project, zone string, diskType string) (string, string) { // Create Disk disk := typeToDisk[diskType] - disk.params.AccessMode = "READ_WRITE_MANY" + + disk.params[common.ParameterAccessMode] = "READ_WRITE_MANY" volName := testNamePrefix + string(uuid.NewUUID()) volume, err := client.CreateVolumeWithCaps(volName, disk.params, defaultMwSizeGb, From 615b44f6e8433938861479740c9bac938c7699f1 Mon Sep 17 00:00:00 2001 From: sjswerdlow <109298351+sjswerdlow@users.noreply.github.com> Date: Mon, 13 Jan 2025 15:29:50 -0500 Subject: [PATCH 3/9] Multiwriter Test Update (#3) * Changes update the tests to use two contexts, one for multiwriter and one for the existing tests. This was deemed necessary as only some disks can support multi-writer, and only some VM shapes can support said disks. --- test/e2e/tests/setup_e2e_test.go | 56 ++++++++++++++++++-------- test/e2e/tests/single_zone_e2e_test.go | 22 ++++------ 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/test/e2e/tests/setup_e2e_test.go b/test/e2e/tests/setup_e2e_test.go index 2e1c8cf51..f7a271a42 100644 --- a/test/e2e/tests/setup_e2e_test.go +++ b/test/e2e/tests/setup_e2e_test.go @@ -36,13 +36,17 @@ import ( ) var ( - project = flag.String("project", "", "Project to run tests in") - serviceAccount = flag.String("service-account", "", "Service account to bring up instance with") - vmNamePrefix = flag.String("vm-name-prefix", "gce-pd-csi-e2e", "VM name prefix") - architecture = flag.String("arch", "amd64", "Architecture pd csi driver build on") - minCpuPlatform = flag.String("min-cpu-platform", "AMD Milan", "Minimum CPU architecture") - zones = flag.String("zones", "us-east4-a,us-east4-c", "Zones to run tests in. If there are multiple zones, separate each by comma") - machineType = flag.String("machine-type", "n2d-standard-2", "Type of machine to provision instance on") + project = flag.String("project", "", "Project to run tests in") + serviceAccount = flag.String("service-account", "", "Service account to bring up instance with") + vmNamePrefix = flag.String("vm-name-prefix", "gce-pd-csi-e2e", "VM name prefix") + architecture = flag.String("arch", "amd64", "Architecture pd csi driver build on") + minCpuPlatform = flag.String("min-cpu-platform", "rome", "Minimum CPU architecture") + mwMinCpuPlatform = flag.String("min-cpu-platform-mw", "sapphirerapids", "Minimum CPU architecture for multiwriter tests") + zones = flag.String("zones", "us-east4-a,us-east4-c", "Zones to run tests in. If there are multiple zones, separate each by comma") + machineType = flag.String("machine-type", "n2d-standard-4", "Type of machine to provision instance on") + // Multi-writer is only supported on M3, C3, and N4 + // https://cloud.google.com/compute/docs/disks/sharing-disks-between-vms#hd-multi-writer + mwMachineType = flag.String("mw-machine-type", "c3-standard-4", "Type of machine to provision instance for multiwriter tests") imageURL = flag.String("image-url", "projects/ubuntu-os-cloud/global/images/family/ubuntu-minimal-2404-lts-amd64", "OS image url to get image from") runInProw = flag.Bool("run-in-prow", false, "If true, use a Boskos loaned project and special CI service accounts and ssh keys") deleteInstances = flag.Bool("delete-instances", false, "Delete the instances after tests run") @@ -50,11 +54,12 @@ var ( extraDriverFlags = flag.String("extra-driver-flags", "", "Extra flags to pass to the driver") enableConfidentialCompute = flag.Bool("enable-confidential-compute", false, "Create VMs with confidential compute mode. This uses NVMe devices") - testContexts = []*remote.TestContext{} - computeService *compute.Service - computeAlphaService *computealpha.Service - computeBetaService *computebeta.Service - kmsClient *cloudkms.KeyManagementClient + testContexts = []*remote.TestContext{} + multiWriterTestContexts = []*remote.TestContext{} + computeService *compute.Service + computeAlphaService *computealpha.Service + computeBetaService *computebeta.Service + kmsClient *cloudkms.KeyManagementClient ) func init() { @@ -70,7 +75,9 @@ func TestE2E(t *testing.T) { var _ = BeforeSuite(func() { var err error tcc := make(chan *remote.TestContext) + mwTcc := make(chan *remote.TestContext) defer close(tcc) + defer close(mwTcc) zones := strings.Split(*zones, ",") @@ -101,13 +108,16 @@ var _ = BeforeSuite(func() { for _, zone := range zones { go func(curZone string) { defer GinkgoRecover() - tcc <- NewTestContext(curZone) + tcc <- NewTestContext(curZone, *machineType, *minCpuPlatform) + mwTcc <- NewTestContext(curZone, *mwMachineType, *mwMinCpuPlatform) }(zone) } for i := 0; i < len(zones); i++ { tc := <-tcc testContexts = append(testContexts, tc) + mwTc := <-mwTcc + multiWriterTestContexts = append(multiWriterTestContexts, mwTc) klog.Infof("Added TestContext for node %s", tc.Instance.GetName()) } }) @@ -120,6 +130,13 @@ var _ = AfterSuite(func() { tc.Instance.DeleteInstance() } } + for _, mwTc := range multiWriterTestContexts { + err := remote.TeardownDriverAndClient(mwTc) + Expect(err).To(BeNil(), "Multiwriter Teardown Driver and Client failed with error") + if *deleteInstances { + mwTc.Instance.DeleteInstance() + } + } }) func notEmpty(v string) bool { @@ -133,17 +150,17 @@ func getDriverConfig() testutils.DriverConfig { } } -func NewTestContext(zone string) *remote.TestContext { - nodeID := fmt.Sprintf("%s-%s", *vmNamePrefix, zone) +func NewTestContext(zone string, machineType string, minCpuPlatform string) *remote.TestContext { + nodeID := fmt.Sprintf("%s-%s-%s", *vmNamePrefix, zone, machineType) klog.Infof("Setting up node %s", nodeID) instanceConfig := remote.InstanceConfig{ Project: *project, Architecture: *architecture, - MinCpuPlatform: *minCpuPlatform, + MinCpuPlatform: minCpuPlatform, Zone: zone, Name: nodeID, - MachineType: *machineType, + MachineType: machineType, ServiceAccount: *serviceAccount, ImageURL: *imageURL, CloudtopHost: *cloudtopHost, @@ -185,3 +202,8 @@ func getRandomTestContext() *remote.TestContext { rn := rand.Intn(len(testContexts)) return testContexts[rn] } +func getRandomMwTestContext() *remote.TestContext { + Expect(multiWriterTestContexts).ToNot(BeEmpty()) + rn := rand.Intn(len(multiWriterTestContexts)) + return multiWriterTestContexts[rn] +} diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index 802099150..80450a3d9 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -79,7 +79,6 @@ const ( ) var _ = Describe("GCE PD CSI Driver", func() { - It("Should get reasonable volume limits from nodes with NodeGetInfo", func() { testContext := getRandomTestContext() resp, err := testContext.Client.NodeGetInfo() @@ -283,6 +282,7 @@ var _ = Describe("GCE PD CSI Driver", func() { Expect(err).To(BeNil(), "Could not find disk in correct zone") } }) + // TODO(hime): Enable this test once all release branches contain the fix from PR#1708. // It("Should return InvalidArgument when disk size exceeds limit", func() { // // If this returns a different error code (like Unknown), the error wrapping logic in #1708 has regressed. @@ -906,16 +906,12 @@ var _ = Describe("GCE PD CSI Driver", func() { It("Should create and delete multi-writer disk", func() { Expect(testContexts).ToNot(BeEmpty()) - testContext := getRandomTestContext() + testContext := getRandomMwTestContext() - p, _, _ := testContext.Instance.GetIdentity() + p, z, _ := testContext.Instance.GetIdentity() client := testContext.Client - - // Hardcode to us-east1-a while feature is in alpha - zone := "us-east1-a" - // Create and Validate Disk - volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, zone, hdbDiskType) + volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, z, hdbDiskType) defer func() { // Delete Disk @@ -923,13 +919,13 @@ var _ = Describe("GCE PD CSI Driver", func() { Expect(err).To(BeNil(), "DeleteVolume failed") // Validate Disk Deleted - _, err = computeAlphaService.Disks.Get(p, zone, volName).Do() + _, err = computeService.Disks.Get(p, z, volName).Do() Expect(gce.IsGCEError(err, "notFound")).To(BeTrue(), "Expected disk to not be found") }() }) It("Should complete entire disk lifecycle with multi-writer disk", func() { - testContext := getRandomTestContext() + testContext := getRandomMwTestContext() p, z, _ := testContext.Instance.GetIdentity() client := testContext.Client @@ -1710,7 +1706,6 @@ func createAndValidateUniqueZonalMultiWriterDisk(client *remote.CsiClient, proje disk := typeToDisk[diskType] disk.params[common.ParameterAccessMode] = "READ_WRITE_MANY" - volName := testNamePrefix + string(uuid.NewUUID()) volume, err := client.CreateVolumeWithCaps(volName, disk.params, defaultMwSizeGb, &csi.TopologyRequirement{ @@ -1738,12 +1733,9 @@ func createAndValidateUniqueZonalMultiWriterDisk(client *remote.CsiClient, proje Expect(cloudDisk.Status).To(Equal(readyState)) Expect(cloudDisk.SizeGb).To(Equal(defaultMwSizeGb)) Expect(cloudDisk.Name).To(Equal(volName)) + Expect(cloudDisk.AccessMode).To(Equal("READ_WRITE_MANY")) disk.validate(cloudDisk) - alphaDisk, err := computeAlphaService.Disks.Get(project, zone, volName).Do() - Expect(err).To(BeNil(), "Failed to get cloud disk using alpha API") - Expect(alphaDisk.MultiWriter).To(Equal(true)) - return volName, volume.VolumeId } From c06aea7faf70ad34f928c8faccb8f74ee34329c4 Mon Sep 17 00:00:00 2001 From: sjswerdlow <109298351+sjswerdlow@users.noreply.github.com> Date: Mon, 13 Jan 2025 18:21:30 -0500 Subject: [PATCH 4/9] Update parameters.go Fixing a linting issue. --- pkg/common/parameters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index bae7b65f4..5484885a1 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -23,7 +23,7 @@ import ( const ( // Disk Params - ParameterAccessMode = "access-mode" + ParameterAccessMode = "access-mode" // Parameters for StorageClass ParameterKeyType = "type" From ca945cfa32e1796787b6eb1db878d1d5e4d665e3 Mon Sep 17 00:00:00 2001 From: sjswerdlow <109298351+sjswerdlow@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:34:56 -0500 Subject: [PATCH 5/9] Karkunpavan (#4) * Removing alpha disk from tests. * Changes setup the test to run with hyperdisk extreme. * Updates the disk type back to balanced, as HDX doesn't support multi writer. * Moving over to m1 megamem as thats the only type of machine that can support all needed disk types. * Changes update the tests to use two contexts, one for multiwriter and one for the existing tests. This was deemed necessary as only some disks can support multi-writer, and only some VM shapes can support said disks. * Changes update the tests to use two contexts, one for multiwriter and one for the existing tests. This was deemed necessary as only some disks can support multi-writer, and only some VM shapes can support said disks. * Fixing some git oddness * Fixing some formatting. * More formatting fixes. * Hopefully last changes for formatting. * Fixing linting issue. * Cleaning up un-used / un-needed GetMultiWriter test function. --- pkg/gce-cloud-provider/compute/cloud-disk.go | 13 ------------- pkg/gce-cloud-provider/compute/fake-gce.go | 2 +- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/pkg/gce-cloud-provider/compute/cloud-disk.go b/pkg/gce-cloud-provider/compute/cloud-disk.go index 71ed6490b..c4e60b2e0 100644 --- a/pkg/gce-cloud-provider/compute/cloud-disk.go +++ b/pkg/gce-cloud-provider/compute/cloud-disk.go @@ -213,19 +213,6 @@ func (d *CloudDisk) GetKMSKeyName() string { return "" } -func (d *CloudDisk) GetMultiWriter() bool { - switch { - case d.disk != nil: - return false - case d.disk != nil && d.disk.AccessMode == "READ_WRITE_MANY": - return true - case d.betaDisk != nil: - return d.betaDisk.MultiWriter - default: - return false - } -} - func (d *CloudDisk) GetEnableConfidentialCompute() bool { switch { case d.disk != nil: diff --git a/pkg/gce-cloud-provider/compute/fake-gce.go b/pkg/gce-cloud-provider/compute/fake-gce.go index f2289a398..fee1e9fa8 100644 --- a/pkg/gce-cloud-provider/compute/fake-gce.go +++ b/pkg/gce-cloud-provider/compute/fake-gce.go @@ -213,7 +213,7 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp * } // We are assuming here that a multiWriter disk could be used as non-multiWriter - if multiWriter && !resp.GetMultiWriter() { + if multiWriter { return fmt.Errorf("disk already exists with incompatible capability. Need MultiWriter. Got non-MultiWriter") } From 81ce104d08f564f84fb7cf95c0db18f1c1c87724 Mon Sep 17 00:00:00 2001 From: Sam Serdlow Date: Thu, 23 Jan 2025 17:03:58 +0000 Subject: [PATCH 6/9] Completley removing the multiwriter check from fake-gce. --- pkg/gce-cloud-provider/compute/fake-gce.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/gce-cloud-provider/compute/fake-gce.go b/pkg/gce-cloud-provider/compute/fake-gce.go index fee1e9fa8..f9b646b64 100644 --- a/pkg/gce-cloud-provider/compute/fake-gce.go +++ b/pkg/gce-cloud-provider/compute/fake-gce.go @@ -212,11 +212,6 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp * params.DiskType, respType[len(respType)-1]) } - // We are assuming here that a multiWriter disk could be used as non-multiWriter - if multiWriter { - return fmt.Errorf("disk already exists with incompatible capability. Need MultiWriter. Got non-MultiWriter") - } - klog.V(4).Infof("Compatible disk already exists") return ValidateDiskParameters(resp, params) } From a062e8941c72de5206df491167fb6eb715a38e76 Mon Sep 17 00:00:00 2001 From: sjswerdlow <109298351+sjswerdlow@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:19:10 -0500 Subject: [PATCH 7/9] Changes remove the API version concept from GetDisk and InsertDisk, now just defaults to using the Beta API. (#5) --- pkg/gce-cloud-provider/compute/fake-gce.go | 2 +- pkg/gce-cloud-provider/compute/gce-compute.go | 107 ++++++------------ pkg/gce-pd-csi-driver/controller.go | 24 ++-- pkg/gce-pd-csi-driver/controller_test.go | 8 +- 4 files changed, 51 insertions(+), 90 deletions(-) diff --git a/pkg/gce-cloud-provider/compute/fake-gce.go b/pkg/gce-cloud-provider/compute/fake-gce.go index f9b646b64..0ffcd39e9 100644 --- a/pkg/gce-cloud-provider/compute/fake-gce.go +++ b/pkg/gce-cloud-provider/compute/fake-gce.go @@ -184,7 +184,7 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string } // Disk Methods -func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) { +func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key) (*CloudDisk, error) { disk, ok := cloud.disks[volKey.String()] if !ok { return nil, notFoundError() diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index 940d9d86f..b493a3962 100644 --- a/pkg/gce-cloud-provider/compute/gce-compute.go +++ b/pkg/gce-cloud-provider/compute/gce-compute.go @@ -99,7 +99,7 @@ type GCECompute interface { GetDefaultProject() string GetDefaultZone() string // Disk Methods - GetDisk(ctx context.Context, project string, volumeKey *meta.Key, gceAPIVersion GCEAPIVersion) (*CloudDisk, error) + GetDisk(ctx context.Context, project string, volumeKey *meta.Key) (*CloudDisk, error) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) ValidateExistingDisk(ctx context.Context, disk *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64, multiWriter bool) error InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, volumeContentSourceVolumeID string, multiWriter bool, accessMode string) error @@ -321,26 +321,16 @@ func (cloud *CloudProvider) ListSnapshots(ctx context.Context, filter string) ([ return items, "", nil } -func (cloud *CloudProvider) GetDisk(ctx context.Context, project string, key *meta.Key, gceAPIVersion GCEAPIVersion) (*CloudDisk, error) { +func (cloud *CloudProvider) GetDisk(ctx context.Context, project string, key *meta.Key) (*CloudDisk, error) { klog.V(5).Infof("Getting disk %v", key) switch key.Type() { case meta.Zonal: - if gceAPIVersion == GCEAPIVersionBeta { - disk, err := cloud.getZonalBetaDiskOrError(ctx, project, key.Zone, key.Name) - return CloudDiskFromBeta(disk), err - } else { - disk, err := cloud.getZonalDiskOrError(ctx, project, key.Zone, key.Name) - return CloudDiskFromV1(disk), err - } + disk, err := cloud.getZonalBetaDiskOrError(ctx, project, key.Zone, key.Name) + return CloudDiskFromBeta(disk), err case meta.Regional: - if gceAPIVersion == GCEAPIVersionBeta { - disk, err := cloud.getRegionalBetaDiskOrError(ctx, project, key.Region, key.Name) - return CloudDiskFromBeta(disk), err - } else { - disk, err := cloud.getRegionalDiskOrError(ctx, project, key.Region, key.Name) - return CloudDiskFromV1(disk), err - } + disk, err := cloud.getRegionalBetaDiskOrError(ctx, project, key.Region, key.Name) + return CloudDiskFromBeta(disk), err default: return nil, fmt.Errorf("key was neither zonal nor regional, got: %v", key.String()) } @@ -633,17 +623,11 @@ func (cloud *CloudProvider) insertRegionalDisk( description string, multiWriter bool) error { var ( - err error - opName string - gceAPIVersion = GCEAPIVersionV1 + err error + opName string ) - // Use beta API for non-hyperdisk types in multi-writer mode. - if multiWriter && !strings.Contains(params.DiskType, "hyperdisk") { - gceAPIVersion = GCEAPIVersionBeta - } - - diskToCreate := &computev1.Disk{ + diskToCreate := &computebeta.Disk{ Name: volKey.Name, SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, @@ -672,7 +656,7 @@ func (cloud *CloudProvider) insertRegionalDisk( diskToCreate.ReplicaZones = replicaZones } if params.DiskEncryptionKMSKey != "" { - diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{ + diskToCreate.DiskEncryptionKey = &computebeta.CustomerEncryptionKey{ KmsKeyName: params.DiskEncryptionKMSKey, } } @@ -682,29 +666,21 @@ func (cloud *CloudProvider) insertRegionalDisk( } if len(resourceTags) > 0 { - diskToCreate.Params = &computev1.DiskParams{ + diskToCreate.Params = &computebeta.DiskParams{ ResourceManagerTags: resourceTags, } } - if gceAPIVersion == GCEAPIVersionBeta { - var insertOp *computebeta.Operation - betaDiskToCreate := convertV1DiskToBetaDisk(diskToCreate) - betaDiskToCreate.MultiWriter = multiWriter - insertOp, err = cloud.betaService.RegionDisks.Insert(project, volKey.Region, betaDiskToCreate).Context(ctx).Do() - if insertOp != nil { - opName = insertOp.Name - } - } else { - var insertOp *computev1.Operation - insertOp, err = cloud.service.RegionDisks.Insert(project, volKey.Region, diskToCreate).Context(ctx).Do() - if insertOp != nil { - opName = insertOp.Name - } + var insertOp *computebeta.Operation + diskToCreate.MultiWriter = multiWriter + insertOp, err = cloud.betaService.RegionDisks.Insert(project, volKey.Region, diskToCreate).Context(ctx).Do() + if insertOp != nil { + opName = insertOp.Name } + if err != nil { if IsGCEError(err, "alreadyExists") { - disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion) + disk, err := cloud.GetDisk(ctx, project, volKey) if err != nil { // failed to GetDisk, however the Disk may already exist // the error code should be non-Final @@ -730,7 +706,7 @@ func (cloud *CloudProvider) insertRegionalDisk( // the error code returned should be non-final if err != nil { if IsGCEError(err, "alreadyExists") { - disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion) + disk, err := cloud.GetDisk(ctx, project, volKey) if err != nil { return common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error when getting disk: %w", err)) } @@ -762,17 +738,11 @@ func (cloud *CloudProvider) insertZonalDisk( multiWriter bool, accessMode string) error { var ( - err error - opName string - gceAPIVersion = GCEAPIVersionV1 + err error + opName string ) - // Use beta API for non-hyperdisk types in multi-writer mode. - if multiWriter && !strings.Contains(params.DiskType, "hyperdisk") { - gceAPIVersion = GCEAPIVersionBeta - } - - diskToCreate := &computev1.Disk{ + diskToCreate := &computebeta.Disk{ Name: volKey.Name, SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, @@ -814,7 +784,7 @@ func (cloud *CloudProvider) insertZonalDisk( } if params.DiskEncryptionKMSKey != "" { - diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{ + diskToCreate.DiskEncryptionKey = &computebeta.CustomerEncryptionKey{ KmsKeyName: params.DiskEncryptionKMSKey, } } @@ -826,31 +796,22 @@ func (cloud *CloudProvider) insertZonalDisk( } if len(resourceTags) > 0 { - diskToCreate.Params = &computev1.DiskParams{ + diskToCreate.Params = &computebeta.DiskParams{ ResourceManagerTags: resourceTags, } } - diskToCreate.AccessMode = accessMode - if gceAPIVersion == GCEAPIVersionBeta { - var insertOp *computebeta.Operation - betaDiskToCreate := convertV1DiskToBetaDisk(diskToCreate) - betaDiskToCreate.MultiWriter = multiWriter - insertOp, err = cloud.betaService.Disks.Insert(project, volKey.Zone, betaDiskToCreate).Context(ctx).Do() - if insertOp != nil { - opName = insertOp.Name - } - } else { - var insertOp *computev1.Operation - insertOp, err = cloud.service.Disks.Insert(project, volKey.Zone, diskToCreate).Context(ctx).Do() - if insertOp != nil { - opName = insertOp.Name - } + diskToCreate.AccessMode = accessMode + var insertOp *computebeta.Operation + diskToCreate.MultiWriter = multiWriter + insertOp, err = cloud.betaService.Disks.Insert(project, volKey.Zone, diskToCreate).Context(ctx).Do() + if insertOp != nil { + opName = insertOp.Name } if err != nil { if IsGCEError(err, "alreadyExists") { - disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion) + disk, err := cloud.GetDisk(ctx, project, volKey) if err != nil { // failed to GetDisk, however the Disk may already exist // the error code should be non-Final @@ -877,7 +838,7 @@ func (cloud *CloudProvider) insertZonalDisk( // failed to wait for Op to finish, however, the Op possibly is still running as expected // the error code returned should be non-final if IsGCEError(err, "alreadyExists") { - disk, err := cloud.GetDisk(ctx, project, volKey, gceAPIVersion) + disk, err := cloud.GetDisk(ctx, project, volKey) if err != nil { return common.NewTemporaryError(codes.Unavailable, fmt.Errorf("error when getting disk: %w", err)) } @@ -1176,7 +1137,7 @@ func (cloud *CloudProvider) waitForAttachOnDisk(ctx context.Context, project str start := time.Now() return wait.ExponentialBackoff(AttachDiskBackoff, func() (bool, error) { klog.V(6).Infof("Polling disks.get for attach of disk %v to instance %v to complete for %v", volKey.Name, instanceName, time.Since(start)) - disk, err := cloud.GetDisk(ctx, project, volKey, GCEAPIVersionV1) + disk, err := cloud.GetDisk(ctx, project, volKey) if err != nil { return false, fmt.Errorf("GetDisk failed to get disk: %w", err) } @@ -1426,7 +1387,7 @@ func (cloud *CloudProvider) DeleteImage(ctx context.Context, project, imageName // k8s.io/apimachinery/quantity package for better size handling func (cloud *CloudProvider) ResizeDisk(ctx context.Context, project string, volKey *meta.Key, requestBytes int64) (int64, error) { klog.V(5).Infof("Resizing disk %v to size %v", volKey, requestBytes) - cloudDisk, err := cloud.GetDisk(ctx, project, volKey, GCEAPIVersionV1) + cloudDisk, err := cloud.GetDisk(ctx, project, volKey) if err != nil { return -1, fmt.Errorf("failed to get disk: %w", err) } diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index b096305dc..6f14a4473 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -602,7 +602,7 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi } // Validate if disk already exists - existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, gceCS.CloudProvider.GetDefaultProject(), volKey, getGCEApiVersion(multiWriter)) + existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, gceCS.CloudProvider.GetDefaultProject(), volKey) if err != nil { if !gce.IsGCEError(err, "notFound") { // failed to GetDisk, however the Disk may already be created, the error code should be non-Final @@ -657,7 +657,7 @@ func (gceCS *GCEControllerServer) createSingleDisk(ctx context.Context, req *csi } // Verify that the volume in VolumeContentSource exists. - diskFromSourceVolume, err := gceCS.CloudProvider.GetDisk(ctx, project, sourceVolKey, getGCEApiVersion(multiWriter)) + diskFromSourceVolume, err := gceCS.CloudProvider.GetDisk(ctx, project, sourceVolKey) if err != nil { if gce.IsGCEError(err, "notFound") { return nil, status.Errorf(codes.NotFound, "CreateVolume source volume %s does not exist", volumeContentSourceVolumeID) @@ -787,7 +787,7 @@ func (gceCS *GCEControllerServer) ControllerModifyVolume(ctx context.Context, re } klog.V(4).Infof("Modify Volume Parameters for %s: %v", volumeID, volumeModifyParams) - existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionBeta) + existingDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey) metrics.UpdateRequestMetadataFromDisk(ctx, existingDisk) if err != nil { @@ -883,7 +883,7 @@ func (gceCS *GCEControllerServer) deleteMultiZoneDisk(ctx context.Context, req * Region: volKey.Region, Zone: zone, } - disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, zonalVolKey, gce.GCEAPIVersionV1) + disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, zonalVolKey) // TODO: Consolidate the parameters here, rather than taking the last. metrics.UpdateRequestMetadataFromDisk(ctx, disk) err := gceCS.CloudProvider.DeleteDisk(ctx, project, zonalVolKey) @@ -916,7 +916,7 @@ func (gceCS *GCEControllerServer) deleteSingleDeviceDisk(ctx context.Context, re return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, volumeID) } defer gceCS.volumeLocks.Release(volumeID) - disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1) + disk, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey) metrics.UpdateRequestMetadataFromDisk(ctx, disk) err = gceCS.CloudProvider.DeleteDisk(ctx, project, volKey) if err != nil { @@ -1085,7 +1085,7 @@ func (gceCS *GCEControllerServer) executeControllerPublishVolume(ctx context.Con return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, lockingVolumeID), nil } defer gceCS.volumeLocks.Release(lockingVolumeID) - disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1) + disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey) if err != nil { if gce.IsGCENotFoundError(err) { return nil, status.Errorf(codes.NotFound, "Could not find disk %v: %v", volKey.String(), err.Error()), disk @@ -1231,7 +1231,7 @@ func (gceCS *GCEControllerServer) executeControllerUnpublishVolume(ctx context.C return nil, status.Errorf(codes.Aborted, common.VolumeOperationAlreadyExistsFmt, lockingVolumeID), nil } defer gceCS.volumeLocks.Release(lockingVolumeID) - diskToUnpublish, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1) + diskToUnpublish, _ := gceCS.CloudProvider.GetDisk(ctx, project, volKey) instance, err := gceCS.CloudProvider.GetInstanceOrError(ctx, instanceZone, instanceName) if err != nil { if gce.IsGCENotFoundError(err) { @@ -1298,7 +1298,7 @@ func (gceCS *GCEControllerServer) ValidateVolumeCapabilities(ctx context.Context } defer gceCS.volumeLocks.Release(volumeID) - disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1) + disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey) metrics.UpdateRequestMetadataFromDisk(ctx, disk) if err != nil { if gce.IsGCENotFoundError(err) { @@ -1539,7 +1539,7 @@ func (gceCS *GCEControllerServer) CreateSnapshot(ctx context.Context, req *csi.C defer gceCS.volumeLocks.Release(volumeID) // Check if volume exists - disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1) + disk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey) metrics.UpdateRequestMetadataFromDisk(ctx, disk) if err != nil { if gce.IsGCENotFoundError(err) { @@ -1881,7 +1881,7 @@ func (gceCS *GCEControllerServer) ControllerExpandVolume(ctx context.Context, re return nil, status.Errorf(codes.InvalidArgument, "ControllerExpandVolume is not supported with the multi-zone PVC volumeHandle feature. Please re-create the volume %v from source if you want a larger size", volumeID) } - sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey, gce.GCEAPIVersionV1) + sourceDisk, err := gceCS.CloudProvider.GetDisk(ctx, project, volKey) metrics.UpdateRequestMetadataFromDisk(ctx, sourceDisk) resizedGb, err := gceCS.CloudProvider.ResizeDisk(ctx, project, volKey, reqBytes) @@ -2437,7 +2437,7 @@ func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name gceAPIVersion = gce.GCEAPIVersionBeta } // failed to GetDisk, however the Disk may already be created, the error code should be non-Final - disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region), gceAPIVersion) + disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region)) if err != nil { return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("failed to get disk after creating regional disk: %w", err)) } @@ -2460,7 +2460,7 @@ func createSingleZoneDisk(ctx context.Context, cloudProvider gce.GCECompute, nam gceAPIVersion = gce.GCEAPIVersionBeta } // failed to GetDisk, however the Disk may already be created, the error code should be non-Final - disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone), gceAPIVersion) + disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone)) if err != nil { return nil, common.NewTemporaryError(codes.Unavailable, fmt.Errorf("failed to get disk after creating zonal disk: %w", err)) } diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go index 06db7222e..d46c0859d 100644 --- a/pkg/gce-pd-csi-driver/controller_test.go +++ b/pkg/gce-pd-csi-driver/controller_test.go @@ -1565,7 +1565,7 @@ func TestMultiZoneVolumeCreation(t *testing.T) { for _, zone := range tc.expZones { volumeKey := meta.ZonalKey(name, zone) - disk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta) + disk, err := fcp.GetDisk(context.Background(), project, volumeKey) if err != nil { t.Fatalf("Get Disk failed for created disk with error: %v", err) } @@ -1859,7 +1859,7 @@ func TestCreateVolumeWithVolumeAttributeClassParameters(t *testing.T) { t.Fatalf("Failed to convert volume id to key: %v", err) } - disk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta) + disk, err := fcp.GetDisk(context.Background(), project, volumeKey) if err != nil { t.Fatalf("Failed to get disk: %v", err) @@ -1964,7 +1964,7 @@ func TestVolumeModifyOperation(t *testing.T) { } } - modifiedVol, err := fcp.GetDisk(context.Background(), project, volKey, gce.GCEAPIVersionBeta) + modifiedVol, err := fcp.GetDisk(context.Background(), project, volKey) if err != nil { t.Errorf("Failed to get volume: %v", err) @@ -5241,7 +5241,7 @@ func TestCreateConfidentialVolume(t *testing.T) { volumeId := resp.GetVolume().VolumeId project, volumeKey, err := common.VolumeIDToKey(volumeId) - createdDisk, err := fcp.GetDisk(context.Background(), project, volumeKey, gce.GCEAPIVersionBeta) + createdDisk, err := fcp.GetDisk(context.Background(), project, volumeKey) if err != nil { t.Fatalf("Get Disk failed for created disk with error: %v", err) } From a64c11fd2485588125ea6a1cb81bcdc9f7f2b969 Mon Sep 17 00:00:00 2001 From: sjswerdlow <109298351+sjswerdlow@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:32:03 -0500 Subject: [PATCH 8/9] Followup (#6) * Changes remove the API version concept from GetDisk and InsertDisk, now just defaults to using the Beta API. * Cleaning up un-used variables. --- pkg/gce-pd-csi-driver/controller.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/gce-pd-csi-driver/controller.go b/pkg/gce-pd-csi-driver/controller.go index 6f14a4473..8fe595981 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -2432,10 +2432,6 @@ func createRegionalDisk(ctx context.Context, cloudProvider gce.GCECompute, name return nil, fmt.Errorf("failed to insert regional disk: %w", err) } - gceAPIVersion := gce.GCEAPIVersionV1 - if multiWriter { - gceAPIVersion = gce.GCEAPIVersionBeta - } // failed to GetDisk, however the Disk may already be created, the error code should be non-Final disk, err := cloudProvider.GetDisk(ctx, project, meta.RegionalKey(name, region)) if err != nil { @@ -2455,10 +2451,6 @@ func createSingleZoneDisk(ctx context.Context, cloudProvider gce.GCECompute, nam return nil, fmt.Errorf("failed to insert zonal disk: %w", err) } - gceAPIVersion := gce.GCEAPIVersionV1 - if multiWriter { - gceAPIVersion = gce.GCEAPIVersionBeta - } // failed to GetDisk, however the Disk may already be created, the error code should be non-Final disk, err := cloudProvider.GetDisk(ctx, project, meta.ZonalKey(name, diskZone)) if err != nil { From ed9ebec65108c27d6eb3a8adf44bd999196ad66e Mon Sep 17 00:00:00 2001 From: sjswerdlow <109298351+sjswerdlow@users.noreply.github.com> Date: Thu, 6 Feb 2025 16:53:40 -0500 Subject: [PATCH 9/9] Followup (#7) * Changes remove the API version concept from GetDisk and InsertDisk, now just defaults to using the Beta API. * Cleaning up un-used variables. * pkg/gce-cloud-provider/compute/fake-gce.go --- pkg/common/utils_test.go | 2 +- pkg/gce-pd-csi-driver/controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go index 0df0ed1d1..554494e2e 100644 --- a/pkg/common/utils_test.go +++ b/pkg/common/utils_test.go @@ -1409,7 +1409,7 @@ func TestIsUserMultiAttachError(t *testing.T) { }, } for _, test := range cases { - code, err := isUserMultiAttachError(fmt.Errorf(test.errorString)) + code, err := isUserMultiAttachError(fmt.Errorf("%s", test.errorString)) if test.expectCode { if err != nil || code != test.expectedCode { t.Errorf("Failed with non-nil error %v or bad code %v: %s", err, code, test.errorString) diff --git a/pkg/gce-pd-csi-driver/controller_test.go b/pkg/gce-pd-csi-driver/controller_test.go index d46c0859d..d4eb0b666 100644 --- a/pkg/gce-pd-csi-driver/controller_test.go +++ b/pkg/gce-pd-csi-driver/controller_test.go @@ -1741,7 +1741,7 @@ func TestMultiZoneVolumeCreationErrHandling(t *testing.T) { } for _, volKey := range tc.wantDisks { - disk, err := fcp.GetDisk(context.Background(), project, volKey, gce.GCEAPIVersionV1) + disk, err := fcp.GetDisk(context.Background(), project, volKey) if err != nil { t.Errorf("Unexpected err fetching disk %v: %v", volKey, err) }