From f7a31660f9da5d189bb43b2b6bb2c04ff7fd179a Mon Sep 17 00:00:00 2001 From: Kirtana Ashok Date: Thu, 7 Sep 2023 17:35:07 -0700 Subject: [PATCH] Support adding mount to running containers - Extend hcsTask.Update() to process and add mount for running process isolated and hyperV wcow containers Signed-off-by: Kirtana Ashok (cherry picked from commit aad7467e620da7c4958945bb68e2f10bb0ea26b2) Signed-off-by: Kirtana Ashok --- .../service_internal_taskshim_test.go | 65 +++++++++++ cmd/containerd-shim-runhcs-v1/task.go | 1 + cmd/containerd-shim-runhcs-v1/task_hcs.go | 106 +++++++++++++++++- cmd/containerd-shim-runhcs-v1/task_test.go | 20 +++- internal/guestpath/paths.go | 2 + internal/uvm/share.go | 34 ++++-- pkg/ctrdtaskapi/update.go | 8 ++ 7 files changed, 217 insertions(+), 19 deletions(-) diff --git a/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go b/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go index a6954fb8d5..a1e39ebdab 100644 --- a/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go +++ b/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go @@ -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" @@ -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) diff --git a/cmd/containerd-shim-runhcs-v1/task.go b/cmd/containerd-shim-runhcs-v1/task.go index a54dfe421c..4798a1356d 100644 --- a/cmd/containerd-shim-runhcs-v1/task.go +++ b/cmd/containerd-shim-runhcs-v1/task.go @@ -110,6 +110,7 @@ func verifyTaskUpdateResourcesType(data interface{}) error { case *specs.WindowsResources: case *specs.LinuxResources: case *ctrdtaskapi.PolicyFragment: + case *ctrdtaskapi.ContainerMount: default: return errNotSupportedResourcesRequest } diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go index 31d4cc7419..6cb828abfd 100644 --- a/cmd/containerd-shim-runhcs-v1/task_hcs.go +++ b/cmd/containerd-shim-runhcs-v1/task_hcs.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" "time" @@ -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" @@ -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) { @@ -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) @@ -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) @@ -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 +} diff --git a/cmd/containerd-shim-runhcs-v1/task_test.go b/cmd/containerd-shim-runhcs-v1/task_test.go index 921d9557ff..756eafd250 100644 --- a/cmd/containerd-shim-runhcs-v1/task_test.go +++ b/cmd/containerd-shim-runhcs-v1/task_test.go @@ -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" @@ -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 { diff --git a/internal/guestpath/paths.go b/internal/guestpath/paths.go index fd1479da8c..e74de0a551 100644 --- a/internal/guestpath/paths.go +++ b/internal/guestpath/paths.go @@ -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" diff --git a/internal/uvm/share.go b/internal/uvm/share.go index 41ecda3a3a..b36903e14a 100644 --- a/internal/uvm/share.go +++ b/internal/uvm/share.go @@ -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 } diff --git a/pkg/ctrdtaskapi/update.go b/pkg/ctrdtaskapi/update.go index dd68cb0e40..bc6dd91f21 100644 --- a/pkg/ctrdtaskapi/update.go +++ b/pkg/ctrdtaskapi/update.go @@ -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 { @@ -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 +}