Separate TestMultipleVMs_Isolated from other tests

This test separates TestMultipleVMs_Isolated's 100 VMs run from other
tests by

- Changing the number of VMs from 100 to 10 in the runtime tests
- Keeping 100 VMs run as a separate job with its own concurrency group.

In addition to that, the test increases the number of VMs slowly,
instead of running 100 VMs from the beginning to figure out the breaking
point.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>

Author: Kazuyoshi Kato

.buildkite/pipeline.yml

@@ -73,7 +73,24 @@ steps:
       - make test-in-docker
     timeout_in_minutes: 10
 
-  - label: ":rotating_light: :running_shirt_with_sash: runtime isolated tests"
+  - label: ":running: runtime isolated tests"
+    agents:
+      queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
+      distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
+      hostname: "${BUILDKITE_AGENT_META_DATA_HOSTNAME}"
+    env:
+      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/*"
+    command:
+      - make -C runtime integ-test FICD_DM_POOL=build_${BUILDKITE_BUILD_NUMBER}_runtime
+
+  - label: ":weight_lifter: stress tests"
+    concurrency_group: stress
+    concurrency: 1
     agents:
       queue: "${BUILDKITE_AGENT_META_DATA_QUEUE:-default}"
       distro: "${BUILDKITE_AGENT_META_DATA_DISTRO}"
@@ -86,7 +103,7 @@ steps:
     artifact_paths:
       - "runtime/logs/*"
     command:
-      - make -C runtime integ-test FICD_DM_POOL=build_${BUILDKITE_BUILD_NUMBER}_runtime
+      - make -C runtime integ-test-TestMultipleVMs_Isolated FICD_DM_POOL=stress$BUILDKITE_BUILD_NUMBER
 
   - label: ":rotating_light: :exclamation: example tests"
     agents:

runtime/service_integ_test.go

@@ -242,22 +242,55 @@ func createTapDevice(ctx context.Context, tapName string) error {
 func TestMultipleVMs_Isolated(t *testing.T) {
 	integtest.Prepare(t)
 
-	// This test starts multiple VMs and some may hit firecracker-containerd's
-	// default timeout. So overriding the timeout to wait longer.
-	// One hour should be enough to start a VM, regardless of the load of
-	// the underlying host.
-	const createVMTimeout = time.Hour
-
-	netns, err := ns.GetCurrentNS()
-	require.NoError(t, err, "failed to get a namespace")
+	var err error
 
 	// numberOfVmsEnvName = NUMBER_OF_VMS ENV and is configurable from buildkite
 	numberOfVms := defaultNumberOfVms
 	if str := os.Getenv(numberOfVmsEnvName); str != "" {
 		numberOfVms, err = strconv.Atoi(str)
 		require.NoError(t, err, "failed to get NUMBER_OF_VMS env")
 	}
-	t.Logf("TestMultipleVMs_Isolated: will run %d vm's", numberOfVms)
+	t.Logf("TestMultipleVMs_Isolated: will run up to %d VMs", numberOfVms)
+
+	// We should be able to run 10 VMs without any issues.
+	if numberOfVms <= 10 {
+		testMultipleVMs(t, 10)
+		return
+	}
+
+	// We have issues running 100 VMs (see #581).
+	// Incrementally increase the number of VMs to find the breaking point.
+	for i := 10; i <= numberOfVms; i += 10 {
+		success := t.Run(fmt.Sprintf("VMs=%d", i), func(t *testing.T) {
+			testMultipleVMs(t, i)
+		})
+		if !success {
+			// If running N VMs doesn't work, no point to go further.
+			return
+		}
+	}
+}
+
+type Event int
+
+const (
+	Created Event = iota
+	Stopped
+)
+
+func testMultipleVMs(t *testing.T, count int) {
+	// This test starts multiple VMs and some may hit firecracker-containerd's
+	// default timeout. So overriding the timeout to wait longer.
+	// One hour should be enough to start a VM, regardless of the load of
+	// the underlying host.
+	const createVMTimeout = 1 * time.Hour
+
+	// Apparently writing a lot from Firecracker's serial console blocks VMs.
+	// 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"
+
+	netns, err := ns.GetCurrentNS()
+	require.NoError(t, err, "failed to get a namespace")
 
 	tapPrefix := os.Getenv(tapPrefixEnvName)
 
@@ -278,6 +311,7 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 		},
 		{
 			MaxContainers: 3,
+
 			JailerConfig: &proto.JailerConfig{
 				UID:        300000,
 				GID:        300000,
@@ -299,39 +333,56 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 	cfg, err := config.LoadConfig("")
 	require.NoError(t, err, "failed to load config")
 
+	eventCh := make(chan Event)
+
+	// Creating tap devices without goroutines somehow stabilize this test.
+	var devices []string
+
+	defer func() {
+		for _, dev := range devices {
+			err := deleteTapDevice(testCtx, dev)
+			assert.NoError(t, err)
+		}
+	}()
+
+	for i := 0; i < count; i++ {
+		tapName := fmt.Sprintf("%stap%d", tapPrefix, i)
+		err := createTapDevice(testCtx, tapName)
+		if err != nil {
+			t.Errorf("failed to create %q: %s", tapName, err)
+			return
+		}
+		devices = append(devices, tapName)
+	}
+
 	// 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.
 	vmEg, vmEgCtx := errgroup.WithContext(testCtx)
-	for i := 0; i < numberOfVms; i++ {
+	for i, device := range devices {
 		caseTypeNumber := i % len(cases)
 		vmID := i
+		device := device
 		c := cases[caseTypeNumber]
 
 		f := func(ctx context.Context) error {
 			containerCount := c.MaxContainers
 			jailerConfig := c.JailerConfig
 
-			tapName := fmt.Sprintf("%stap%d", tapPrefix, vmID)
-			err := createTapDevice(ctx, tapName)
-			if err != nil {
-				return err
-			}
-			defer deleteTapDevice(ctx, tapName)
-
 			rootfsPath := cfg.RootDrive
 
 			vmIDStr := strconv.Itoa(vmID)
 			req := &proto.CreateVMRequest{
-				VMID: vmIDStr,
+				KernelArgs: kernelArgs,
+				VMID:       vmIDStr,
 				RootDrive: &proto.FirecrackerRootDrive{
 					HostPath: rootfsPath,
 				},
 				NetworkInterfaces: []*proto.FirecrackerNetworkInterface{
 					{
 						AllowMMDS: true,
 						StaticConfig: &proto.StaticNetworkConfiguration{
-							HostDevName: tapName,
+							HostDevName: device,
 							MacAddress:  vmIDtoMacAddr(uint(vmID)),
 						},
 					},
@@ -349,6 +400,7 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 			if err != nil {
 				return err
 			}
+			defer fcClient.Close()
 
 			resp, createVMErr := fcClient.CreateVM(ctx, req)
 			if createVMErr != nil {
@@ -365,6 +417,7 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 					createVMErr,
 				)
 			}
+			eventCh <- Created
 
 			containerEg, containerCtx := errgroup.WithContext(vmEgCtx)
 			for containerID := 0; containerID < int(containerCount); containerID++ {
@@ -425,10 +478,8 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 			}
 
 			_, err = fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: strconv.Itoa(vmID), TimeoutSeconds: 5})
-			if err != nil {
-				return err
-			}
-			return nil
+			eventCh <- Stopped
+			return err
 		}
 
 		vmEg.Go(func() error {
@@ -440,8 +491,26 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 		})
 	}
 
-	err = vmEg.Wait()
-	require.NoError(t, err)
+	ticker := time.NewTicker(10 * time.Second)
+	defer ticker.Stop()
+
+	var created int
+	for stopped := 0; stopped < count; {
+		select {
+		case <-vmEgCtx.Done():
+			require.NoError(t, vmEg.Wait())
+			return
+		case e := <-eventCh:
+			switch e {
+			case Created:
+				created++
+			case Stopped:
+				stopped++
+			}
+		case <-ticker.C:
+			t.Logf("created=%d/%d stopped=%d/%d", created, count, stopped, count)
+		}
+	}
 }
 
 func testMultipleExecs(
@@ -478,7 +547,7 @@ func testMultipleExecs(
 	if err != nil {
 		return err
 	}
-	defer newContainer.Delete(ctx)
+	defer newContainer.Delete(ctx, containerd.WithSnapshotCleanup)
 
 	var taskStdout bytes.Buffer
 	var taskStderr bytes.Buffer