Skip to content

Commit 4d6be71

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1732 from pwschuurman/add-extra-flags-param
Improvements to e2e test suite runner
2 parents 6d2a1b2 + 9a5b915 commit 4d6be71

File tree

4 files changed

+90
-28
lines changed

4 files changed

+90
-28
lines changed

‎test/e2e/tests/multi_zone_e2e_test.go

+48-12
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,11 @@ var _ = Describe("GCE PD CSI Driver Multi-Zone", func() {
325325

326326
// Attach disk to instance in the first zone.
327327
tc0 := zoneToContext[zones[0]]
328-
err, detacher, args := testAttachAndMount(volume.VolumeId, volName, tc0.Instance, tc0.Client, false /* useBlock */, false /* forceAttach */)
328+
err, detacher, args := testAttachAndMount(volume.VolumeId, volName, tc0.Instance, tc0.Client, attachAndMountArgs{
329+
readOnly: false,
330+
useBlock: false,
331+
forceAttach: false,
332+
})
329333
detachers = append(detachers, detacher)
330334
Expect(err).To(BeNil(), "failed attach in zone 0")
331335
testFileName := filepath.Join(args.publishDir, "force-attach-test")
@@ -341,7 +345,11 @@ var _ = Describe("GCE PD CSI Driver Multi-Zone", func() {
341345

342346
// Now force attach to the second instance without detaching.
343347
tc1 := zoneToContext[zones[1]]
344-
err, detacher, args = testAttachAndMount(volume.VolumeId, volName, tc1.Instance, tc1.Client, false /* useBlock */, true /* forceAttach */)
348+
err, detacher, _ = testAttachAndMount(volume.VolumeId, volName, tc1.Instance, tc1.Client, attachAndMountArgs{
349+
readOnly: false,
350+
useBlock: false,
351+
forceAttach: true,
352+
})
345353
detachers = append(detachers, detacher)
346354
Expect(err).To(BeNil(), "failed force attach in zone 1")
347355
readContents, err = testutils.ReadFile(tc1.Instance, testFileName)
@@ -389,9 +397,20 @@ func testAttachWriteReadDetach(volID string, volName string, instance *remote.In
389397
return testLifecycleWithVerify(volID, volName, instance, client, readOnly, false /* fs */, writeFile, verifyReadFile)
390398
}
391399

392-
func testAttachAndMount(volID string, volName string, instance *remote.InstanceInfo, client *remote.CsiClient, useBlock, forceAttach bool) (error, func(), *verifyArgs) {
400+
type attachAndMountArgs struct {
401+
readOnly bool
402+
useBlock bool
403+
forceAttach bool
404+
}
405+
406+
func testAttachAndMount(volID string, volName string, instance *remote.InstanceInfo, client *remote.CsiClient, args attachAndMountArgs) (error, func(), *verifyArgs) {
393407
// Attach Disk
394-
err := client.ControllerPublishVolumeReadWrite(volID, instance.GetNodeID(), forceAttach)
408+
var err error
409+
if args.readOnly {
410+
err = client.ControllerPublishVolumeReadOnly(volID, instance.GetNodeID())
411+
} else {
412+
err = client.ControllerPublishVolumeReadWrite(volID, instance.GetNodeID(), args.forceAttach)
413+
}
395414
if err != nil {
396415
return fmt.Errorf("ControllerPublishVolume failed with error for disk %v on node %v: %v", volID, instance.GetNodeID(), err.Error()), nil, nil
397416
}
@@ -406,7 +425,7 @@ func testAttachAndMount(volID string, volName string, instance *remote.InstanceI
406425

407426
// Stage Disk
408427
stageDir := filepath.Join("/tmp/", volName, "stage")
409-
if useBlock {
428+
if args.useBlock {
410429
err = client.NodeStageBlockVolume(volID, stageDir)
411430
} else {
412431
err = client.NodeStageExt4Volume(volID, stageDir)
@@ -435,7 +454,7 @@ func testAttachAndMount(volID string, volName string, instance *remote.InstanceI
435454
// Mount Disk
436455
publishDir := filepath.Join("/tmp/", volName, "mount")
437456

438-
if useBlock {
457+
if args.useBlock {
439458
err = client.NodePublishBlockVolume(volID, stageDir, publishDir)
440459
} else {
441460
err = client.NodePublishVolume(volID, stageDir, publishDir)
@@ -445,23 +464,40 @@ func testAttachAndMount(volID string, volName string, instance *remote.InstanceI
445464
unstageAndDetach()
446465
return fmt.Errorf("NodePublishVolume failed with error: %v", err.Error()), nil, nil
447466
}
448-
err = testutils.ForceChmod(instance, filepath.Join("/tmp/", volName), "777")
449-
if err != nil {
467+
468+
unpublish := func() {
469+
// Unpublish Disk
470+
err = client.NodeUnpublishVolume(volID, publishDir)
471+
if err != nil {
472+
klog.Errorf("Failed to unpublish volume: %v", err)
473+
}
474+
}
475+
unpublishUnstageAndDetach := func() {
476+
unpublish()
450477
unstageAndDetach()
478+
}
479+
480+
err = testutils.ForceChmod(instance, filepath.Join("/tmp/", volName), "777", !args.readOnly /* recursive */)
481+
if err != nil {
482+
unpublishUnstageAndDetach()
451483
return fmt.Errorf("Chmod failed with error: %v", err.Error()), nil, nil
452484
}
453485

454-
args := &verifyArgs{
486+
returnArgs := &verifyArgs{
455487
publishDir: publishDir,
456488
stageDir: stageDir,
457489
}
458490

459-
return nil, unstageAndDetach, args
491+
return nil, unpublishUnstageAndDetach, returnArgs
460492
}
461493

462494
func testLifecycleWithVerify(volID string, volName string, instance *remote.InstanceInfo, client *remote.CsiClient, readOnly, useBlock bool, firstMountVerify, secondMountVerify verifyFunc) error {
463495
klog.Infof("Starting testAttachWriteReadDetach with volume %v node %v with readonly %v\n", volID, instance.GetNodeID(), readOnly)
464-
err, detacher, args := testAttachAndMount(volID, volName, instance, client, useBlock, false /* forceAttach */)
496+
err, detacher, args := testAttachAndMount(volID, volName, instance, client, attachAndMountArgs{
497+
readOnly: readOnly,
498+
useBlock: useBlock,
499+
forceAttach: false,
500+
})
465501
if err != nil {
466502
return fmt.Errorf("failed to attach and mount: %w", err)
467503
}
@@ -489,7 +525,7 @@ func testLifecycleWithVerify(volID string, volName string, instance *remote.Inst
489525
if err != nil {
490526
return fmt.Errorf("NodePublishVolume failed with error: %v", err.Error())
491527
}
492-
err = testutils.ForceChmod(instance, filepath.Join("/tmp/", volName), "777")
528+
err = testutils.ForceChmod(instance, filepath.Join("/tmp/", volName), "777", !readOnly /* recursive */)
493529
if err != nil {
494530
return fmt.Errorf("Chmod failed with error: %v", err)
495531
}

‎test/e2e/tests/setup_e2e_test.go

+22-10
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,22 @@ import (
3030
computebeta "google.golang.org/api/compute/v0.beta"
3131
compute "google.golang.org/api/compute/v1"
3232
"k8s.io/klog/v2"
33+
"k8s.io/utils/strings/slices"
3334
testutils "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/e2e/utils"
3435
remote "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/test/remote"
3536
)
3637

3738
var (
38-
project = flag.String("project", "", "Project to run tests in")
39-
serviceAccount = flag.String("service-account", "", "Service account to bring up instance with")
40-
architecture = flag.String("arch", "amd64", "Architecture pd csi driver build on")
41-
zones = flag.String("zones", "us-east4-a,us-east4-c", "Zones to run tests in. If there are multiple zones, separate each by comma")
42-
machineType = flag.String("machine-type", "n2-standard-2", "Type of machine to provision instance on")
43-
imageURL = flag.String("image-url", "projects/debian-cloud/global/images/family/debian-11", "OS image url to get image from")
44-
runInProw = flag.Bool("run-in-prow", false, "If true, use a Boskos loaned project and special CI service accounts and ssh keys")
45-
deleteInstances = flag.Bool("delete-instances", false, "Delete the instances after tests run")
46-
cloudtopHost = flag.Bool("cloudtop-host", false, "The local host is cloudtop, a kind of googler machine with special requirements to access GCP")
39+
project = flag.String("project", "", "Project to run tests in")
40+
serviceAccount = flag.String("service-account", "", "Service account to bring up instance with")
41+
architecture = flag.String("arch", "amd64", "Architecture pd csi driver build on")
42+
zones = flag.String("zones", "us-east4-a,us-east4-c", "Zones to run tests in. If there are multiple zones, separate each by comma")
43+
machineType = flag.String("machine-type", "n2-standard-2", "Type of machine to provision instance on")
44+
imageURL = flag.String("image-url", "projects/debian-cloud/global/images/family/debian-11", "OS image url to get image from")
45+
runInProw = flag.Bool("run-in-prow", false, "If true, use a Boskos loaned project and special CI service accounts and ssh keys")
46+
deleteInstances = flag.Bool("delete-instances", false, "Delete the instances after tests run")
47+
cloudtopHost = flag.Bool("cloudtop-host", false, "The local host is cloudtop, a kind of googler machine with special requirements to access GCP")
48+
extraDriverFlags = flag.String("extra-driver-flags", "", "Extra flags to pass to the driver")
4749

4850
testContexts = []*remote.TestContext{}
4951
computeService *compute.Service
@@ -117,6 +119,16 @@ var _ = AfterSuite(func() {
117119
}
118120
})
119121

122+
func notEmpty(v string) bool {
123+
return v != ""
124+
}
125+
126+
func getDriverConfig() testutils.DriverConfig {
127+
return testutils.DriverConfig{
128+
ExtraFlags: slices.Filter(nil, strings.Split(*extraDriverFlags, ","), notEmpty),
129+
}
130+
}
131+
120132
func getRemoteInstanceConfig() *remote.InstanceConfig {
121133
return &remote.InstanceConfig{
122134
Project: *project,
@@ -152,7 +164,7 @@ func NewTestContext(zone string) *remote.TestContext {
152164
}
153165

154166
klog.Infof("Creating new driver and client for node %s", i.GetName())
155-
tc, err := testutils.GCEClientAndDriverSetup(i, "")
167+
tc, err := testutils.GCEClientAndDriverSetup(i, getDriverConfig())
156168
if err != nil {
157169
klog.Fatalf("Failed to set up TestContext for instance %v: %v", i.GetName(), err)
158170
}

‎test/e2e/tests/single_zone_e2e_test.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -1327,7 +1327,7 @@ var _ = Describe("GCE PD CSI Driver", func() {
13271327

13281328
// Create new driver and client with valid, empty endpoint
13291329
klog.Infof("Setup driver with empty compute endpoint %s\n", i.GetName())
1330-
tcEmpty, err := testutils.GCEClientAndDriverSetup(i, "")
1330+
tcEmpty, err := testutils.GCEClientAndDriverSetup(i, getDriverConfig())
13311331
if err != nil {
13321332
klog.Fatalf("Failed to set up Test Context for instance %v: %v", i.GetName(), err)
13331333
}
@@ -1336,7 +1336,9 @@ var _ = Describe("GCE PD CSI Driver", func() {
13361336
Expect(err).To(BeNil(), "no error expected when passed empty compute url")
13371337

13381338
// Create new driver and client w/ valid, passed-in endpoint
1339-
tcValid, err := testutils.GCEClientAndDriverSetup(i, "https://compute.googleapis.com")
1339+
driverConfig := getDriverConfig()
1340+
driverConfig.ComputeEndpoint = "https://compute.googleapis.com"
1341+
tcValid, err := testutils.GCEClientAndDriverSetup(i, driverConfig)
13401342
if err != nil {
13411343
klog.Fatalf("Failed to set up Test Context for instance %v: %v", i.GetName(), err)
13421344
}

‎test/e2e/utils/utils.go

+16-4
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ var (
4343
boskos, _ = boskosclient.NewClient(os.Getenv("JOB_NAME"), "http://boskos", "", "")
4444
)
4545

46-
func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint string) (*remote.TestContext, error) {
46+
type DriverConfig struct {
47+
ComputeEndpoint string
48+
ExtraFlags []string
49+
}
50+
51+
func GCEClientAndDriverSetup(instance *remote.InstanceInfo, driverConfig DriverConfig) (*remote.TestContext, error) {
4752
port := fmt.Sprintf("%v", 1024+rand.Intn(10000))
4853
goPath, ok := os.LookupEnv("GOPATH")
4954
if !ok {
@@ -61,7 +66,8 @@ func GCEClientAndDriverSetup(instance *remote.InstanceInfo, computeEndpoint stri
6166
"--use-instance-api-to-poll-attachment-disk-types=pd-ssd",
6267
"--use-instance-api-to-list-volumes-published-nodes",
6368
}
64-
extra_flags = append(extra_flags, fmt.Sprintf("--compute-endpoint=%s", computeEndpoint))
69+
extra_flags = append(extra_flags, fmt.Sprintf("--compute-endpoint=%s", driverConfig.ComputeEndpoint))
70+
extra_flags = append(extra_flags, driverConfig.ExtraFlags...)
6571

6672
workspace := remote.NewWorkspaceDir("gce-pd-e2e-")
6773
// Log at V(6) as the compute API calls are emitted at that level and it's
@@ -150,7 +156,7 @@ func SetupProwConfig(resourceType string) (project, serviceAccount string) {
150156
return project, serviceAccount
151157
}
152158

153-
func ForceChmod(instance *remote.InstanceInfo, filePath string, perms string) error {
159+
func ForceChmod(instance *remote.InstanceInfo, filePath string, perms string, recursive bool) error {
154160
originalumask, err := instance.SSHNoSudo("umask")
155161
if err != nil {
156162
return fmt.Errorf("failed to umask. Output: %v, errror: %v", originalumask, err.Error())
@@ -159,7 +165,13 @@ func ForceChmod(instance *remote.InstanceInfo, filePath string, perms string) er
159165
if err != nil {
160166
return fmt.Errorf("failed to umask. Output: %v, errror: %v", output, err.Error())
161167
}
162-
output, err = instance.SSH("chmod", "-R", perms, filePath)
168+
chmodOptions := []string{}
169+
if recursive {
170+
chmodOptions = []string{"-R"}
171+
}
172+
chmodOptions = append(chmodOptions, perms, filePath)
173+
chmodCmd := append([]string{"chmod"}, chmodOptions...)
174+
output, err = instance.SSH(chmodCmd...)
163175
if err != nil {
164176
return fmt.Errorf("failed to chmod file %s. Output: %v, errror: %v", filePath, output, err.Error())
165177
}

0 commit comments

Comments
 (0)