Skip to content

Commit a9de986

Browse files
committed
Fix race on network_test.go
Sadly go-openapi/runtime has a globally-shared mutable logger. So calling NewMachine() should be serialized to avoid race. Signed-off-by: Kazuyoshi Kato <[email protected]>
1 parent d89d60d commit a9de986

File tree

1 file changed

+22
-12
lines changed

1 file changed

+22
-12
lines changed

network_test.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -295,19 +295,25 @@ func TestNetworkMachineCNI(t *testing.T) {
295295
var vmWg sync.WaitGroup
296296
for i := 0; i < numVMs; i++ {
297297
vmWg.Add(1)
298-
go func(vmID string) {
299-
defer vmWg.Done()
300298

301-
ctx, cancel := context.WithCancel(context.Background())
299+
vmID := fmt.Sprintf("%d-%s-%d", timestamp, networkName, i)
300+
301+
firecrackerSockPath := filepath.Join(testCNIDir, fmt.Sprintf("%s.sock", vmID))
302+
rootfsPath := filepath.Join(testCNIDir, fmt.Sprintf("%s.img", vmID))
303+
304+
ctx, cancel := context.WithCancel(context.Background())
305+
// NewMachine cannot be in the goroutine below, since go-openapi/runtime has a globally-shared mutable logger...
306+
// https://github.com/go-openapi/runtime/blob/553c9d1fb273d9550562d9f76949a413af265138/client/runtime.go#L463
307+
m := newCNIMachine(t, ctx, firecrackerSockPath, rootfsPath, cniConfDir, cniCacheDir, networkName, ifName, vmID, cniBinPath)
308+
309+
go func(ctx context.Context, cancel func(), m *Machine, vmID string) {
310+
defer vmWg.Done()
302311
defer cancel()
303312

304-
firecrackerSockPath := filepath.Join(testCNIDir, fmt.Sprintf("%s.sock", vmID))
305-
rootfsPath := filepath.Join(testCNIDir, fmt.Sprintf("%s.img", vmID))
306313
expectedCacheDirPath := filepath.Join(cniCacheDir, "results",
307314
fmt.Sprintf("%s-%s-%s", networkName, vmID, ifName))
308315

309-
m, vmIP := startCNIMachine(t, ctx,
310-
firecrackerSockPath, rootfsPath, cniConfDir, cniCacheDir, networkName, ifName, vmID, cniBinPath)
316+
vmIP := startCNIMachine(t, ctx, m)
311317
vmIPs <- vmIP
312318

313319
assert.FileExists(t, expectedCacheDirPath, "CNI cache dir doesn't exist after vm startup")
@@ -326,7 +332,7 @@ func TestNetworkMachineCNI(t *testing.T) {
326332
_, err := os.Stat(expectedCacheDirPath)
327333
assert.True(t, os.IsNotExist(err), "expected CNI cache dir to not exist after vm exit")
328334

329-
}(fmt.Sprintf("%d-%s-%d", timestamp, networkName, i))
335+
}(ctx, cancel, m, vmID)
330336
}
331337
vmWg.Wait()
332338
close(vmIPs)
@@ -341,7 +347,7 @@ func TestNetworkMachineCNI(t *testing.T) {
341347
}
342348
}
343349

344-
func startCNIMachine(t *testing.T,
350+
func newCNIMachine(t *testing.T,
345351
ctx context.Context,
346352
firecrackerSockPath,
347353
rootfsPath,
@@ -351,7 +357,7 @@ func startCNIMachine(t *testing.T,
351357
ifName,
352358
vmID string,
353359
cniBinPath []string,
354-
) (*Machine, string) {
360+
) *Machine {
355361
rootfsBytes, err := ioutil.ReadFile(testRootfs)
356362
require.NoError(t, err, "failed to read rootfs file")
357363
err = ioutil.WriteFile(rootfsPath, rootfsBytes, 0666)
@@ -391,7 +397,11 @@ func startCNIMachine(t *testing.T,
391397
}, WithProcessRunner(cmd))
392398
require.NoError(t, err, "failed to create machine with CNI network interface")
393399

394-
err = m.Start(context.Background())
400+
return m
401+
}
402+
403+
func startCNIMachine(t *testing.T, ctx context.Context, m *Machine) string {
404+
err := m.Start(ctx)
395405
require.NoError(t, err, "failed to start machine")
396406

397407
staticConfig := m.Cfg.NetworkInterfaces[0].StaticConfiguration
@@ -406,7 +416,7 @@ func startCNIMachine(t *testing.T,
406416
require.NotNil(t, ipConfig,
407417
"cni configuration should have updated network interface ip configuration")
408418

409-
return m, ipConfig.IPAddr.IP.String()
419+
return ipConfig.IPAddr.IP.String()
410420
}
411421

412422
func testPing(t *testing.T, ip string, count int, timeout time.Duration) {

0 commit comments

Comments
 (0)