Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for LCOW tests and benchmarks #1349

Closed
wants to merge 2 commits into from

Conversation

helsaawy
Copy link
Contributor

@helsaawy helsaawy commented Apr 13, 2022

Updated go-winio to include bugfixes for closing hvsockets, specifically for writing (which is needed by internal\cmd to signal that the stdin stream has finished.

Exposed or added certain functionality needed by tests and benchmarks both in the functional directory and uploaded directly onto a Linux UVM. For example, ability to access an hcsv2 containers init process and ID, as well creating and deleting network namespaces on the Linux UVM.

Added more functionality to internal\tools\uvmboot, to include adding SCSI VHD and file mounts, as well as disabling the time sync, and setting the uVM security policy.

Bug in Linux GCS where namespaces are not deleted when the pod or standalone container is deleted.

Added winapi call to check if process is elevated, to allow returning a clear error message before attempting to start uVMs.

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

@helsaawy helsaawy force-pushed the he/testsupport branch 2 times, most recently from 75f96e0 to 7e26c0e Compare April 13, 2022 19:47
@helsaawy helsaawy marked this pull request as ready for review April 13, 2022 21:18
@helsaawy helsaawy requested a review from a team as a code owner April 13, 2022 21:18
@kevpar
Copy link
Member

kevpar commented Apr 13, 2022

My initial impression is there are a lot of different changes here. Can we break this up into a bunch of separate PRs? That would make it much easier to review.

@helsaawy
Copy link
Contributor Author

helsaawy commented Apr 13, 2022

My initial impression is there are a lot of different changes here. Can we break this up into a bunch of separate PRs? That would make it much easier to review.

I split it out, so this is predominantly changes to hcsshim itself that support functional and GCS tests, and the tests are here #1351 and #1352
They rely on all these changes, so I'll keep it in draft format for now

@helsaawy helsaawy mentioned this pull request Apr 13, 2022
3 tasks
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort of agree with @kevpar here, maybe refactoring and adding getter functions can be in a separate PR.

internal/guest/runtime/hcsv2/process.go Outdated Show resolved Hide resolved
internal/tools/uvmboot/mounts.go Show resolved Hide resolved
@@ -664,7 +664,7 @@ func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcs
}

if log.IsScrubbingEnabled() {
opts.ExecCommandLine += " --scrub-logs"
opts.ExecCommandLine += " -scrub-logs"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a different PR?

internal/tools/uvmboot/lcow.go Show resolved Hide resolved
internal/tools/uvmboot/lcow.go Outdated Show resolved Hide resolved
internal/tools/uvmboot/mounts.go Outdated Show resolved Hide resolved
internal/tools/uvmboot/mounts.go Show resolved Hide resolved
Updated go-winio to include bugfixes for closing hvsockets, specifically
for writing (which is needed by `internal\cmd` to signal that the stdin
stream has finished.

Expose or add certain functionality needed by tests and benchmarks both
in the functional directory and uploaded directly onto a Linux UVM.
For example, ability to access an hcsv2 containers init process and ID,
as well creating and deleting network namespaces on the Linux UVM.

Added more functionality to `internal\tools\uvmboot`, to include adding
SCSI VHD and file mounts, as well as disabling the time sync, and setting
the uVM security policy.

Added winapi call to check if process is elevated, to allow returning a
clear error message before attempting to start uVMs.

Split out `internal\guest\runtime\runc\runc.go` into `runc.go`,
`container.go` and `process.go`.

Reorganized makefile to read from top to bottom, and added additional
files to LCOW rootfs that include the time stamp of of the vhd creation,
the image build date, and its full name (pulled from a
`*.testdata.json` file in the LSG release, that appears to be
the only location of that information).

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
@helsaawy
Copy link
Contributor Author

@anmaxvl Do you have a preference for how to split things up?
Should I do:

  1. uvmboot
  2. Internal code refactoring
  3. Updates to test

@anmaxvl
Copy link
Contributor

anmaxvl commented Apr 19, 2022

@anmaxvl Do you have a preference for how to split things up? Should I do:

  1. uvmboot
  2. Internal code refactoring
  3. Updates to test

above sounds reasonable to me. it also seems like we have some unrelated .gitignore updates in this PR.

@helsaawy
Copy link
Contributor Author

helsaawy commented Apr 19, 2022

Closing and splitting into two PRs:
#1359
#1360

Tests themselves are:
#1351
#1352

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants