From ab2ec87d7bab51b11468dc3dba1d02d63374ea82 Mon Sep 17 00:00:00 2001 From: Kern Walster Date: Fri, 22 Sep 2023 22:46:52 +0000 Subject: [PATCH] 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 --- internal/integtest/vmid.go | 43 +++++++++++++++++++++++++++++++ runtime/cni_integ_test.go | 18 +++++++------ runtime/service_integ_test.go | 36 ++++++++++++++++---------- snapshotter/metrics_integ_test.go | 21 ++++++++------- snapshotter/service_integ_test.go | 14 +++++----- 5 files changed, 96 insertions(+), 36 deletions(-) create mode 100644 internal/integtest/vmid.go diff --git a/internal/integtest/vmid.go b/internal/integtest/vmid.go new file mode 100644 index 000000000..fb9054aa9 --- /dev/null +++ b/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) +} diff --git a/runtime/cni_integ_test.go b/runtime/cni_integ_test.go index e2c3d8cee..bff081feb 100644 --- a/runtime/cni_integ_test.go +++ b/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, ), ) diff --git a/runtime/service_integ_test.go b/runtime/service_integ_test.go index d51463313..742807842 100644 --- a/runtime/service_integ_test.go +++ b/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, @@ -435,21 +438,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 +468,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 +480,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 +518,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 +830,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 +853,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 +872,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 +1454,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 +1472,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 +1494,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, diff --git a/snapshotter/metrics_integ_test.go b/snapshotter/metrics_integ_test.go index 63bfe0ad9..b8a28f6fd 100644 --- a/snapshotter/metrics_integ_test.go +++ b/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) }, ) diff --git a/snapshotter/service_integ_test.go b/snapshotter/service_integ_test.go index 7882352bb..207e1563f 100644 --- a/snapshotter/service_integ_test.go +++ b/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) }