Use unique VMIDs in integ tests

Many of our integ tests use an integer as the VMID. Some of the test
suites run in parallel which risks either creating the same ID multiple
times or one test tearing down another test's resources.

This change makes each test's VMIDs unique

Signed-off-by: Kern Walster <walster@amazon.com>

internal/integtest/vmid.go

@@ -0,0 +1,43 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//	http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package integtest
+
+import (
+	"fmt"
+
+	"github.com/gofrs/uuid"
+)
+
+// VMIDGen generates namespaced VMIDs that are unique across different VMIDGens.
+// We have many tests which use a numeric ID as the VMID which could conflict with
+// concurrently running test. VMIDGen guarantees uniqueness across different tests.
+type VMIDGen struct {
+	prefix string
+}
+
+// NewVMIDGen creates a new VMIDGen
+func NewVMIDGen() (*VMIDGen, error) {
+	uid, err := uuid.NewV4()
+	if err != nil {
+		return nil, err
+	}
+	return &VMIDGen{uid.String()}, nil
+}
+
+// VMID creates a namespaced VMID given an integer seed.
+// A VMIDGen will generate the same VMID multiple times given the same seed,
+// but two different VMIDGens will generate different ids given the same seed.
+func (gen *VMIDGen) VMID(seed int) string {
+	return fmt.Sprintf("%s-%d", gen.prefix, seed)
+}

runtime/cni_integ_test.go

@@ -53,11 +53,14 @@ func TestCNISupport_Isolated(t *testing.T) {
 	image, err := alpineImage(ctx, client, defaultSnapshotterName)
 	require.NoError(t, err, "failed to get alpine image")
 
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "failed to create a VMIDGen")
+
 	numVMs := 5
 	var vmIDs []string
 	webpages := make(map[string]string)
 	for i := 0; i < numVMs; i++ {
-		vmID := fmt.Sprintf("vm-%d", i)
+		vmID := gen.VMID(i)
 		vmIDs = append(vmIDs, vmID)
 		webpages[vmID] = fmt.Sprintf("Hello, my virtual machine %s\n", vmID)
 	}
@@ -247,9 +250,8 @@ func TestCNIPlugin_Performance(t *testing.T) {
 	runCommand(ctx, t, "ip", "addr", "add", ipCidr, "dev", testDevName)
 	runCommand(ctx, t, "ip", "link", "set", "dev", testDevName, "up")
 
-	vmID := func(vmIndex int) string {
-		return fmt.Sprintf("vm-%d", vmIndex)
-	}
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
 
 	var vmGroup sync.WaitGroup
 	containers := make(chan containerd.Container, numVMs)
@@ -272,7 +274,7 @@ func TestCNIPlugin_Performance(t *testing.T) {
 			}()
 
 			_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
-				VMID: vmID(vmIndex),
+				VMID: gen.VMID(vmIndex),
 				MachineCfg: &proto.FirecrackerMachineConfiguration{
 					MemSizeMib: uint32(vmMemSizeMB),
 				},
@@ -285,8 +287,8 @@ func TestCNIPlugin_Performance(t *testing.T) {
 			})
 			require.NoError(t, err, "failed to create vm")
 
-			containerName := fmt.Sprintf("%s-container", vmID(vmIndex))
-			snapshotName := fmt.Sprintf("%s-snapshot", vmID(vmIndex))
+			containerName := fmt.Sprintf("%s-container", gen.VMID(vmIndex))
+			snapshotName := fmt.Sprintf("%s-snapshot", gen.VMID(vmIndex))
 
 			newContainer, err := client.NewContainer(ctx,
 				containerName,
@@ -300,7 +302,7 @@ func TestCNIPlugin_Performance(t *testing.T) {
 						"--interval", "60",
 						"--client", ipAddr,
 					),
-					firecrackeroci.WithVMID(vmID(vmIndex)),
+					firecrackeroci.WithVMID(gen.VMID(vmIndex)),
 					firecrackeroci.WithVMNetwork,
 				),
 			)

runtime/service_integ_test.go

@@ -355,6 +355,9 @@ func testMultipleVMs(ctx context.Context, t *testing.T, count int) {
 		stopped int64
 	)
 
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
+
 	// This test spawns separate VMs in parallel and ensures containers are spawned within each expected VM. It asserts each
 	// container ends up in the right VM by assigning each VM a network device with a unique mac address and having each container
 	// print the mac address it sees inside its VM.
@@ -371,7 +374,7 @@ func testMultipleVMs(ctx context.Context, t *testing.T, count int) {
 
 			rootfsPath := cfg.RootDrive
 
-			vmIDStr := strconv.Itoa(vmID)
+			vmIDStr := gen.VMID(vmID)
 			req := &proto.CreateVMRequest{
 				KernelArgs: kernelArgs,
 				VMID:       vmIDStr,
@@ -426,6 +429,7 @@ func testMultipleVMs(ctx context.Context, t *testing.T, count int) {
 					return testMultipleExecs(
 						containerCtx,
 						vmID,
+						gen,
 						containerID,
 						client, image,
 						jailerConfig,
@@ -435,21 +439,21 @@ func testMultipleVMs(ctx context.Context, t *testing.T, count int) {
 			}
 
 			// verify duplicate CreateVM call fails with right error
-			_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{VMID: strconv.Itoa(vmID)})
+			_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{VMID: vmIDStr})
 			if err == nil {
 				return fmt.Errorf("creating the same VM must return an error")
 			}
 
 			// verify GetVMInfo returns expected data
-			vmInfoResp, err := fcClient.GetVMInfo(ctx, &proto.GetVMInfoRequest{VMID: strconv.Itoa(vmID)})
+			vmInfoResp, err := fcClient.GetVMInfo(ctx, &proto.GetVMInfoRequest{VMID: vmIDStr})
 			if err != nil {
 				return err
 			}
-			if vmInfoResp.VMID != strconv.Itoa(vmID) {
-				return fmt.Errorf("%q must be %q", vmInfoResp.VMID, strconv.Itoa(vmID))
+			if vmInfoResp.VMID != vmIDStr {
+				return fmt.Errorf("%q must be %q", vmInfoResp.VMID, vmIDStr)
 			}
 
-			nspVMid := defaultNamespace + "#" + strconv.Itoa(vmID)
+			nspVMid := defaultNamespace + "#" + vmIDStr
 			cfg, err := config.LoadConfig("")
 			if err != nil {
 				return err
@@ -465,7 +469,7 @@ func testMultipleVMs(ctx context.Context, t *testing.T, count int) {
 			// just verify that updating the metadata doesn't return an error, a separate test case is needed
 			// to very the MMDS update propagates to the container correctly
 			_, err = fcClient.SetVMMetadata(ctx, &proto.SetVMMetadataRequest{
-				VMID:     strconv.Itoa(vmID),
+				VMID:     vmIDStr,
 				Metadata: "{}",
 			})
 			if err != nil {
@@ -477,7 +481,7 @@ func testMultipleVMs(ctx context.Context, t *testing.T, count int) {
 				return fmt.Errorf("unexpected error from the containers in VM %d: %w", vmID, err)
 			}
 
-			_, err = fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: strconv.Itoa(vmID), TimeoutSeconds: 5})
+			_, err = fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: vmIDStr, TimeoutSeconds: 5})
 			atomic.AddInt64(&stopped, 1)
 			return err
 		}
@@ -515,13 +519,14 @@ loop:
 func testMultipleExecs(
 	ctx context.Context,
 	vmID int,
+	gen *integtest.VMIDGen,
 	containerID int,
 	client *containerd.Client,
 	image containerd.Image,
 	jailerConfig *proto.JailerConfig,
 	cgroupPath string,
 ) error {
-	vmIDStr := strconv.Itoa(vmID)
+	vmIDStr := gen.VMID(vmID)
 	testTimeout := 600 * time.Second
 
 	containerName := fmt.Sprintf("container-%d-%d", vmID, containerID)
@@ -826,6 +831,9 @@ func TestStubBlockDevices_Isolated(t *testing.T) {
 
 	const vmID = 0
 
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "failed to create VMIDGen")
+
 	ctx := namespaces.WithNamespace(context.Background(), "default")
 
 	client, err := containerd.New(integtest.ContainerdSockPath, containerd.WithDefaultRuntime(firecrackerRuntime))
@@ -846,7 +854,7 @@ func TestStubBlockDevices_Isolated(t *testing.T) {
 	require.NoError(t, err, "failed to create fccontrol client")
 
 	_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
-		VMID: strconv.Itoa(vmID),
+		VMID: gen.VMID(vmID),
 		NetworkInterfaces: []*proto.FirecrackerNetworkInterface{
 			{
 				AllowMMDS: true,
@@ -865,7 +873,7 @@ func TestStubBlockDevices_Isolated(t *testing.T) {
 		containerd.WithSnapshotter(defaultSnapshotterName),
 		containerd.WithNewSnapshot(snapshotName, image),
 		containerd.WithNewSpec(
-			firecrackeroci.WithVMID(strconv.Itoa(vmID)),
+			firecrackeroci.WithVMID(gen.VMID(vmID)),
 			oci.WithProcessArgs("/bin/sh", "/var/firecracker-containerd-test/scripts/lsblk.sh"),
 
 			oci.WithMounts([]specs.Mount{
@@ -1447,6 +1455,9 @@ func TestMemoryBalloon_Isolated(t *testing.T) {
 	}
 	t.Logf("TestMemoryBalloon_Isolated: will run %d vm's", numberOfVms)
 
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
+
 	var vmGroup sync.WaitGroup
 	for i := 0; i < numberOfVms; i++ {
 		vmGroup.Add(1)
@@ -1462,7 +1473,7 @@ func TestMemoryBalloon_Isolated(t *testing.T) {
 			require.NoError(t, err, "failed to create fccontrol client")
 
 			_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
-				VMID: strconv.Itoa(vmID),
+				VMID: gen.VMID(vmID),
 				MachineCfg: &proto.FirecrackerMachineConfiguration{
 					MemSizeMib: 512,
 				},
@@ -1484,7 +1495,7 @@ func TestMemoryBalloon_Isolated(t *testing.T) {
 			require.NoError(t, err, "failed to create vm")
 
 			// Test UpdateBalloon correctly updates amount of memory for the balloon device
-			vmIDStr := strconv.Itoa(vmID)
+			vmIDStr := gen.VMID(vmID)
 			newAmountMib := int64(50)
 			_, err = fcClient.UpdateBalloon(ctx, &proto.UpdateBalloonRequest{
 				VMID:      vmIDStr,

snapshotter/metrics_integ_test.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strconv"
 	"testing"
 
 	"github.com/containerd/containerd"
@@ -43,15 +42,17 @@ const (
 func TestSnapshotterMetrics_Isolated(t *testing.T) {
 	integtest.Prepare(t)
 
-	vmID := 0
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
+	vmID := gen.VMID(0)
 
-	ctx := namespaces.WithNamespace(context.Background(), strconv.Itoa(vmID))
+	ctx := namespaces.WithNamespace(context.Background(), vmID)
 
 	fcClient, err := integtest.NewFCControlClient(integtest.ContainerdSockPath)
-	defer fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: strconv.Itoa(vmID)})
+	defer fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: vmID})
 	require.NoError(t, err, "Failed to create fccontrol client")
 
-	require.NoError(t, pullImageWithRemoteSnapshotterInVM(ctx, strconv.Itoa(vmID), fcClient))
+	require.NoError(t, pullImageWithRemoteSnapshotterInVM(ctx, vmID, fcClient))
 	verifyMetricsResponse(t, 1)
 }
 
@@ -63,15 +64,17 @@ func TestSnapshotterMetricsMultipleVMs_Isolated(t *testing.T) {
 	require.NoError(t, err, "Failed to create fccontrol client")
 
 	group, ctx := errgroup.WithContext(context.Background())
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
 
 	for vmID := 0; vmID < numberOfVms; vmID++ {
-		id := vmID
-		ctxNamespace := namespaces.WithNamespace(ctx, strconv.Itoa(id))
-		defer fcClient.StopVM(ctxNamespace, &proto.StopVMRequest{VMID: strconv.Itoa(id)})
+		id := gen.VMID(vmID)
+		ctxNamespace := namespaces.WithNamespace(ctx, id)
+		defer fcClient.StopVM(ctxNamespace, &proto.StopVMRequest{VMID: id})
 
 		group.Go(
 			func() error {
-				return pullImageWithRemoteSnapshotterInVM(ctxNamespace, strconv.Itoa(id), fcClient)
+				return pullImageWithRemoteSnapshotterInVM(ctxNamespace, id, fcClient)
 			},
 		)
 

snapshotter/service_integ_test.go

@@ -16,7 +16,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"strconv"
 	"testing"
 
 	"github.com/containerd/containerd"
@@ -51,25 +50,28 @@ const (
 func TestLaunchContainerWithRemoteSnapshotter_Isolated(t *testing.T) {
 	integtest.Prepare(t, integtest.WithDefaultNetwork())
 
-	vmID := 0
-	err := launchContainerWithRemoteSnapshotterInVM(context.Background(), strconv.Itoa(vmID))
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
+	err = launchContainerWithRemoteSnapshotterInVM(context.Background(), gen.VMID(0))
 	require.NoError(t, err)
 }
 
 func TestLaunchMultipleContainersWithRemoteSnapshotter_Isolated(t *testing.T) {
 	integtest.Prepare(t, integtest.WithDefaultNetwork())
+	gen, err := integtest.NewVMIDGen()
+	require.NoError(t, err, "Failed to create a VMIDGen")
 
 	eg, ctx := errgroup.WithContext(context.Background())
 
 	numberOfVms := integtest.NumberOfVms
 	for vmID := 0; vmID < numberOfVms; vmID++ {
 		ctx := ctx
-		id := vmID
+		id := gen.VMID(vmID)
 		eg.Go(func() error {
-			return launchContainerWithRemoteSnapshotterInVM(ctx, strconv.Itoa(id))
+			return launchContainerWithRemoteSnapshotterInVM(ctx, id)
 		})
 	}
-	err := eg.Wait()
+	err = eg.Wait()
 	require.NoError(t, err)
 }