From 94f4f06d13047394d73bf77e4bf1cf41f03102ca Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 11 Sep 2020 14:36:42 -0700 Subject: [PATCH] test flake: fix data race in csi_test.go The attach goroutine can currently t.Log/t.Error during or after the subtest completion. This causes races like: ``` ================== WARNING: DATA RACE Read at 0x00c000e90ac3 by goroutine 1231: testing.(*common).logDepth() GOROOT/src/testing/testing.go:736 +0xa9 testing.(*common).log() GOROOT/src/testing/testing.go:729 +0x8f testing.(*common).Logf() GOROOT/src/testing/testing.go:775 +0x21 k8s.io/kubernetes/pkg/volume/csi.TestCSI_VolumeAll.func21.1() pkg/volume/csi/csi_test.go:313 +0x1a4 Previous write at 0x00c000e90ac3 by goroutine 875: testing.tRunner.func1() GOROOT/src/testing/testing.go:1113 +0x484 testing.tRunner() GOROOT/src/testing/testing.go:1131 +0x22a testing.tRunner() GOROOT/src/testing/testing.go:1127 +0x202 Goroutine 1231 (running) created at: k8s.io/kubernetes/pkg/volume/csi.TestCSI_VolumeAll.func21() pkg/volume/csi/csi_test.go:307 +0xf05 testing.tRunner() GOROOT/src/testing/testing.go:1127 +0x202 Goroutine 875 (running) created at: testing.(*T).Run() GOROOT/src/testing/testing.go:1178 +0x796 k8s.io/kubernetes/pkg/volume/csi.TestCSI_VolumeAll() pkg/volume/csi/csi_test.go:223 +0xb2c testing.tRunner() GOROOT/src/testing/testing.go:1127 +0x202 ================== ``` See also this comment: https://github.com/golang/go/blob/07c1788357cfe6a4ee5f6f6a54d4fe9f579fa844/src/testing/testing.go#L1141-L1142 Noticed in: https://github.com/kubernetes/kubernetes/pull/94449 https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/94449/pull-kubernetes-bazel-test/1304519003330318337 --- pkg/volume/csi/csi_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/volume/csi/csi_test.go b/pkg/volume/csi/csi_test.go index f4e09be2bc1..d3c2d2e90dd 100644 --- a/pkg/volume/csi/csi_test.go +++ b/pkg/volume/csi/csi_test.go @@ -304,19 +304,22 @@ func TestCSI_VolumeAll(t *testing.T) { } // creates VolumeAttachment and blocks until it is marked attached (done by external attacher) - go func(spec *volume.Spec, nodeName types.NodeName) { - attachID, err := volAttacher.Attach(spec, nodeName) + attachDone := make(chan struct{}) + go func() { + defer close(attachDone) + attachID, err := volAttacher.Attach(volSpec, host.GetNodeName()) if err != nil { t.Errorf("csiTest.VolumeAll attacher.Attach failed: %s", err) return } t.Logf("csiTest.VolumeAll got attachID %s", attachID) - - }(volSpec, host.GetNodeName()) + }() // Simulates external-attacher and marks VolumeAttachment.Status.Attached = true markVolumeAttached(t, host.GetKubeClient(), fakeWatcher, attachName, storage.VolumeAttachmentStatus{Attached: true}) + <-attachDone + // Observe attach on this node. devicePath, err = volAttacher.WaitForAttach(volSpec, "", pod, 500*time.Millisecond) if err != nil { t.Fatal("csiTest.VolumeAll attacher.WaitForAttach failed:", err)