[gcs.test] update scratch space cleanup order (#1794)

helsaawy · web-flow · commit 9fd0e723a603 · 2023-05-30T18:07:59.000-04:00
Signed-off-by: Hamza El-Saawy &lt;hamzaelsaawy@microsoft.com&gt;
diff --git a/scripts/Test-LCOW-UVM.ps1 b/scripts/Test-LCOW-UVM.ps1
@@ -51,8 +51,10 @@ param (
 
     # gcs test/container options
 
+    # we can no longer specify the guest/destination path for SCSI mounts, so hope the rootfs is the first
+    # path to be SCSI-mounted
     [string]
-    $ContainerRootFSMount = '/run/rootfs',
+    $ContainerRootFSMount = '/run/mounts/scsi/m0',
 
     [string]
     $ContainerRootFSPath = (Join-Path $BootFilesPath 'rootfs.vhd'),
@@ -79,10 +81,10 @@ $UVMBootPath = Resolve-Path $UVMBootPath
 
 $shell = ( $Action -eq 'Shell' )
 
+$date = Get-Date
 if ( $shell ) {
     $cmd = 'ash'
 } else {
-    $date = Get-Date
     $waitfiles = "$ContainerRootFSMount"
     $gcspath = 'gcs.test'
     if ( -not $SkipGCSTestMount ) {
@@ -124,7 +126,7 @@ $boot = "$UVMBootPath -gcs lcow " + `
     "-boot-files-path $BootFilesPath " + `
     "-root-fs-type $BootFSType " + `
     '-kernel-file vmlinux ' + `
-    "-mount-scsi `"$ContainerRootFSPath,$ContainerRootFSMount`" "
+    "-mount-scsi `"$ContainerRootFSPath`" "
 
 if ( -not $SkipGCSTestMount ) {
     $boot += "-share `"$GCSTestPath,$GCSTestMount`" "
diff --git a/test/gcs/container_bench_test.go b/test/gcs/container_bench_test.go
@@ -45,9 +45,7 @@ func BenchmarkContainerCreate(b *testing.B) {
 		// create launches background go-routines
 		// so kill container to end those and avoid future perf hits
 		killContainer(ctx, b, c)
-		deleteContainer(ctx, b, c)
-		removeContainer(ctx, b, host, id)
-		unmountRootfs(ctx, b, scratch)
+		cleanupContainer(ctx, b, host, c)
 	}
 }
 
diff --git a/test/gcs/helper_container_test.go b/test/gcs/helper_container_test.go
@@ -4,7 +4,6 @@ package gcs
 
 import (
 	"context"
-	"os"
 	"path/filepath"
 	"syscall"
 	"testing"
@@ -34,7 +33,8 @@ const tailNull = "tail -f /dev/null"
 
 // Creates an overlay mount, and then a container using that mount that runs until stopped.
 // The container is created on its own, and not associated with a sandbox pod, and is therefore not CRI compliant.
-// [unmountRootfs] is added to the test cleanup.
+//
+// [unmountRootfs] must be called on the scratch directory before deleting the container.
 func createStandaloneContainer(ctx context.Context, tb testing.TB, host *hcsv2.Host, id string, extra ...ctrdoci.SpecOpts) *hcsv2.Container {
 	tb.Helper()
 	ctx = namespaces.WithNamespace(ctx, testoci.DefaultNamespace)
@@ -51,6 +51,8 @@ func createStandaloneContainer(ctx context.Context, tb testing.TB, host *hcsv2.H
 		OCISpecification: s,
 	}
 
+	// should be called before the container is deleted, but call it again incase the test errors before
+	// the container is created/cleanup is scheduled
 	tb.Cleanup(func() {
 		unmountRootfs(ctx, tb, scratch)
 	})
@@ -171,6 +173,9 @@ func deleteContainer(ctx context.Context, tb testing.TB, c *hcsv2.Container) {
 
 func cleanupContainer(ctx context.Context, tb testing.TB, host *hcsv2.Host, c *hcsv2.Container) {
 	tb.Helper()
+	// assume scratch space was created by [mountRootfs]
+	scratch := filepath.Join(guestpath.LCOWRootPrefixInUVM, c.ID())
+	unmountRootfs(ctx, tb, scratch)
 	deleteContainer(ctx, tb, c)
 	removeContainer(ctx, tb, host, c.ID())
 }
@@ -257,9 +262,6 @@ func unmountRootfs(ctx context.Context, tb testing.TB, path string) {
 	if err := storage.UnmountAllInPath(ctx, path, true); err != nil {
 		tb.Fatalf("could not unmount container rootfs: %v", err)
 	}
-	if err := os.RemoveAll(path); err != nil {
-		tb.Fatalf("could not remove container directory: %v", err)
-	}
 }
 
 //

Original file line number	Diff line number	Diff line change
`@@ -45,9 +45,7 @@ func BenchmarkContainerCreate(b *testing.B) {`
`45`	`45`	`// create launches background go-routines`
`46`	`46`	`// so kill container to end those and avoid future perf hits`
`47`	`47`	`killContainer(ctx, b, c)`
`48`		`- deleteContainer(ctx, b, c)`
`49`		`- removeContainer(ctx, b, host, id)`
`50`		`- unmountRootfs(ctx, b, scratch)`
	`48`	`+ cleanupContainer(ctx, b, host, c)`
`51`	`49`	`}`
`52`	`50`	`}`
`53`	`51`