PR: doc, simplified signatures

Added deprecated warning to `layers.LayerFolders`, which relies on
docker.
Added doc comment to functional tests to clarify overlap with other
tests.
Removed unnecessary parameter in `WaitForError`.
Updated snapshotter logic

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>

test/functional/lcow_container_bench_test.go

@@ -30,8 +30,8 @@ func BenchmarkLCOW_Container(b *testing.B) {
 	require.Build(b, osversion.RS5)
 
 	ctx, _, client := newContainerdClient(context.Background(), b)
-	cid := containerd.PullImage(ctx, b, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
-	ls := layers.FromChainID(ctx, b, client, cid, constants.SnapshotterLinux)
+	chainID := containerd.PullImage(ctx, b, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
+	ls := layers.FromChainID(ctx, b, client, chainID, constants.SnapshotterLinux)
 
 	// Create a new uvm per benchmark in case any left over state lingers
 
@@ -225,5 +225,4 @@ func BenchmarkLCOW_Container(b *testing.B) {
 			cleanup()
 		}
 	})
-
 }

test/functional/lcow_container_test.go

@@ -27,8 +27,8 @@ func TestLCOW_ContainerLifecycle(t *testing.T) {
 	require.Build(t, osversion.RS5)
 
 	ctx, _, client := newContainerdClient(context.Background(), t)
-	cid := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
-	ls := layers.FromChainID(ctx, t, client, cid, constants.SnapshotterLinux)
+	chainID := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
+	ls := layers.FromChainID(ctx, t, client, chainID, constants.SnapshotterLinux)
 
 	opts := defaultLCOWOptions(t, t.Name())
 	vm := uvm.CreateAndStartLCOWFromOpts(ctx, t, opts)
@@ -84,8 +84,8 @@ func TestLCOW_ContainerIO(t *testing.T) {
 	require.Build(t, osversion.RS5)
 
 	ctx, _, client := newContainerdClient(context.Background(), t)
-	cid := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
-	ls := layers.FromChainID(ctx, t, client, cid, constants.SnapshotterLinux)
+	chainID := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
+	ls := layers.FromChainID(ctx, t, client, chainID, constants.SnapshotterLinux)
 
 	opts := defaultLCOWOptions(t, t.Name())
 	cache := layers.CacheFile(ctx, t, "")
@@ -129,8 +129,8 @@ func TestLCOW_ContainerExec(t *testing.T) {
 	require.Build(t, osversion.RS5)
 
 	ctx, _, client := newContainerdClient(context.Background(), t)
-	cid := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
-	ls := layers.FromChainID(ctx, t, client, cid, constants.SnapshotterLinux)
+	chainID := containerd.PullImage(ctx, t, client, constants.ImageLinuxAlpineLatest, constants.PlatformLinux)
+	ls := layers.FromChainID(ctx, t, client, chainID, constants.SnapshotterLinux)
 
 	opts := defaultLCOWOptions(t, t.Name())
 	vm := uvm.CreateAndStartLCOWFromOpts(ctx, t, opts)

test/functional/main_test.go

@@ -1,5 +1,10 @@
 //go:build windows && functional
 
+// This package tests the internals of hcsshim, independent of the OCI interfaces it exposes
+// and the container runtime (or CRI API) that normally would be communicating with the shim.
+//
+// While these tests may overlap with CRI/containerd or shim tests, they exercise `internal/*`
+// code paths and primitives directly.
 package functional
 
 import (

test/internal/constants/constants.go

@@ -1,15 +1,30 @@
 package constants
 
+import (
+	"fmt"
+
+	"github.com/containerd/containerd/errdefs"
+	"github.com/containerd/containerd/platforms"
+)
+
 const (
 	PlatformWindows    = "windows"
 	PlatformLinux      = "linux"
 	SnapshotterWindows = "windows"
 	SnapshotterLinux   = "windows-lcow"
 )
 
-func SnapshotterFromPlatform(platform string) string {
-	if platform == PlatformWindows {
-		return SnapshotterWindows
+func SnapshotterFromPlatform(platform string) (string, error) {
+	p, err := platforms.Parse(platform)
+	if err != nil {
+		return "", err
+	}
+	switch p.OS {
+	case PlatformWindows:
+		return SnapshotterWindows, nil
+	case PlatformLinux:
+		return SnapshotterLinux, nil
+	default:
 	}
-	return SnapshotterLinux
+	return "", fmt.Errorf("unknown platform os %q: %v", p.OS, errdefs.ErrInvalidArgument)
 }

test/internal/containerd/containerd.go

@@ -168,33 +168,21 @@ func PullImage(ctx context.Context, t testing.TB, client *containerd.Client, ref
 		return chainID.(string)
 	}
 
-	// r := GetResolver(ctx, t)
-	// ss := constants.SnapshotterFromPlatform(plat)
-
 	opts := []containerd.RemoteOpt{
-		// containerd.WithResolver(r),
-		containerd.WithPullSnapshotter(constants.SnapshotterFromPlatform(plat)),
 		containerd.WithSchema1Conversion,
 		containerd.WithPlatform(plat),
 		containerd.WithPullUnpack,
 	}
 
+	if s, err := constants.SnapshotterFromPlatform(plat); err == nil {
+		opts = append(opts, containerd.WithPullSnapshotter(s))
+	}
+
 	img, err := client.Pull(ctx, ref, opts...)
 	if err != nil {
 		t.Fatalf("could not pull image %q: %v", ref, err)
 	}
 
-	// img, err := client.Fetch(ctx, ref, opts...)
-	// if err != nil {
-	// 	t.Fatalf("could not fetch image %q: %v", ref, err)
-	// }
-	// t.Logf("fetched image %q", img.Name)
-
-	// i := containerd.NewImageWithPlatform(client, img, p)
-	// if err := i.Unpack(ctx, ss); err != nil {
-	// 	t.Fatalf("could not unpack image %q: %v", i.Name(), err)
-	// }
-
 	name := img.Name()
 	diffIDs, err := img.RootFS(ctx)
 	if err != nil {

test/internal/layers/layerfolders.go

@@ -48,6 +48,7 @@ func FromMount(_ context.Context, t testing.TB, m mount.Mount) (layers []string)
 	return layers
 }
 
+// Deprecated: This relies on docker. Use [FromChainID] or [FromMount] instead.
 func LayerFolders(t testing.TB, imageName string) []string {
 	if _, ok := imageLayers[imageName]; !ok {
 		imageLayers[imageName] = getLayers(t, imageName)

test/internal/timeout/timeout.go

@@ -13,11 +13,11 @@ const ConnectTimeout = time.Second * 10
 type ErrorFunc func(error) error
 
 // WaitForError waits until f returns or the context is done.
-// The returned error will be passed to the error processing functions fe or fec, respectively.
-// If processed error is non-nil, it is passed to t.Fatal().
+// The returned error will be passed to the error processing functions fe, and (if non-nil),
+// then passed to to [testing.Fatal].
 //
-// Both error processing functions should expect nil errors.
-func WaitForError(ctx context.Context, t testing.TB, f func() error, fe, fec ErrorFunc) {
+// fe should expect nil errors.
+func WaitForError(ctx context.Context, t testing.TB, f func() error, fe ErrorFunc) {
 	var err error
 	ch := make(chan struct{})
 
@@ -30,7 +30,7 @@ func WaitForError(ctx context.Context, t testing.TB, f func() error, fe, fec Err
 	case <-ch:
 		fatalOnError(t, fe, err)
 	case <-ctx.Done():
-		fatalOnError(t, fec, ctx.Err())
+		fatalOnError(t, fe, ctx.Err())
 	}
 }
 

test/internal/uvm/uvm.go

@@ -36,7 +36,7 @@ func Wait(ctx context.Context, t testing.TB, vm *uvm.UtilityVM) {
 
 		return err
 	}
-	timeout.WaitForError(ctx, t, vm.Wait, fe, fe)
+	timeout.WaitForError(ctx, t, vm.Wait, fe)
 }
 
 func Kill(ctx context.Context, t testing.TB, vm *uvm.UtilityVM) {
@@ -55,5 +55,5 @@ func Close(ctx context.Context, t testing.TB, vm *uvm.UtilityVM) {
 
 		return err
 	}
-	timeout.WaitForError(ctx, t, vm.Close, fe, fe)
+	timeout.WaitForError(ctx, t, vm.Close, fe)
 }