PR: rebase, comments, bugs, cleanup, security policy, linting

Fixed bug with calling `*hcsv2.Host.GetContainer` instead of `*hcsv2.Host.GetCreatedContainer`. Removed left over comments, added clarifying comments to `assertNumberContainers` and `listContaienrStates` interactions. Reordered namespace and rootfs cleanup. Removed underscore from consts Removed unneeded constants Added flag to test-lcow-uvm script to change boot type from vhd to initrd. Update security policy code to use enforcer. Updated script for changes to `uvmboot` and to use default executable name (`gcs.test`), as produced by `go test -c`. Linting issues: - `switch` to `if` - unused `getContainer()` - unused receivers Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
microsoft · Sep 13, 2022 · 9a9eec5 · 9a9eec5
1 parent bffebd8
commit 9a9eec5
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 107 deletions.
diff --git a/scripts/Test-LCOW-UVM.ps1 b/scripts/Test-LCOW-UVM.ps1
@@ -1,4 +1,4 @@
-#ex:  .\scripts\Test-LCOW-UVM.ps1 -vb -Action Bench  -BootFilesPath C:\ContainerPlat\LinuxBootFiles\ -MountGCSTest -Count 2 -Benchtime '3s'
+#ex: .\scripts\Test-LCOW-UVM.ps1 -vb -Action Bench -BootFilesPath C:\ContainerPlat\LinuxBootFiles\ -MountGCSTest -Count 2 -Benchtime '3s'
 # benchstat via  `go install golang.org/x/perf/cmd/benchstat@latest`
 
 [CmdletBinding()]
@@ -35,24 +35,30 @@ param (
     $OutDirectory = '.\test\results',
 
     # uvm parameters
+
     [string]
     $BootFilesPath = 'C:\ContainerPlat\LinuxBootFiles',
 
+    [ValidateSet('vhd', 'initrd')]
+    [string]
+    $BootFSType = 'vhd',
+
     [switch]
     $DisableTimeSync,
 
+    # gcs test/container options
+
     [string]
-    $RootfsMount = '/run/rootfs',
+    $ContainerRootFSMount = '/run/rootfs',
 
     [string]
-    $RootfsPath = (Join-Path $BootFilesPath 'rootfs.vhd'),
-    # $RootfsPath = "C:\ContainerPlatData\root\io.containerd.snapshotter.v1.windows-lcow\snapshots\2\layer.vhd",
+    $ContainerRootFSPath = (Join-Path $BootFilesPath 'rootfs.vhd'),
 
     [string]
     $GCSTestMount = '/run/bin',
 
     [string]
-    $GCSTestPath = '.\bin\test\gcstest',
+    $GCSTestPath = '.\bin\test\gcs.test',
 
     [switch]
     $MountGCSTest,
@@ -66,7 +72,7 @@ Import-Module ( Join-Path $PSScriptRoot Testing.psm1 ) -Force
 $CodePath = Resolve-Path $CodePath
 $OutDirectory = Resolve-Path $OutDirectory
 $BootFilesPath = Resolve-Path $BootFilesPath
-$RootfsPath = Resolve-Path $RootfsPath
+$ContainerRootFSPath = Resolve-Path $ContainerRootFSPath
 $GCSTestPath = Resolve-Path $GCSTestPath
 
 $shell = ( $Action -eq 'Shell' )
@@ -75,11 +81,11 @@ if ( $shell ) {
     $cmd = 'ash'
 } else {
     $date = Get-Date
-    $waitfiles = "$RootfsMount"
-    $gcspath = 'gcstest'
+    $waitfiles = "$ContainerRootFSMount"
+    $gcspath = 'gcs.test'
     if ( $MountGCSTest ) {
         $waitfiles += ",$GCSTestMount"
-        $gcspath = "$GCSTestMount/gcstest"
+        $gcspath = "$GCSTestMount/gcs.test"
     }
 
     $pre = "wait-paths -p $waitfiles -t 5 ; " + `
@@ -106,17 +112,16 @@ if ( $shell ) {
         -Feature $Feature `
         -Verbose:$Verbose
 
-    $testcmd += " `'-rootfs-path=$RootfsMount`' "
+    $testcmd += " `'-rootfs-path=$ContainerRootFSMount`' "
     $cmd = $pre + $testcmd
 }
 
-$boot = '.\bin\tools\uvmboot.exe -gcs lcow ' + `
+$boot = '.\bin\tool\uvmboot.exe -gcs lcow ' + `
     '-fwd-stdout -fwd-stderr -output-handling stdout ' + `
     "-boot-files-path $BootFilesPath " + `
-    '-root-fs-type vhd ' + `
+    "-root-fs-type $BootFSType " + `
     '-kernel-file vmlinux ' + `
-    "-security-policy=`"`" " + `
-    "-mount `"$RootfsPath,$RootfsMount`" "
+    "-mount-scsi `"$ContainerRootFSPath,$ContainerRootFSMount`" "
 
 if ( $MountGCSTest ) {
     $boot += "-share `"$GCSTestPath,$GCSTestMount`" "

diff --git a/test/gcs/container_bench_test.go b/test/gcs/container_bench_test.go
@@ -59,7 +59,7 @@ func BenchmarkContainerStart(b *testing.B) {
 	b.StopTimer()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		id, r, cleanup := _standaloneContainerRequest(ctx, b, host)
+		id, r, cleanup := standaloneContainerRequest(ctx, b, host)
 
 		c := createContainer(ctx, b, host, id, r)
 
@@ -82,7 +82,7 @@ func BenchmarkContainerKill(b *testing.B) {
 	b.StopTimer()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		id, r, cleanup := _standaloneContainerRequest(ctx, b, host)
+		id, r, cleanup := standaloneContainerRequest(ctx, b, host)
 		c := createContainer(ctx, b, host, id, r)
 		p := startContainer(ctx, b, c, stdio.ConnectionSettings{})
 
@@ -102,7 +102,7 @@ func BenchmarkContainerKill(b *testing.B) {
 	}
 }
 
-// container create through till wait and exit
+// benchmark container create through wait until exit.
 func BenchmarkContainerCompleteExit(b *testing.B) {
 	requireFeatures(b, featureStandalone)
 	ctx := context.Background()
@@ -111,7 +111,7 @@ func BenchmarkContainerCompleteExit(b *testing.B) {
 	b.StopTimer()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		id, r, cleanup := _standaloneContainerRequest(ctx, b, host, oci.WithProcessArgs("/bin/sh", "-c", "true"))
+		id, r, cleanup := standaloneContainerRequest(ctx, b, host, oci.WithProcessArgs("/bin/sh", "-c", "true"))
 
 		b.StartTimer()
 		c := createContainer(ctx, b, host, id, r)
@@ -144,7 +144,7 @@ func BenchmarkContainerCompleteKill(b *testing.B) {
 	b.StopTimer()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		id, r, cleanup := _standaloneContainerRequest(ctx, b, host)
+		id, r, cleanup := standaloneContainerRequest(ctx, b, host)
 
 		b.StartTimer()
 		c := createContainer(ctx, b, host, id, r)
@@ -177,7 +177,6 @@ func BenchmarkContainerExec(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		ps := testoci.CreateLinuxSpec(ctx, b, id,
-			// oci.WithTTY,
 			oci.WithDefaultPathEnv,
 			oci.WithProcessArgs("/bin/sh", "-c", "true"),
 		).Process
@@ -199,7 +198,12 @@ func BenchmarkContainerExec(b *testing.B) {
 	cleanupContainer(ctx, b, host, c)
 }
 
-func _standaloneContainerRequest(ctx context.Context, t testing.TB, host *hcsv2.Host, extra ...oci.SpecOpts) (string, *prot.VMHostedContainerSettingsV2, func()) {
+func standaloneContainerRequest(
+	ctx context.Context,
+	t testing.TB,
+	host *hcsv2.Host,
+	extra ...oci.SpecOpts,
+) (string, *prot.VMHostedContainerSettingsV2, func()) {
 	ctx = namespaces.WithNamespace(ctx, testoci.DefaultNamespace)
 	id := t.Name() + cri_util.GenerateID()
 	scratch, rootfs := mountRootfs(ctx, t, host, id)

diff --git a/test/gcs/container_test.go b/test/gcs/container_test.go
@@ -45,6 +45,7 @@ func TestContainerCreate(t *testing.T) {
 
 	assertNumberContainers(ctx, t, rtime, 1)
 	css := listContainerStates(ctx, t, rtime)
+	// guaranteed by assertNumberContainers that css will only have 1 element
 	cs := css[0]
 	if cs.ID != id {
 		t.Fatalf("got id %q, wanted %q", cs.ID, id)
@@ -76,10 +77,9 @@ func TestContainerDelete(t *testing.T) {
 
 	cleanupContainer(ctx, t, host, c)
 
-	// getContainer will Fatal
-	_, err := host.GetContainer(id)
+	_, err := host.GetCreatedContainer(id)
 	if hr, herr := gcserr.GetHresult(err); herr != nil || hr != gcserr.HrVmcomputeSystemNotFound {
-		t.Fatalf("GetContainer returned %v, wanted %v", err, gcserr.HrVmcomputeSystemNotFound)
+		t.Fatalf("GetCreatedContainer returned %v, wanted %v", err, gcserr.HrVmcomputeSystemNotFound)
 	}
 	assertNumberContainers(ctx, t, rtime, 0)
 }
@@ -140,7 +140,6 @@ func TestContainerIO(t *testing.T) {
 			})
 
 			c := createStandaloneContainer(ctx, t, host, id,
-				// oci.WithTTY,
 				oci.WithProcessArgs(tt.args...),
 			)
 			t.Cleanup(func() {
@@ -154,7 +153,7 @@ func TestContainerIO(t *testing.T) {
 
 			waitContainer(ctx, t, c, p, false)
 
-			g.Wait()
+			_ = g.Wait()
 			t.Logf("stdout: %q", outStr)
 			t.Logf("stderr: %q", errStr)
 
@@ -192,13 +191,11 @@ func TestContainerExec(t *testing.T) {
 	for _, tt := range ioTests {
 		t.Run(tt.name, func(t *testing.T) {
 			ps := testoci.CreateLinuxSpec(ctx, t, id,
-				// oci.WithTTY,
 				oci.WithDefaultPathEnv,
 				oci.WithProcessArgs(tt.args...),
 			).Process
 			con := newConnectionSettings(tt.in != "", true, true)
 			f := createStdIO(ctx, t, con)
-			// t.Logf("got channels %+v", f)
 
 			var outStr, errStr string
 			g := &errgroup.Group{}
@@ -226,7 +223,7 @@ func TestContainerExec(t *testing.T) {
 				t.Errorf("process exited with error code %d", i)
 			}
 
-			g.Wait()
+			_ = g.Wait()
 			t.Logf("stdout: %q", outStr)
 			t.Logf("stderr: %q", errStr)
 

diff --git a/test/gcs/cri_bench_test.go b/test/gcs/cri_bench_test.go
@@ -39,8 +39,8 @@ func BenchmarkCRISanboxCreate(b *testing.B) {
 		// so kill container to end those and avoid future perf hits
 		killContainer(ctx, b, c)
 		cleanupContainer(ctx, b, host, c)
-		unmountRootfs(ctx, b, scratch)
 		removeNamespace(ctx, b, nns)
+		unmountRootfs(ctx, b, scratch)
 	}
 }
 
@@ -71,8 +71,8 @@ func BenchmarkCRISandboxStart(b *testing.B) {
 		killContainer(ctx, b, c)
 		waitContainer(ctx, b, c, p, true)
 		cleanupContainer(ctx, b, host, c)
-		unmountRootfs(ctx, b, scratch)
 		removeNamespace(ctx, b, nns)
+		unmountRootfs(ctx, b, scratch)
 	}
 }
 
@@ -149,7 +149,7 @@ func BenchmarkCRIWorkload(b *testing.B) {
 		b.StopTimer()
 		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
-			id, r, cleanup := _workloadContainerRequest(ctx, b, host, sid, uint32(sandboxInit.Pid()), nns)
+			id, r, cleanup := workloadContainerRequest(ctx, b, host, sid, uint32(sandboxInit.Pid()), nns)
 
 			b.StartTimer()
 			c := createContainer(ctx, b, host, id, r)
@@ -170,7 +170,7 @@ func BenchmarkCRIWorkload(b *testing.B) {
 		b.StopTimer()
 		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
-			id, r, cleanup := _workloadContainerRequest(ctx, b, host, sid, uint32(sandboxInit.Pid()), nns)
+			id, r, cleanup := workloadContainerRequest(ctx, b, host, sid, uint32(sandboxInit.Pid()), nns)
 			c := createContainer(ctx, b, host, id, r)
 
 			b.StartTimer()
@@ -188,7 +188,7 @@ func BenchmarkCRIWorkload(b *testing.B) {
 		b.StopTimer()
 		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
-			id, r, cleanup := _workloadContainerRequest(ctx, b, host, sid, uint32(sandboxInit.Pid()), nns)
+			id, r, cleanup := workloadContainerRequest(ctx, b, host, sid, uint32(sandboxInit.Pid()), nns)
 			c := createContainer(ctx, b, host, id, r)
 			p := startContainer(ctx, b, c, stdio.ConnectionSettings{})
 
@@ -209,7 +209,14 @@ func BenchmarkCRIWorkload(b *testing.B) {
 	})
 }
 
-func _workloadContainerRequest(ctx context.Context, t testing.TB, host *hcsv2.Host, sid string, spid uint32, nns string) (string, *prot.VMHostedContainerSettingsV2, func()) {
+func workloadContainerRequest(
+	ctx context.Context,
+	t testing.TB,
+	host *hcsv2.Host,
+	sid string,
+	spid uint32,
+	nns string,
+) (string, *prot.VMHostedContainerSettingsV2, func()) {
 	id := sid + cri_util.GenerateID()
 	scratch, rootfs := mountRootfs(ctx, t, host, id)
 	spec := containerSpec(ctx, t,

diff --git a/test/gcs/cri_test.go b/test/gcs/cri_test.go
@@ -14,6 +14,9 @@ import (
 // tests for operations on sandbox and workload (CRI) containers
 //
 
+// TestCRILifecycle tests the entire CRI container workflow: creating and starting a CRI sandbox
+// pod container, creating, starting, and stopping a workload container within that pod, asserting
+// that all operations were successful, and mounting (and unmounting) rootfs's as necessary.
 func TestCRILifecycle(t *testing.T) {
 	requireFeatures(t, featureCRI)