From 5789a076f72723fda662497caa30f7b04e54c0ea Mon Sep 17 00:00:00 2001 From: karkunpavan Date: Fri, 10 Jan 2025 12:56:38 +0000 Subject: [PATCH] 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{