[test] Update WCOW uVM and vSMB, and HostProcess functional tests #1965
Conversation
helsaawy
force-pushed
the
func-tests
branch
2 times, most recently
from
241c958
to
f056672
Compare
helsaawy
changed the title
helsaawy
force-pushed
the
func-tests
branch
4 times, most recently
from
4c91058
to
2d896c5
Compare
helsaawy
force-pushed
the
func-tests
branch
2 times, most recently
from
f592f71
to
fcb7cc9
Compare
|
Seems like there are a bunch of changes combined in this PR, would it be possible to split it into multiple commits so that it is easier to review? |
WCOW tests can be integrated directly into existing LCOW tests as subtests, after generalizing the original (LCOW-only) tests to run both types of uVMs and containers. Break the change into two: (1) move (and rename) the original LCOW-only tests; and (2) generalize the tests and add the WCOW components. To simplify the diffs, this commit only includes the first process. Specifically, move: - `lcow_bench_test.go` to `uvm_bench_test.go` - `lcow_container_test.go` to `container_test.go` - `lcow_test.go` to `lcow_uvm_test.go` Within `lcow_uvm_test.go`, combine and generalize kernel arg tests (i.e., `TestLCOW_UVMNoSCSINoVPMemInitrd` and `TestLCOW_UVMNoSCSISingleVPMemVHD`) to `TestLCOW_UVM_KernelArgs`. Combine and generalize boot/time tests (e.g., `TestLCOW_TimeUVMStartVHD`, `TestLCOW_UVMStart_KernelDirect_VHD`) to `TestLCOW_UVM_Boot`. Also, since go1.21, `"github.com/Microsoft/hcsshim/internal/sync"` is no longer necessary, so replace it with `"sync".OnceValue[s]`. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Un-skip and fix WCOW uVM and container tests. Add WCOW: - uVM benchmarks - vSMB tests - Host Process tests For WCOW host process tests, add dedicated tests for setting username, and verifying hostname and volume mounts. Fix bug where removing a direct-mapped vSMB share fails. Run (non-virtualization/uVM) functional tests within CI. Starting Host Process containers requires SYSTEM to create a process with a specified token, so use PsExec.exe (from sysutils) to run tests. Make sure container specs are created with the default working directory (`C:\`), similar to how `internal\cmd` works). Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
|
|
||
| ioArgs.TestStdOutContains(t, tc.wantArgs, tc.notWantArgs) | ||
|
|
||
| // some boot options (notably using initrd) need to validated by looking at dmesg logs |
There was a problem hiding this comment.
If dmesg output has the entire command line (i.e. what we are reading from /proc/cmdlind plus some other options) why not only read dmesg logs and avoid reading /proc/cmdline?
There was a problem hiding this comment.
basically cause the TestStdOutContains function checks that the sting contains each of args, so unless we first parse the dmesg output for only the command line bit, the check might turn a false positive if the arg we want shows up elsewhere in the logs.
I didn't want to assume a stable output for dmesg, nor a stable ordering of the arguments in the command line (eg, searching for the string root=/dev/sda rootwait init=/init instead of each component individually)
| require.Build(t, osversion.RS5) | ||
| requireFeatures(t, featureLCOW, featureUVM) | ||
|
|
||
| numIters := 3 |
There was a problem hiding this comment.
What is the purpose of doing 3 iterations of this? If it is to catch any concurrency issues, should we do 3 starts in 3 parallel goroutines?
There was a problem hiding this comment.
its in serial, and the original tests had it (
hcsshim/test/functional/lcow_test.go
Line 171 in 575f7f8
| var fileBindingSupportedOnce = sync.OnceValue(func() bool { | ||
| // TODO: use windows.NewLazySystemDLL("bindfltapi.dll").Load() (or windows.LoadLibraryEx directly) | ||
|
|
||
| root := os.Getenv("SystemRoot") | ||
| if root == "" { | ||
| root = `C:\windows` // shouldn't really need this fall back, but ... | ||
| } | ||
| bindDLL := filepath.Join(root, `system32\bindfltapi.dll`) | ||
| st, err := os.Stat(bindDLL) | ||
| return err == nil && st.Mode().IsRegular() | ||
| }) | ||
|
|
There was a problem hiding this comment.
Can we just do this in an init function and set a FileBindingSupported variable?
Also, since this isn't a test code change, we should probably make it into a separate commit, in case we need to revert stuff.
There was a problem hiding this comment.
I can make a separate commit/PR, but my worry with exposing the variable directly is it can be (inadvertently) changed, which id like to prevent doing
| if !errors.Is(errGive, err) { | ||
| tb.Fatalf("got stderr: %v; wanted: %v", errGive, err) | ||
| outGot, errGot := b.Output() | ||
| if !errors.Is(errGot, err) { |
There was a problem hiding this comment.
If errGot is say errors.New("file not found") will it match with os.ErrNotExist?
There was a problem hiding this comment.
nope, errors.Is checks that it matches the original error object, not the message itself
| out = strings.ToLower(strings.TrimSpace(out)) | ||
| outGot = strings.ToLower(strings.TrimSpace(outGot)) | ||
| if diff := cmp.Diff(out, outGot); diff != "" { | ||
| tb.Fatalf("stdout mismatch (-want +got):\n%s", diff) |
There was a problem hiding this comment.
stdout could be huge, probably should avoid logging the entire diff. Same for the function below.
There was a problem hiding this comment.
i tried that, but i found that whenever the test failed, having the stdout in the logs was pretty helpful to debug the failure immediately.
otherwise i went down a rabbit hole of replicating locally and then logging stdout anyways
Update and un-skip WCOW uVM and container tests as well as WCOW vSMB tests.
Add:
Unified container and uVM lifecycle, exec, and IO tests for LCOW and WCOW.
Moved tests from:
lcow_bench_test.gotouvm_bench_test.golcow_container_test.gotocontainer_test.golcow_test.gotolcow_uvm_test.goanduvm_test.goFix bug where removing a direct-mapped vSMB share failes.
Run (non-virtualization/uVM) functional tests within CI.
Starting Host Process containers requires SYSTEM to create a process with a specified token, so use PsExec.exe (from sysutils) to run tests.
PR is split into two commits: the first does preparatory reorg work, specifically for LCOW-only tests that are generalized to work on both LCOW and WCOW in the subsequent commit.