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

[release/0.11] Support adding mount to running containers #1936

Merged
merged 1 commit into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@ import (
"context"
"fmt"
"math/rand"
"os"
"path/filepath"
"strconv"
"testing"
"time"

"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
"github.com/Microsoft/hcsshim/internal/hcsoci"
"github.com/Microsoft/hcsshim/pkg/ctrdtaskapi"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/runtime/v2/task"
"github.com/containerd/typeurl"
Expand Down Expand Up @@ -535,6 +539,67 @@ func Test_TaskShim_updateInternal_Success(t *testing.T) {
}
}

// Tests if a requested mount is valid for windows containers.
// Currently only host volumes/directories are supported to be mounted
// on a running windows container.
func Test_TaskShimWindowsMount_updateInternal_Success(t *testing.T) {
s, t1, _ := setupTaskServiceWithFakes(t)
t1.isWCOW = true

hostRWSharedDirectory := t.TempDir()
fRW, _ := os.OpenFile(filepath.Join(hostRWSharedDirectory, "readwrite"), os.O_RDWR|os.O_CREATE, 0755)
fRW.Close()

resources := &ctrdtaskapi.ContainerMount{
HostPath: hostRWSharedDirectory,
ContainerPath: hostRWSharedDirectory,
ReadOnly: true,
Type: "",
}
any, err := typeurl.MarshalAny(resources)
if err != nil {
t.Fatal(err)
}

resp, err := s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t1.ID(), Resources: any})
if err != nil {
t.Fatalf("should not have failed update mount with error, got: %v", err)
}
if resp == nil {
t.Fatalf("should have returned an empty resp")
}
}

func Test_TaskShimWindowsMount_updateInternal_Error(t *testing.T) {
s, t1, _ := setupTaskServiceWithFakes(t)
t1.isWCOW = true

hostRWSharedDirectory := t.TempDir()
tmpVhdPath := filepath.Join(hostRWSharedDirectory, "test-vhd.vhdx")

fRW, _ := os.OpenFile(filepath.Join(tmpVhdPath, "readwrite"), os.O_RDWR|os.O_CREATE, 0755)
fRW.Close()

resources := &ctrdtaskapi.ContainerMount{
HostPath: tmpVhdPath,
ContainerPath: tmpVhdPath,
ReadOnly: true,
Type: hcsoci.MountTypeVirtualDisk,
}
any, err := typeurl.MarshalAny(resources)
if err != nil {
t.Fatal(err)
}

resp, err := s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t1.ID(), Resources: any})
if err == nil {
t.Fatalf("should have failed update mount with error")
}
if resp != nil {
t.Fatalf("should have returned a nil resp, got: %v", resp)
}
}

func Test_TaskShim_updateInternal_Error(t *testing.T) {
s, t1, _ := setupTaskServiceWithFakes(t)

Expand Down
1 change: 1 addition & 0 deletions cmd/containerd-shim-runhcs-v1/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func verifyTaskUpdateResourcesType(data interface{}) error {
case *specs.WindowsResources:
case *specs.LinuxResources:
case *ctrdtaskapi.PolicyFragment:
case *ctrdtaskapi.ContainerMount:
default:
return errNotSupportedResourcesRequest
}
Expand Down
106 changes: 100 additions & 6 deletions cmd/containerd-shim-runhcs-v1/task_hcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"sync"
"time"

Expand All @@ -20,10 +21,12 @@ import (
"github.com/sirupsen/logrus"
"go.opencensus.io/trace"

"github.com/Microsoft/go-winio/pkg/fs"
runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
"github.com/Microsoft/hcsshim/internal/cmd"
"github.com/Microsoft/hcsshim/internal/cow"
"github.com/Microsoft/hcsshim/internal/guestpath"
"github.com/Microsoft/hcsshim/internal/hcs"
"github.com/Microsoft/hcsshim/internal/hcs/resourcepaths"
"github.com/Microsoft/hcsshim/internal/hcs/schema1"
Expand All @@ -42,6 +45,7 @@ import (
"github.com/Microsoft/hcsshim/internal/uvm"
"github.com/Microsoft/hcsshim/osversion"
"github.com/Microsoft/hcsshim/pkg/annotations"
"github.com/Microsoft/hcsshim/pkg/ctrdtaskapi"
)

func newHcsStandaloneTask(ctx context.Context, events publisher, req *task.CreateTaskRequest, s *specs.Spec) (shimTask, error) {
Expand Down Expand Up @@ -838,7 +842,15 @@ func (ht *hcsTask) Update(ctx context.Context, req *task.UpdateTaskRequest) erro

func (ht *hcsTask) updateTaskContainerResources(ctx context.Context, data interface{}, annotations map[string]string) error {
if ht.isWCOW {
return ht.updateWCOWResources(ctx, data, annotations)
switch resources := data.(type) {
case *specs.WindowsResources:
return ht.updateWCOWResources(ctx, resources, annotations)
case *ctrdtaskapi.ContainerMount:
// Adding mount to a running container is currently only supported for windows containers
return ht.updateWCOWContainerMount(ctx, resources, annotations)
default:
return errNotSupportedResourcesRequest
}
}

return ht.updateLCOWResources(ctx, data, annotations)
Expand Down Expand Up @@ -874,11 +886,7 @@ func isValidWindowsCPUResources(c *specs.WindowsCPUResources) bool {
(c.Maximum != nil && (c.Count == nil && c.Shares == nil))
}

func (ht *hcsTask) updateWCOWResources(ctx context.Context, data interface{}, annotations map[string]string) error {
resources, ok := data.(*specs.WindowsResources)
if !ok {
return errors.New("must have resources be type *WindowsResources when updating a wcow container")
}
func (ht *hcsTask) updateWCOWResources(ctx context.Context, resources *specs.WindowsResources, annotations map[string]string) error {
if resources.Memory != nil && resources.Memory.Limit != nil {
newMemorySizeInMB := *resources.Memory.Limit / memory.MiB
memoryLimit := hcsoci.NormalizeMemorySize(ctx, ht.id, newMemorySizeInMB)
Expand Down Expand Up @@ -937,3 +945,89 @@ func (ht *hcsTask) ProcessorInfo(ctx context.Context) (*processorInfo, error) {
count: ht.host.ProcessorCount(),
}, nil
}

func (ht *hcsTask) requestAddContainerMount(ctx context.Context, resourcePath string, settings interface{}) error {
modification := &hcsschema.ModifySettingRequest{
ResourcePath: resourcePath,
RequestType: guestrequest.RequestTypeAdd,
Settings: settings,
}
return ht.c.Modify(ctx, modification)
}

func isMountTypeSupported(hostPath, mountType string) bool {
// currently we only support mounting of host volumes/directories
switch mountType {
case hcsoci.MountTypeBind, hcsoci.MountTypePhysicalDisk,
hcsoci.MountTypeVirtualDisk, hcsoci.MountTypeExtensibleVirtualDisk:
return false
default:
// Ensure that host path is not sandbox://, hugepages://
if strings.HasPrefix(hostPath, guestpath.SandboxMountPrefix) ||
strings.HasPrefix(hostPath, guestpath.HugePagesMountPrefix) ||
strings.HasPrefix(hostPath, guestpath.PipePrefix) {
return false
} else {
// hcsshim treats mountType == "" as a normal directory mount
// and this is supported
return mountType == ""
}
}
}

func (ht *hcsTask) updateWCOWContainerMount(ctx context.Context, resources *ctrdtaskapi.ContainerMount, annotations map[string]string) error {
// Hcsschema v2 should be supported
if osversion.Build() < osversion.RS5 {
// OSVerions < RS5 only support hcsshema v1
return fmt.Errorf("hcsschema v1 unsupported")
}

if resources.HostPath == "" || resources.ContainerPath == "" {
return fmt.Errorf("invalid OCI spec - a mount must have both host and container path set")
}

// Check for valid mount type
if !isMountTypeSupported(resources.HostPath, resources.Type) {
return fmt.Errorf("invalid mount type %v. Currently only host volumes/directories can be mounted to running containers", resources.Type)
}

if ht.host == nil {
// HCS has a bug where it does not correctly resolve file (not dir) paths
// if the path includes a symlink. Therefore, we resolve the path here before
// passing it in. The issue does not occur with VSMB, so don't need to worry
// about the isolated case.
hostPath, err := fs.ResolvePath(resources.HostPath)
if err != nil {
return errors.Wrapf(err, "failed to resolve path for hostPath %s", resources.HostPath)
}

// process isolated windows container
settings := hcsschema.MappedDirectory{
HostPath: hostPath,
ContainerPath: resources.ContainerPath,
ReadOnly: resources.ReadOnly,
}
if err := ht.requestAddContainerMount(ctx, resourcepaths.SiloMappedDirectoryResourcePath, settings); err != nil {
return errors.Wrapf(err, "failed to add mount to process isolated container")
}
} else {
// if it is a mount request for a running hyperV WCOW container, we should first mount volume to the
// UVM as a VSMB share and then mount to the running container using the src path as seen by the UVM
vsmbShare, guestPath, err := ht.host.AddVsmbAndGetSharePath(ctx, resources.HostPath, resources.ContainerPath, resources.ReadOnly)
if err != nil {
return err
}
// Add mount to list of resources to be released on container cleanup
ht.cr.Add(vsmbShare)

settings := hcsschema.MappedDirectory{
HostPath: guestPath,
ContainerPath: resources.ContainerPath,
ReadOnly: resources.ReadOnly,
}
if err := ht.requestAddContainerMount(ctx, resourcepaths.SiloMappedDirectoryResourcePath, settings); err != nil {
return errors.Wrapf(err, "failed to add mount to hyperV container")
}
}
return nil
}
20 changes: 19 additions & 1 deletion cmd/containerd-shim-runhcs-v1/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
"github.com/Microsoft/hcsshim/internal/shimdiag"
"github.com/Microsoft/hcsshim/pkg/ctrdtaskapi"
v1 "github.com/containerd/cgroups/stats/v1"
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/runtime/v2/task"
Expand Down Expand Up @@ -107,7 +108,24 @@ func (tst *testShimTask) Update(ctx context.Context, req *task.UpdateTaskRequest
if err != nil {
return errors.Wrapf(err, "failed to unmarshal resources for container %s update request", req.ID)
}
return verifyTaskUpdateResourcesType(data)
if err := verifyTaskUpdateResourcesType(data); err != nil {
return err
}

if tst.isWCOW {
switch request := data.(type) {
case *ctrdtaskapi.ContainerMount:
// Adding mount to a running container is currently only supported for windows containers
if isMountTypeSupported(request.HostPath, request.Type) {
return nil
} else {
return errNotSupportedResourcesRequest
}
default:
return nil
}
}
return nil
}

func (tst *testShimTask) Share(ctx context.Context, req *shimdiag.ShareRequest) error {
Expand Down
2 changes: 2 additions & 0 deletions internal/guestpath/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ const (
// HugePagesMountPrefix is mount prefix used in container spec to mark a
// huge-pages mount
HugePagesMountPrefix = "hugepages://"
// PipePrefix is the mount prefix used in container spec to mark a named pipe
PipePrefix = `\\.\pipe`
// LCOWMountPathPrefixFmt is the path format in the LCOW UVM where
// non-global mounts, such as Plan9 mounts are added
LCOWMountPathPrefixFmt = "/mounts/m%d"
Expand Down
34 changes: 22 additions & 12 deletions internal/uvm/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,34 @@ import (
hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
"github.com/Microsoft/hcsshim/internal/protocol/guestrequest"
"github.com/Microsoft/hcsshim/internal/protocol/guestresource"
"github.com/pkg/errors"
)

func (uvm *UtilityVM) AddVsmbAndGetSharePath(ctx context.Context, reqHostPath, reqUVMPath string, readOnly bool) (*VSMBShare, string, error) {
options := uvm.DefaultVSMBOptions(readOnly)
vsmbShare, err := uvm.AddVSMB(ctx, reqHostPath, options)
if err != nil {
return nil, "", errors.Wrapf(err, "failed to add mount as vSMB share to UVM")
}
defer func() {
if err != nil {
_ = vsmbShare.Release(ctx)
}
}()

sharePath, err := uvm.GetVSMBUvmPath(ctx, reqHostPath, readOnly)
if err != nil {
return nil, "", errors.Wrapf(err, "failed to get vsmb path")
}

return vsmbShare, sharePath, nil
}

// Share shares in file(s) from `reqHostPath` on the host machine to `reqUVMPath` inside the UVM.
// This function handles both LCOW and WCOW scenarios.
func (uvm *UtilityVM) Share(ctx context.Context, reqHostPath, reqUVMPath string, readOnly bool) (err error) {
if uvm.OS() == "windows" {
options := uvm.DefaultVSMBOptions(readOnly)
vsmbShare, err := uvm.AddVSMB(ctx, reqHostPath, options)
if err != nil {
return err
}
defer func() {
if err != nil {
_ = vsmbShare.Release(ctx)
}
}()

sharePath, err := uvm.GetVSMBUvmPath(ctx, reqHostPath, readOnly)
_, sharePath, err := uvm.AddVsmbAndGetSharePath(ctx, reqHostPath, reqUVMPath, readOnly)
if err != nil {
return err
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/ctrdtaskapi/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

func init() {
typeurl.Register(&PolicyFragment{}, "github.com/Microsoft/hcsshim/pkg/ctrdtaskapi", "PolicyFragment")
typeurl.Register(&ContainerMount{}, "github.com/Microsoft/hcsshim/pkg/ctrdtaskapi", "ContainerMount")
}

type PolicyFragment struct {
Expand All @@ -15,3 +16,10 @@ type PolicyFragment struct {
// fragment and any additional information required for validation.
Fragment string `json:"fragment,omitempty"`
}

type ContainerMount struct {
HostPath string
ContainerPath string
ReadOnly bool
Type string
}