diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index f41c32336..e88d876b8 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" @@ -107,6 +110,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 } func (dp *DiskParameters) IsRegional() bool { @@ -262,6 +268,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/pkg/gce-cloud-provider/compute/fake-gce.go b/pkg/gce-cloud-provider/compute/fake-gce.go index 2f00ea6e2..ad517fca0 100644 --- a/pkg/gce-cloud-provider/compute/fake-gce.go +++ b/pkg/gce-cloud-provider/compute/fake-gce.go @@ -188,7 +188,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() @@ -220,7 +220,6 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp * if multiWriter && !resp.GetMultiWriter() { 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) } diff --git a/pkg/gce-cloud-provider/compute/gce-compute.go b/pkg/gce-cloud-provider/compute/gce-compute.go index e2a753b22..191da8940 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,28 +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) - // 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 { - 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()) } @@ -553,9 +541,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 +604,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 } @@ -646,16 +628,11 @@ func (cloud *CloudProvider) insertRegionalDisk( description string, multiWriter bool) error { var ( - err error - opName string - gceAPIVersion = GCEAPIVersionV1 + err error + opName string ) - if multiWriter { - gceAPIVersion = GCEAPIVersionBeta - } - - diskToCreate := &computev1.Disk{ + diskToCreate := &computebeta.Disk{ Name: volKey.Name, SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, @@ -684,7 +661,7 @@ func (cloud *CloudProvider) insertRegionalDisk( diskToCreate.ReplicaZones = replicaZones } if params.DiskEncryptionKMSKey != "" { - diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{ + diskToCreate.DiskEncryptionKey = &computebeta.CustomerEncryptionKey{ KmsKeyName: params.DiskEncryptionKMSKey, } } @@ -694,29 +671,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 @@ -742,7 +711,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)) } @@ -774,15 +743,11 @@ func (cloud *CloudProvider) insertZonalDisk( multiWriter bool, accessMode string) error { var ( - err error - opName string - gceAPIVersion = GCEAPIVersionV1 + err error + opName string ) - if multiWriter { - gceAPIVersion = GCEAPIVersionBeta - } - diskToCreate := &computev1.Disk{ + diskToCreate := &computebeta.Disk{ Name: volKey.Name, SizeGb: common.BytesToGbRoundUp(capBytes), Description: description, @@ -824,7 +789,7 @@ func (cloud *CloudProvider) insertZonalDisk( } if params.DiskEncryptionKMSKey != "" { - diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{ + diskToCreate.DiskEncryptionKey = &computebeta.CustomerEncryptionKey{ KmsKeyName: params.DiskEncryptionKMSKey, } } @@ -836,31 +801,21 @@ 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 + 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 @@ -887,7 +842,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)) } @@ -1186,7 +1141,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) } @@ -1436,7 +1391,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 117df6413..3bcee864a 100644 --- a/pkg/gce-pd-csi-driver/controller.go +++ b/pkg/gce-pd-csi-driver/controller.go @@ -598,13 +598,13 @@ 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 = gceReadOnlyManyAccessMode } // 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 @@ -659,7 +659,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) @@ -788,7 +788,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 { @@ -884,7 +884,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) @@ -917,7 +917,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 { @@ -1086,7 +1086,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 @@ -1232,7 +1232,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) { @@ -1300,7 +1300,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) { @@ -1541,7 +1541,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) { @@ -1890,7 +1890,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) @@ -2440,12 +2440,8 @@ 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), 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)) } @@ -2463,12 +2459,8 @@ 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), 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 165fc7f9b..80871c77e 100644 --- a/pkg/gce-pd-csi-driver/controller_test.go +++ b/pkg/gce-pd-csi-driver/controller_test.go @@ -1702,7 +1702,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) } @@ -1878,7 +1878,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) } @@ -1996,7 +1996,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) @@ -2101,7 +2101,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) @@ -5378,7 +5378,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) } diff --git a/test/e2e/tests/setup_e2e_test.go b/test/e2e/tests/setup_e2e_test.go index ebe1a920a..3bc738a2b 100644 --- a/test/e2e/tests/setup_e2e_test.go +++ b/test/e2e/tests/setup_e2e_test.go @@ -40,17 +40,20 @@ var ( 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") + 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-2", "Type of machine to provision instance on") + machineType = flag.String("machine-type", "n2d-standard-4", "Type of machine to provision instance on") 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") cloudtopHost = flag.Bool("cloudtop-host", false, "The local host is cloudtop, a kind of googler machine with special requirements to access GCP") 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") - hdMachineType = flag.String("hyperdisk-machine-type", "c3-standard-4", "Type of machine to provision instance on") - hdMinCpuPlatform = flag.String("hyperdisk-min-cpu-platform", "sapphirerapids", "Minimum CPU architecture") + // Multi-writer is only supported on M3, C3, and N4 + // https://cloud.google.com/compute/docs/disks/sharing-disks-between-vms#hd-multi-writer + hdMachineType = flag.String("hyperdisk-machine-type", "c3-standard-4", "Type of machine to provision instance on") + hdMinCpuPlatform = flag.String("hyperdisk-min-cpu-platform", "sapphirerapids", "Minimum CPU architecture") testContexts = []*remote.TestContext{} hyperdiskTestContexts = []*remote.TestContext{} @@ -132,6 +135,13 @@ var _ = AfterSuite(func() { tc.Instance.DeleteInstance() } } + for _, mwTc := range hyperdiskTestContexts { + 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 { @@ -201,3 +211,8 @@ func getRandomTestContext() *remote.TestContext { rn := rand.Intn(len(testContexts)) return testContexts[rn] } +func getRandomMwTestContext() *remote.TestContext { + Expect(hyperdiskTestContexts).ToNot(BeEmpty()) + rn := rand.Intn(len(hyperdiskTestContexts)) + return hyperdiskTestContexts[rn] +} diff --git a/test/e2e/tests/single_zone_e2e_test.go b/test/e2e/tests/single_zone_e2e_test.go index da83ad0b5..455f5f66e 100644 --- a/test/e2e/tests/single_zone_e2e_test.go +++ b/test/e2e/tests/single_zone_e2e_test.go @@ -80,7 +80,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() @@ -905,19 +904,14 @@ 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() + 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, standardDiskType) + volName, volID := createAndValidateUniqueZonalMultiWriterDisk(client, p, z, hdbDiskType) defer func() { // Delete Disk @@ -925,21 +919,20 @@ 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") }() }) - // Pending while multi-writer feature is in Alpha - PIt("Should complete entire disk lifecycle with multi-writer disk", func() { - testContext := getRandomTestContext() + It("Should complete entire disk lifecycle with multi-writer disk", func() { + testContext := getRandomMwTestContext() p, z, _ := testContext.Instance.GetIdentity() client := testContext.Client 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 @@ -1711,6 +1704,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[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 }