Skip to content

Commit

Permalink
Use unique VMIDs in integ tests
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Kern-- committed Sep 23, 2023
1 parent 3fae0bd commit ab2ec87
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 36 deletions.
43 changes: 43 additions & 0 deletions internal/integtest/vmid.go
Original file line number Diff line number Diff line change
@@ -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)
}
18 changes: 10 additions & 8 deletions runtime/cni_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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),
},
Expand All @@ -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,
Expand All @@ -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,
),
)
Expand Down
36 changes: 23 additions & 13 deletions runtime/service_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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,
Expand All @@ -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{
Expand Down Expand Up @@ -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)
Expand All @@ -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,
},
Expand All @@ -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,
Expand Down
21 changes: 12 additions & 9 deletions snapshotter/metrics_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"io"
"net/http"
"strconv"
"testing"

"github.com/containerd/containerd"
Expand All @@ -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)
}

Expand All @@ -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)
},
)

Expand Down
14 changes: 8 additions & 6 deletions snapshotter/service_integ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package main
import (
"context"
"fmt"
"strconv"
"testing"

"github.com/containerd/containerd"
Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit ab2ec87

Please sign in to comment.