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" 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") } 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 }