Merge pull request #672 from austinvazquez/remote-snapshotter-integ-test

fix: do not pass testing.T to goroutines in remote snapshotter tests

Author: austinvazquez

.buildkite/pipeline.yml

@@ -73,7 +73,7 @@ steps:
       - make test-in-docker
     timeout_in_minutes: 10
 
-  - label: ":running: snapshotter isolated tests"
+  - label: ":running: runtime isolated tests"
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
@@ -82,12 +82,17 @@ steps:
       DOCKER_IMAGE_TAG: "$BUILDKITE_BUILD_NUMBER"
       NUMBER_OF_VMS: 10
       EXTRAGOARGS: "-v -count=1 -race"
+      FICD_DM_VOLUME_GROUP: fcci-vg
     artifact_paths:
-      - "snapshotter/logs/*"
+      - "runtime/logs/*"
     command:
-      - make -C snapshotter integ-test
+      - make -C runtime integ-test FICD_DM_POOL=build_${BUILDKITE_BUILD_NUMBER}_runtime
 
-  - label: ":running: runtime isolated tests"
+  - wait
+
+  # Let's isolate the remote snapshotter integration tests.
+  # See https://github.com/firecracker-microvm/firecracker-containerd/issues/673
+  - label: ":running: snapshotter isolated tests"
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
@@ -96,11 +101,12 @@ steps:
       DOCKER_IMAGE_TAG: "$BUILDKITE_BUILD_NUMBER"
       NUMBER_OF_VMS: 10
       EXTRAGOARGS: "-v -count=1 -race"
-      FICD_DM_VOLUME_GROUP: fcci-vg
     artifact_paths:
-      - "runtime/logs/*"
+      - "snapshotter/logs/*"
     command:
-      - make -C runtime integ-test FICD_DM_POOL=build_${BUILDKITE_BUILD_NUMBER}_runtime
+      - make -C snapshotter integ-test
+
+  - wait
 
   - label: ":weight_lifter: stress tests"
     concurrency_group: stress

snapshotter/service_integ_test.go

@@ -17,7 +17,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"sync"
 	"testing"
 	"time"
 
@@ -29,6 +28,7 @@ import (
 	"github.com/firecracker-microvm/firecracker-containerd/runtime/firecrackeroci"
 	"github.com/firecracker-microvm/firecracker-containerd/snapshotter/internal/integtest/stargz/fs/source"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sync/errgroup"
 )
 
 const (
@@ -58,7 +58,8 @@ func TestLaunchContainerWithRemoteSnapshotter_Isolated(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
 	defer cancel()
 
-	launchContainerWithRemoteSnapshotterInVM(ctx, t, strconv.Itoa(vmID))
+	err := launchContainerWithRemoteSnapshotterInVM(ctx, strconv.Itoa(vmID))
+	require.NoError(t, err)
 }
 
 func TestLaunchMultipleContainersWithRemoteSnapshotter_Isolated(t *testing.T) {
@@ -68,33 +69,43 @@ func TestLaunchMultipleContainersWithRemoteSnapshotter_Isolated(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
 	defer cancel()
 
-	var wg sync.WaitGroup
+	eg, ctx := errgroup.WithContext(ctx)
 
 	numberOfVms := integtest.NumberOfVms
 	for vmID := 0; vmID < numberOfVms; vmID++ {
-		wg.Add(1)
-		go func(id int) {
-			defer wg.Done()
-			launchContainerWithRemoteSnapshotterInVM(ctx, t, strconv.Itoa(id))
-		}(vmID)
+		ctx := ctx
+		id := vmID
+		eg.Go(func() error {
+			return launchContainerWithRemoteSnapshotterInVM(ctx, strconv.Itoa(id))
+		})
 	}
-	wg.Wait()
+	err := eg.Wait()
+	require.NoError(t, err)
 }
 
-func launchContainerWithRemoteSnapshotterInVM(ctx context.Context, t *testing.T, vmID string) {
+func launchContainerWithRemoteSnapshotterInVM(ctx context.Context, vmID string) error {
 	// For integration testing, assume the namespace is same as the VM ID.
 	namespace := vmID
 
 	ctx = namespaces.WithNamespace(ctx, namespace)
 
 	client, err := containerd.New(integtest.ContainerdSockPath, containerd.WithDefaultRuntime(integtest.FirecrackerRuntime))
-	require.NoError(t, err, "Unable to create client to containerd service at %s, is containerd running?", integtest.ContainerdSockPath)
+	if err != nil {
+		return fmt.Errorf("Unable to create client to containerd service at %s, is containerd running? [%v]", integtest.ContainerdSockPath, err)
+	}
 
 	fcClient, err := integtest.NewFCControlClient(integtest.ContainerdSockPath)
-	require.NoError(t, err, "Failed to create fccontrol client")
+	if err != nil {
+		return fmt.Errorf("Failed to create fccontrol client. [%v]", err)
+	}
+
+	// Disable 8250 serial device and lessen the number of log messages written to the serial console.
+	// https://github.com/firecracker-microvm/firecracker/blob/v1.1.0/docs/prod-host-setup.md
+	kernelArgs := integtest.DefaultRuntimeConfig.KernelArgs + " 8250.nr_uarts=0 quiet loglevel=1"
 
 	_, err = fcClient.CreateVM(ctx, &proto.CreateVMRequest{
-		VMID: vmID,
+		VMID:       vmID,
+		KernelArgs: kernelArgs,
 		RootDrive: &proto.FirecrackerRootDrive{
 			HostPath: "/var/lib/firecracker-containerd/runtime/rootfs-stargz.img",
 		},
@@ -111,23 +122,30 @@ func launchContainerWithRemoteSnapshotterInVM(ctx context.Context, t *testing.T,
 			VcpuCount:  1,
 			MemSizeMib: 1024,
 		},
+		TimeoutSeconds: 60,
 		ContainerCount: 1,
 	})
-	require.NoErrorf(t, err, "Failed to create microVM[%s]", vmID)
+	if err != nil {
+		return fmt.Errorf("Failed to create microVM[%s] [%v]", vmID, err)
+	}
 	defer fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: vmID})
 
 	_, err = fcClient.SetVMMetadata(ctx, &proto.SetVMMetadataRequest{
 		VMID:     vmID,
 		Metadata: fmt.Sprintf(dockerMetadataTemplate, "ghcr.io", noAuth, noAuth),
 	})
-	require.NoError(t, err, "Failed to configure VM metadata for registry resolution")
+	if err != nil {
+		return fmt.Errorf("Failed to configure VM metadata for registry resolution [%v]", err)
+	}
 
 	image, err := client.Pull(ctx, imageRef,
 		containerd.WithPullUnpack,
 		containerd.WithPullSnapshotter(snapshotterName),
 		containerd.WithImageHandlerWrapper(source.AppendDefaultLabelsHandlerWrapper(imageRef, 10*1024*1024)),
 	)
-	require.NoError(t, err, "Failed to pull image for VM: %s", vmID)
+	if err != nil {
+		return fmt.Errorf("Failed to pull image for VM: %s [%v]", vmID, err)
+	}
 	defer client.ImageService().Delete(ctx, image.Name())
 
 	container, err := client.NewContainer(ctx, fmt.Sprintf("container-%s", vmID),
@@ -141,9 +159,14 @@ func launchContainerWithRemoteSnapshotterInVM(ctx context.Context, t *testing.T,
 			firecrackeroci.WithVMNetwork,
 		),
 	)
-	require.NoError(t, err, "Failed to create container in VM: %s", vmID)
+	if err != nil {
+		return fmt.Errorf("Failed to create container in VM: %s, [%v]", vmID, err)
+	}
 	defer container.Delete(ctx, containerd.WithSnapshotCleanup)
 
 	_, err = integtest.RunTask(ctx, container)
-	require.NoError(t, err, "Failed to run task in VM: %s", vmID)
+	if err != nil {
+		return fmt.Errorf("Failed to run task in VM: %s [%v]", vmID, err)
+	}
+	return nil
 }