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

Features/bugfixes to support LCOW GCS tests #1360

Merged
merged 1 commit into from May 3, 2022

Conversation

helsaawy
Copy link
Contributor

Exposed data and functionality for testing GCS:

  • internal\guest\runtime\hcsv2.Container.ID()
  • internal\guest\runtime\hcsv2.Container.InitProcess()
  • internal\guest\runtime\hcsv2.GetOrAddNetworkNamespace()
  • internal\guest\runtime\hcsv2.RemoveNetworkNamespace()
  • internal\guest\runtime\hcsv2.Host.SecurityPolicyEnforcer()
  • internal\guest\runtime\hcsv2.Host.Transport()

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

Fixed bug where host.RemoveContainer did not remove the network
namespace for standalone and pod containers.

Updated go-winio version to include bugfixes for closing hvsockets,
specifically to close a socket for writing (needed by internal\cmd
to signal that the stdin stream has finished).

Tests are in these PRs: #1352 #1351

PR Broken off from: #1349

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

@helsaawy helsaawy requested a review from a team as a code owner April 19, 2022 19:42
@helsaawy helsaawy changed the title GCS tests and features/bugfixes to support them Features/bugfixes to support LCOW GCS tests Apr 19, 2022
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.

lgtm. small question.

internal/guest/runtime/hcsv2/container.go Show resolved Hide resolved
internal/guest/runtime/hcsv2/network.go Show resolved Hide resolved
@helsaawy
Copy link
Contributor Author

Rebased to resolve conflict

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

dcantah commented May 3, 2022

Is the winio bump needed to get the tests up and running (or just fixes a longstanding issue with windows and linux ctrs in this repo)? I'd prefer to just have a separate pr/commit for that if possible where the description goes over what's in the new payload and what it fixes for us here instead of being jumbled with this

Exposed data and functionality for testing GCS:
* `internal\guest\runtime\hcsv2.Container.InitProcess()`
* `internal\guest\runtime\hcsv2.GetOrAddNetworkNamespace()`
* `internal\guest\runtime\hcsv2.RemoveNetworkNamespace()`
* `internal\guest\runtime\hcsv2.Host.SecurityPolicyEnforcer()`
* `internal\guest\runtime\hcsv2.Host.Transport()`

Fixed bug where `host.RemoveContainer` did not remove the network
namespace for standalone and pod containers.

Updated go-winio version to include bugfixes for closing hvsockets,
specifically to close a socket for writing (needed by internal\cmd
to signal that the stdin stream has finished).

Added doc.go files to guest packages to prevent linter/compiler errors
under windows.

The tests themselves are broken out here:
microsoft#1352
microsoft#1351

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

helsaawy commented May 3, 2022

Is the winio bump needed to get the tests up and running (or just fixes a longstanding issue with windows and linux ctrs in this repo)? I'd prefer to just have a separate pr/commit for that if possible where the description goes over what's in the new payload and what it fixes for us here instead of being jumbled with this

Yeah, its needed to get the tests running. The functional tests were hanging cause of the bug in hvsock where we dont close the pipes properly (microsoft/go-winio#231)

Copy link
Contributor

@dcantah dcantah left a comment

Choose a reason for hiding this comment

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

Gotcha, lgtm

@helsaawy helsaawy merged commit 1e01dcc into microsoft:master May 3, 2022
@helsaawy helsaawy deleted the he/funcsupport branch May 3, 2022 22:42
anmaxvl added a commit that referenced this pull request Feb 7, 2023
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