Skip to content

Commit

Permalink
Add WCOW and vSMB functional tests
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
helsaawy committed Apr 10, 2024
1 parent 87288f4 commit e00dfc0
Show file tree
Hide file tree
Showing 14 changed files with 1,939 additions and 443 deletions.
59 changes: 56 additions & 3 deletions .github/workflows/ci.yml
Expand Up @@ -325,6 +325,30 @@ jobs:
- name: Install gotestsum
run: go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }}

# Download PsExec so we can run (functional) tests as 'NT Authority\System'.
# Needed for hostprocess tests, as well ensuring backup and restore privileges for
# unpacking WCOW images.
- name: Install PsExec.exe
run: |
New-Item -ItemType Directory -Force '${{ github.workspace }}\bin' > $null
'${{ github.workspace }}\bin' | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
curl.exe -L --no-progress-meter --fail-with-body -o 'C:\PSTools.zip' `
'https://download.sysinternals.com/files/PSTools.zip' 2>&1
if ( $LASTEXITCODE ) {
Write-Output '::error::Could not download PSTools.zip'
exit $LASTEXITCODE
}
tar.exe xf 'C:\PSTools.zip' -C '${{ github.workspace }}\bin' 'PsExec*' 2>&1
if ( $LASTEXITCODE ) {
Write-Output '::error::Could not extract PsExec.exe'
exit $LASTEXITCODE
}
# accept the eula
& '${{ github.workspace }}/bin/psexec' -accepteula -nobanner cmd /c "exit 0" 2>$null
# run tests
- name: Test repo
run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags admin -timeout=20m ./...
Expand Down Expand Up @@ -354,13 +378,42 @@ jobs:
${{ env.GOTESTSUM_CMD_RAW }} ./containerd-shim-runhcs-v1.test.exe '-test.v'
working-directory: test

- name: Build and run functional testing binary
run: |
${{ env.GO_BUILD_TEST_CMD }} ./functional
if ( $LASTEXITCODE ) {
Write-Output '::error::Could not build functional.test.exe'
exit $LASTEXITCODE
}
# PsExec doesn't load GOBIN into path, so resolve gotestsum path
$gotestsum = Get-Command -Name 'gotestsum' -CommandType Application -ErrorAction 'Stop' |
Select-Object -First 1 -ExpandProperty Source
if ( [string]::IsNullOrEmpty($gotestsum) ) {
Write-Output '::error::could not find 'gotestsum.exe' path'
exit $LASTEXITCODE
}
# Don't run uVM (ie, nested virt) or LCOW integrity tests
$cmd = '${{ env.GOTESTSUM_CMD_RAW }} ./functional.test.exe -exclude=LCOW,LCOWIntegrity,uVM -test.timeout=1h -test.v -log-level=info'
$cmd = $cmd -replace 'gotestsum', $gotestsum
Write-Host "gotestsum command: $cmd"
# Apparently, in a GH runner, PsExec always runs noninteractively (even with `-i`)
# and doesn't capture or redirect std IO.
# So redirect stdout/stderr to a file.
psexec -nobanner -w (Get-Location) -s cmd /c "$cmd > out.txt 2>&1"
$ec = $LASTEXITCODE
Get-Content out.txt
exit $ec
working-directory: test

# build testing binaries
- name: Build cri-containerd Testing Binary
run: ${{ env.GO_BUILD_TEST_CMD }} ./cri-containerd
working-directory: test
- name: Build functional Testing Binary
run: ${{ env.GO_BUILD_TEST_CMD }} ./functional
working-directory: test
- name: Build runhcs Testing Binary
run: ${{ env.GO_BUILD_TEST_CMD }} ./runhcs
working-directory: test
Expand Down
18 changes: 3 additions & 15 deletions internal/jobcontainers/jobcontainer.go
Expand Up @@ -31,11 +31,6 @@ import (
"golang.org/x/sys/windows"
)

var (
fileBindingSupport bool
checkBindSupportOnce sync.Once
)

const (
// jobContainerNameFmt is the naming format that job objects for job containers will follow.
jobContainerNameFmt = "JobContainer_%s"
Expand Down Expand Up @@ -186,15 +181,8 @@ func Create(ctx context.Context, id string, s *specs.Spec, createOpts CreateOpti
// show up at beforehand as you would need to know the containers ID before you launched it. Now that the
// rootfs location can be static, a user can easily supply C:\hpc\rest\of\path as their work dir and still
// supply anything outside of C:\hpc if they want another location on the host.
checkBindSupportOnce.Do(func() {
bindDLL := `C:\windows\system32\bindfltapi.dll`
if _, err := os.Stat(bindDLL); err == nil {
fileBindingSupport = true
}
})

var closer resources.ResourceCloser
if fileBindingSupport {
if FileBindingSupported() {
closer, err = container.bindSetup(ctx, s, createOpts)
} else {
closer, err = container.fallbackSetup(ctx, s, createOpts)
Expand Down Expand Up @@ -259,7 +247,7 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_
// If the working directory was changed, that means the user supplied %CONTAINER_SANDBOX_MOUNT_POINT%\\my\dir or something similar.
// In that case there's nothing left to do, as we don't want to join it with the mount point again.. If it *wasn't* changed, and there's
// no bindflt support then we need to join it with the mount point, as it's some normal path.
if !changed && !fileBindingSupport {
if !changed && !FileBindingSupported() {
workDir = filepath.Join(c.rootfsLocation, removeDriveLetter(workDir))
}
}
Expand Down Expand Up @@ -340,7 +328,7 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_
// (cmd in this case) after launch can now see C:\<rootfslocation> as it's in the silo. We could
// also add a new mode/flag for the shim where it's just a dummy process launcher, so we can invoke
// the shim instead of cmd and have more control over things.
if fileBindingSupport {
if FileBindingSupported() {
commandLine = "cmd /c " + commandLine
}

Expand Down
33 changes: 29 additions & 4 deletions internal/jobcontainers/storage.go
Expand Up @@ -5,24 +5,34 @@ package jobcontainers
import (
"context"
"fmt"
"os"
"path/filepath"
"sync"

specs "github.com/opencontainers/runtime-spec/specs-go"

"github.com/Microsoft/hcsshim/internal/layers"
"github.com/Microsoft/hcsshim/internal/log"
"github.com/Microsoft/hcsshim/internal/resources"
specs "github.com/opencontainers/runtime-spec/specs-go"
)

// fallbackRootfsFormat is the fallback location for the rootfs if file binding support isn't available.
// %s will be expanded with the container ID. Trailing backslash required for SetVolumeMountPoint and
// DeleteVolumeMountPoint
// DeleteVolumeMountPoint.
const fallbackRootfsFormat = `C:\hpc\%s\`

// defaultSiloRootfsLocation is the default location the rootfs for the container will show up
// inside of a given silo. If bind filter support isn't available the rootfs will be
// C:\hpc\<containerID>
// C:\hpc\<containerID>.
const defaultSiloRootfsLocation = `C:\hpc\`

func (c *JobContainer) mountLayers(ctx context.Context, containerID string, s *specs.Spec, wl layers.WCOWLayers, volumeMountPath string) (_ resources.ResourceCloser, err error) {
func (*JobContainer) mountLayers(
ctx context.Context,
containerID string,
s *specs.Spec,
wl layers.WCOWLayers,
volumeMountPath string,
) (_ resources.ResourceCloser, err error) {
if s.Root == nil {
s.Root = &specs.Root{}
}
Expand Down Expand Up @@ -75,3 +85,18 @@ func (c *JobContainer) setupRootfsBinding(root, target string) error {
}
return nil
}

// don't expose fileBindingSupportedOnce directly, since it can be reassigned.
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()
})

func FileBindingSupported() bool { return fileBindingSupportedOnce() }
25 changes: 24 additions & 1 deletion internal/uvm/vsmb.go
Expand Up @@ -133,7 +133,12 @@ func forceNoDirectMap(path string) (bool, error) {
var info winapi.FILE_ID_INFO
// We check for any error, rather than just ERROR_INVALID_PARAMETER. It seems better to also
// fall back if e.g. some other backing filesystem is used which returns a different error.
if err := windows.GetFileInformationByHandleEx(h, winapi.FileIdInfo, (*byte)(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info))); err != nil {
if err := windows.GetFileInformationByHandleEx(
h,
winapi.FileIdInfo,
(*byte)(unsafe.Pointer(&info)),
uint32(unsafe.Sizeof(info)),
); err != nil {
return true, nil
}
return false, nil
Expand Down Expand Up @@ -282,6 +287,24 @@ func (uvm *UtilityVM) removeVSMB(ctx context.Context, hostPath string, readOnly,
return nil
}

// Cannot remove a directmapped vSMB share without first closing all open handles to the
// share files from inside the the uVM (otherwise, the removal would un-map the files from
// the uVM's memory and subsequent access's would fail).
// Rather than forgetting about the share on the host side, keep it (with refCount == 0)
// in case that directory is re-added back for some reason.
//
// Note: HCS (vmcompute.exe) issues a remove vSMB request to the guest GCS iff:
// - vmwp.exe direct mapped the vSMB share; and
// - the GCS (on its internal bridge) has the PurgeVSmbCachedHandlesSupported capability.
// We do not (currently) have the ability to check for either.
if !share.options.NoDirectmap {
log.G(ctx).WithFields(logrus.Fields{
"name": share.name,
"path": hostPath,
}).Debug("skipping remove of directmapped vSMB share")
return nil
}

modification := &hcsschema.ModifySettingRequest{
RequestType: guestrequest.RequestTypeRemove,
Settings: hcsschema.VirtualSmbShare{Name: share.name},
Expand Down

0 comments on commit e00dfc0

Please sign in to comment.