From a5874d7a98bb316b8a3b70343ed2a58cc2d919a5 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Mon, 16 Dec 2024 15:23:29 -0500 Subject: [PATCH] Use abs path to testing binary Use the full path to the `functional.test.exe` binary when sharing into the uVM or container for the `TestHVSock_*` test cases in `test\functional\hvsock_test.go` to prevent vSMB share issues. Otherwise, `os.Args[0]` will return the path that the tests were run with (e.g., `.\functional.test.exe`), which can cause vSMB to fail with `The parameter is incorrect.` (likely because it cannot find the current file). Signed-off-by: Hamza El-Saawy --- test/functional/hvsock_test.go | 61 +++++++++++++++++++--------- test/internal/util/reexec_windows.go | 29 +++++++++++++ test/internal/util/util_windows.go | 27 ++++++++++++ 3 files changed, 97 insertions(+), 20 deletions(-) create mode 100644 test/internal/util/reexec_windows.go diff --git a/test/functional/hvsock_test.go b/test/functional/hvsock_test.go index a762692268..4e1f3a0949 100644 --- a/test/functional/hvsock_test.go +++ b/test/functional/hvsock_test.go @@ -8,8 +8,6 @@ import ( "fmt" "io" "net" - "os" - "path/filepath" "sync" "testing" "time" @@ -22,6 +20,7 @@ import ( "golang.org/x/sys/windows" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/osversion" testcmd "github.com/Microsoft/hcsshim/test/internal/cmd" @@ -134,8 +133,7 @@ func TestHVSock_UVM_HostBind(t *testing.T) { return err }) - guestPath := filepath.Join(`C:\`, filepath.Base(os.Args[0])) - testuvm.Share(ctx, t, vm, os.Args[0], guestPath, true) + guestPath := reExecSelfShareUVM(ctx, t, vm, "") reexecCmd := fmt.Sprintf(`%s -test.run=%s`, guestPath, util.TestNameRegex(t)) if testing.Verbose() { @@ -231,8 +229,7 @@ func TestHVSock_UVM_GuestBind(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, hvsockTestTimeout) //nolint:govet // ctx is shadowed t.Cleanup(cancel) - guestPath := filepath.Join(`C:\`, filepath.Base(os.Args[0])) - testuvm.Share(ctx, t, vm, os.Args[0], guestPath, true) + guestPath := reExecSelfShareUVM(ctx, t, vm, "") reexecCmd := fmt.Sprintf(`%s -test.run=%s`, guestPath, util.TestNameRegex(t)) if testing.Verbose() { @@ -353,11 +350,7 @@ func TestHVSock_Container_HostBind(t *testing.T) { vm := testuvm.CreateAndStart(ctx, t, opts) - guestPath := filepath.Join(`C:\`, filepath.Base(os.Args[0])) - reexecCmd := fmt.Sprintf(`%s -test.run=%s`, guestPath, util.TestNameRegex(t)) - if testing.Verbose() { - reexecCmd += " -test.v" - } + hostPath, guestPath, reExecCmd := reExecSelfCmd(ctx, t, "") cID := vm.ID() + "-container" scratch := layers.WCOWScratchDir(ctx, t, "") @@ -366,9 +359,9 @@ func TestHVSock_Container_HostBind(t *testing.T) { testoci.WithWindowsLayerFolders(append(windowsImageLayers(ctx, t), scratch)), ctrdoci.WithUsername(`NT AUTHORITY\SYSTEM`), ctrdoci.WithEnv([]string{util.ReExecEnv + "=1"}), - ctrdoci.WithProcessCommandLine(reexecCmd), + ctrdoci.WithProcessCommandLine(reExecCmd), ctrdoci.WithMounts([]specs.Mount{{ - Source: os.Args[0], + Source: hostPath, Destination: guestPath, Options: []string{"ro"}, }}), @@ -501,11 +494,7 @@ func TestHVSock_Container_GuestBind(t *testing.T) { vm := testuvm.CreateAndStart(ctx, t, opts) - guestPath := filepath.Join(`C:\`, filepath.Base(os.Args[0])) - reexecCmd := fmt.Sprintf(`%s -test.run=%s`, guestPath, util.TestNameRegex(t)) - if testing.Verbose() { - reexecCmd += " -test.v" - } + hostPath, guestPath, reExecCmd := reExecSelfCmd(ctx, t, "") cID := vm.ID() + "-container" scratch := layers.WCOWScratchDir(ctx, t, "") @@ -514,9 +503,9 @@ func TestHVSock_Container_GuestBind(t *testing.T) { testoci.WithWindowsLayerFolders(append(windowsImageLayers(ctx, t), scratch)), ctrdoci.WithUsername(`NT AUTHORITY\SYSTEM`), ctrdoci.WithEnv([]string{util.ReExecEnv + "=1"}), - ctrdoci.WithProcessCommandLine(reexecCmd), + ctrdoci.WithProcessCommandLine(reExecCmd), ctrdoci.WithMounts([]specs.Mount{{ - Source: os.Args[0], + Source: hostPath, Destination: guestPath, Options: []string{"ro"}, }}), @@ -1148,3 +1137,35 @@ func goBlockT[T any](f func() T) <-chan T { return ch } + +// reExecSelfShareUVM shares the current testing binary directly into the specified uVM, +// +// This assumes that binary will be run directly on a uVM, and not from within a container. +// For the later case, see [reExecSelfCmd]. +// +// See [util.ReExecSelfGuestPath] for information on generating the guest path. +func reExecSelfShareUVM(ctx context.Context, tb testing.TB, vm *uvm.UtilityVM, base string) string { + tb.Helper() + + self, guestPath := util.ReExecSelfGuestPath(ctx, tb, base) + testuvm.Share(ctx, tb, vm, self, guestPath, true) + + return guestPath +} + +// reExecSelfCmd returns the host and guest path for the current test binary, as well as +// the appropriate re-exec command to configure the container spec with. +// +// See [reExecSelfShareUVM] for more details. +func reExecSelfCmd(ctx context.Context, tb testing.TB, base string) (string, string, string) { + tb.Helper() + + self, guestPath := util.ReExecSelfGuestPath(ctx, tb, base) + c := fmt.Sprintf(`%s -test.run=%s`, guestPath, util.TestNameRegex(tb)) + + if testing.Verbose() { + c += " -test.v" + } + + return self, guestPath, c +} diff --git a/test/internal/util/reexec_windows.go b/test/internal/util/reexec_windows.go new file mode 100644 index 0000000000..4641bf5c19 --- /dev/null +++ b/test/internal/util/reexec_windows.go @@ -0,0 +1,29 @@ +//go:build windows + +package util + +import ( + "context" + "path/filepath" + "testing" +) + +// windows only because TestingBinaryPath is Windows only as well, and the default path is `C:\` + +// Default path to share the current testing binary under. +const ReExecDefaultGuestPathBase = `C:\` + +// ReExecSelfGuestPath returns the path to the current testing binary, as well as the +// path should be shared (under the path specified by base). +// +// If base is empty, [ReExecDefaultGuestPathBase] will be used. +func ReExecSelfGuestPath(ctx context.Context, tb testing.TB, base string) (string, string) { + tb.Helper() + + if base == "" { + base = `C:\` + } + + self := TestingBinaryPath(ctx, tb) + return self, filepath.Join(base, filepath.Base(self)) +} diff --git a/test/internal/util/util_windows.go b/test/internal/util/util_windows.go index 36be3cc6ee..1c5b1e8651 100644 --- a/test/internal/util/util_windows.go +++ b/test/internal/util/util_windows.go @@ -4,7 +4,12 @@ package util import ( "context" + "fmt" "os" + "sync" + "testing" + + "github.com/Microsoft/go-winio/pkg/fs" "github.com/Microsoft/hcsshim/internal/wclayer" ) @@ -17,3 +22,25 @@ func DestroyLayer(ctx context.Context, p string) (err error) { } return repeat(func() error { return wclayer.DestroyLayer(ctx, p) }, RemoveAttempts, RemoveWait) } + +// executablePathOnce returns the current testing binary image +var executablePathOnce = sync.OnceValues(func() (string, error) { + // use [os.Executable] over `os.Args[0]` to make sure path is absolute + // as always, this shouldn't really fail, but just to be safe... + p, err := os.Executable() + if err != nil { + return "", fmt.Errorf("retrieve executable path: %w", err) + } + // just to be safe (and address the comments on [os.Executable]), resolve the path + return fs.ResolvePath(p) +}) + +func TestingBinaryPath(_ context.Context, tb testing.TB) string { + tb.Helper() + + p, err := executablePathOnce() + if err != nil { + tb.Fatalf("could not get testing binary path: %v", err) + } + return p +}