From dffc7ef169deb77e36437130f64a9da37515651b Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Fri, 4 Dec 2020 16:02:39 -0800 Subject: [PATCH] support pod and container updates Signed-off-by: Kathryn Baldauf --- .../service_internal.go | 12 +- .../service_internal_podshim_test.go | 44 +- .../service_internal_taskshim_test.go | 45 ++- cmd/containerd-shim-runhcs-v1/task.go | 17 +- cmd/containerd-shim-runhcs-v1/task_hcs.go | 133 +++++- cmd/containerd-shim-runhcs-v1/task_test.go | 9 + .../task_wcow_podsandbox.go | 18 + internal/gcs/container.go | 4 +- internal/gcs/guestconnection.go | 19 - internal/gcs/protocol.go | 5 - internal/guestrequest/types.go | 23 +- internal/hcs/resourcepaths/silo.go | 1 + internal/hcsoci/hcsdoc_wcow.go | 9 +- internal/hcsoci/resources.go | 35 ++ internal/jobcontainers/oci.go | 39 +- internal/jobobject/jobobject.go | 8 +- internal/jobobject/limits.go | 4 +- internal/uvm/update.go | 12 - internal/uvm/update_uvm.go | 56 +++ test/cri-containerd/container_update_test.go | 379 ++++++++++++++++++ test/cri-containerd/exec.go | 2 +- test/cri-containerd/main.go | 8 +- test/cri-containerd/pod_update_test.go | 234 +++++++++++ test/cri-containerd/share.go | 22 + test/cri-containerd/update_utilities.go | 95 +++++ test/functional/wcow_test.go | 2 +- test/internal/schemaversion_test.go | 2 +- .../hcsshim/internal/gcs/container.go | 4 +- .../hcsshim/internal/gcs/guestconnection.go | 19 - .../hcsshim/internal/gcs/protocol.go | 5 - .../hcsshim/internal/guestrequest/types.go | 23 +- .../internal/hcs/resourcepaths/silo.go | 1 + .../hcsshim/internal/hcsoci/hcsdoc_wcow.go | 9 +- .../hcsshim/internal/hcsoci/resources.go | 35 ++ .../Microsoft/hcsshim/internal/uvm/update.go | 12 - .../hcsshim/internal/uvm/update_uvm.go | 56 +++ 36 files changed, 1231 insertions(+), 170 deletions(-) create mode 100644 internal/hcsoci/resources.go delete mode 100644 internal/uvm/update.go create mode 100644 internal/uvm/update_uvm.go create mode 100644 test/cri-containerd/container_update_test.go create mode 100644 test/cri-containerd/pod_update_test.go create mode 100644 test/cri-containerd/share.go create mode 100644 test/cri-containerd/update_utilities.go create mode 100644 test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go delete mode 100644 test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update.go create mode 100644 test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update_uvm.go diff --git a/cmd/containerd-shim-runhcs-v1/service_internal.go b/cmd/containerd-shim-runhcs-v1/service_internal.go index 85e41c550b..eb85f69722 100644 --- a/cmd/containerd-shim-runhcs-v1/service_internal.go +++ b/cmd/containerd-shim-runhcs-v1/service_internal.go @@ -380,7 +380,17 @@ func (s *service) closeIOInternal(ctx context.Context, req *task.CloseIORequest) } func (s *service) updateInternal(ctx context.Context, req *task.UpdateTaskRequest) (*google_protobuf1.Empty, error) { - return nil, errdefs.ErrNotImplemented + if req.Resources == nil { + return nil, errors.Wrapf(errdefs.ErrInvalidArgument, "resources cannot be empty, updating container %s resources failed", req.ID) + } + t, err := s.getTask(req.ID) + if err != nil { + return nil, err + } + if err := t.Update(ctx, req); err != nil { + return nil, err + } + return empty, nil } func (s *service) waitInternal(ctx context.Context, req *task.WaitRequest) (*task.WaitResponse, error) { diff --git a/cmd/containerd-shim-runhcs-v1/service_internal_podshim_test.go b/cmd/containerd-shim-runhcs-v1/service_internal_podshim_test.go index 8bbbb408ae..dc316e6ea3 100644 --- a/cmd/containerd-shim-runhcs-v1/service_internal_podshim_test.go +++ b/cmd/containerd-shim-runhcs-v1/service_internal_podshim_test.go @@ -11,6 +11,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/runtime/v2/task" "github.com/containerd/typeurl" + specs "github.com/opencontainers/runtime-spec/specs-go" ) func setupPodServiceWithFakes(t *testing.T) (*service, *testShimTask, *testShimTask, *testShimExec) { @@ -573,15 +574,46 @@ func Test_PodShim_closeIOInternal_2ndTaskID_2ndExecID_Success(t *testing.T) { } } -func Test_PodShim_updateInternal_Error(t *testing.T) { - s := service{ - tid: t.Name(), - isSandbox: true, +func Test_PodShim_updateInternal_Success(t *testing.T) { + s, t1, _, _ := setupPodServiceWithFakes(t) + + var limit uint64 = 100 + resources := &specs.WindowsResources{ + Memory: &specs.WindowsMemoryResources{ + Limit: &limit, + }, } - resp, err := s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t.Name()}) + any, err := typeurl.MarshalAny(resources) + if err != nil { + t.Fatal(err) + } - verifyExpectedError(t, resp, err, errdefs.ErrNotImplemented) + resp, err := s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t1.ID(), Resources: any}) + if err != nil { + t.Fatalf("should not have failed with error, got: %v", err) + } + if resp == nil { + t.Fatalf("should have returned an empty resp") + } +} + +func Test_PodShim_updateInternal_Error(t *testing.T) { + s, t1, _, _ := setupPodServiceWithFakes(t) + + // resources must be of type *WindowsResources or *LinuxResources + resources := &specs.Process{} + any, err := typeurl.MarshalAny(resources) + if err != nil { + t.Fatal(err) + } + _, err = s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t1.ID(), Resources: any}) + if err == nil { + t.Fatal("expected to get an error for incorrect resource's type") + } + if err != errNotSupportedResourcesRequest { + t.Fatalf("expected to get errNotSupportedResourcesRequest, instead got %v", err) + } } func Test_PodShim_waitInternal_NoTask_Error(t *testing.T) { 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 5b89290e4a..1ee8eb7157 100644 --- a/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go +++ b/cmd/containerd-shim-runhcs-v1/service_internal_taskshim_test.go @@ -11,6 +11,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/runtime/v2/task" "github.com/containerd/typeurl" + "github.com/opencontainers/runtime-spec/specs-go" ) func setupTaskServiceWithFakes(t *testing.T) (*service, *testShimTask, *testShimExec) { @@ -493,15 +494,47 @@ func Test_TaskShim_closeIOInternal_InitTaskID_2ndExecID_Success(t *testing.T) { } } -func Test_TaskShim_updateInternal_Error(t *testing.T) { - s := service{ - tid: t.Name(), - isSandbox: true, +func Test_TaskShim_updateInternal_Success(t *testing.T) { + s, t1, _ := setupTaskServiceWithFakes(t) + + var limit uint64 = 100 + resources := &specs.WindowsResources{ + Memory: &specs.WindowsMemoryResources{ + Limit: &limit, + }, } - resp, err := s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t.Name()}) + any, err := typeurl.MarshalAny(resources) + if err != nil { + t.Fatal(err) + } - verifyExpectedError(t, resp, err, errdefs.ErrNotImplemented) + resp, err := s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t1.ID(), Resources: any}) + if err != nil { + t.Fatalf("should not have failed with error, got: %v", err) + } + if resp == nil { + t.Fatalf("should have returned an empty resp") + } +} + +func Test_TaskShim_updateInternal_Error(t *testing.T) { + s, t1, _ := setupTaskServiceWithFakes(t) + + // resources must be of type *WindowsResources or *LinuxResources + resources := &specs.Process{} + any, err := typeurl.MarshalAny(resources) + if err != nil { + t.Fatal(err) + } + + _, err = s.updateInternal(context.TODO(), &task.UpdateTaskRequest{ID: t1.ID(), Resources: any}) + if err == nil { + t.Fatal("expected to get an error for incorrect resource's type") + } + if err != errNotSupportedResourcesRequest { + t.Fatalf("expected to get errNotSupportedResourcesRequest, instead got %v", err) + } } func Test_TaskShim_waitInternal_NoTask_Error(t *testing.T) { diff --git a/cmd/containerd-shim-runhcs-v1/task.go b/cmd/containerd-shim-runhcs-v1/task.go index 2b3b835adc..34be8d668e 100644 --- a/cmd/containerd-shim-runhcs-v1/task.go +++ b/cmd/containerd-shim-runhcs-v1/task.go @@ -15,7 +15,10 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" ) -var errTaskNotIsolated = errors.New("task is not isolated") +var ( + errTaskNotIsolated = errors.New("task is not isolated") + errNotSupportedResourcesRequest = errors.New("update resources must be of type *WindowsResources or *LinuxResources") +) type shimTask interface { // ID returns the original id used at `Create`. @@ -86,6 +89,18 @@ type shimTask interface { // If the host is hypervisor isolated and this task owns the host additional // metrics on the UVM may be returned as well. Stats(ctx context.Context) (*stats.Statistics, error) + // Update updates a task's container + Update(ctx context.Context, req *task.UpdateTaskRequest) error +} + +func verifyTaskUpdateResourcesType(data interface{}) error { + switch data.(type) { + case *specs.WindowsResources: + case *specs.LinuxResources: + default: + return errNotSupportedResourcesRequest + } + return nil } // isStatsNotFound returns true if the err corresponds to a scenario diff --git a/cmd/containerd-shim-runhcs-v1/task_hcs.go b/cmd/containerd-shim-runhcs-v1/task_hcs.go index 4d0fa50b1c..eaef67b2ab 100644 --- a/cmd/containerd-shim-runhcs-v1/task_hcs.go +++ b/cmd/containerd-shim-runhcs-v1/task_hcs.go @@ -23,18 +23,24 @@ import ( "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/guestrequest" "github.com/Microsoft/hcsshim/internal/hcs" + "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" "github.com/Microsoft/hcsshim/internal/hcs/schema1" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/hcsoci" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oci" + "github.com/Microsoft/hcsshim/internal/processorinfo" + "github.com/Microsoft/hcsshim/internal/requesttype" "github.com/Microsoft/hcsshim/internal/resources" "github.com/Microsoft/hcsshim/internal/shimdiag" "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/osversion" ) +const bytesPerMB = 1024 * 1024 + func newHcsStandaloneTask(ctx context.Context, events publisher, req *task.CreateTaskRequest, s *specs.Spec) (shimTask, error) { log.G(ctx).WithField("tid", req.ID).Debug("newHcsStandaloneTask") @@ -784,25 +790,7 @@ func (ht *hcsTask) Share(ctx context.Context, req *shimdiag.ShareRequest) error if ht.host == nil { return errTaskNotIsolated } - // For hyper-v isolated WCOW the task used isn't the standard hcsTask so we - // only have to deal with the LCOW case here. - st, err := os.Stat(req.HostPath) - if err != nil { - return fmt.Errorf("could not open '%s' path on host: %s", req.HostPath, err) - } - var ( - hostPath string = req.HostPath - restrictAccess bool - fileName string - allowedNames []string - ) - if !st.IsDir() { - hostPath, fileName = filepath.Split(hostPath) - allowedNames = append(allowedNames, fileName) - restrictAccess = true - } - _, err = ht.host.AddPlan9(ctx, hostPath, req.UvmPath, req.ReadOnly, restrictAccess, allowedNames) - return err + return ht.host.Share(ctx, req.HostPath, req.UvmPath, req.ReadOnly) } func hcsPropertiesToWindowsStats(props *hcsschema.Properties) *stats.Statistics_Windows { @@ -859,3 +847,110 @@ func (ht *hcsTask) Stats(ctx context.Context) (*stats.Statistics, error) { } return s, nil } + +func (ht *hcsTask) Update(ctx context.Context, req *task.UpdateTaskRequest) error { + resources, err := typeurl.UnmarshalAny(req.Resources) + if err != nil { + return errors.Wrapf(err, "failed to unmarshal resources for container %s update request", req.ID) + } + + if err := verifyTaskUpdateResourcesType(resources); err != nil { + return err + } + + if ht.ownsHost && ht.host != nil { + return ht.host.UpdateConstraints(ctx, resources, req.Annotations) + } + + return ht.updateTaskContainerResources(ctx, resources, req.Annotations) +} + +func (ht *hcsTask) updateTaskContainerResources(ctx context.Context, data interface{}, annotations map[string]string) error { + if ht.isWCOW { + return ht.updateWCOWResources(ctx, data, annotations) + } + + return ht.updateLCOWResources(ctx, data, annotations) +} + +func (ht *hcsTask) updateWCOWContainerCPU(ctx context.Context, cpu *specs.WindowsCPUResources) error { + // if host is 20h2+ then we can make a request directly to hcs + if osversion.Get().Build >= osversion.V20H2 { + req := &hcsschema.Processor{} + if cpu.Count != nil { + procCount := int32(*cpu.Count) + hostProcs := processorinfo.ProcessorCount() + if ht.host != nil { + hostProcs = ht.host.ProcessorCount() + } + req.Count = hcsoci.NormalizeProcessorCount(ctx, ht.id, procCount, hostProcs) + } + if cpu.Maximum != nil { + req.Maximum = int32(*cpu.Maximum) + } + if cpu.Shares != nil { + req.Weight = int32(*cpu.Shares) + } + return ht.requestUpdateContainer(ctx, resourcepaths.SiloProcessorResourcePath, req) + } + + return errdefs.ErrNotImplemented +} + +func isValidWindowsCPUResources(c *specs.WindowsCPUResources) bool { + return (c.Count != nil && (c.Shares == nil && c.Maximum == nil)) || + (c.Shares != nil && (c.Count == nil && c.Maximum == nil)) || + (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") + } + if resources.Memory != nil && resources.Memory.Limit != nil { + newMemorySizeInMB := *resources.Memory.Limit / bytesPerMB + memoryLimit := hcsoci.NormalizeMemorySize(ctx, ht.id, newMemorySizeInMB) + if err := ht.requestUpdateContainer(ctx, resourcepaths.SiloMemoryResourcePath, memoryLimit); err != nil { + return err + } + } + if resources.CPU != nil { + if !isValidWindowsCPUResources(resources.CPU) { + return fmt.Errorf("invalid cpu resources request for container %s: %v", ht.id, resources.CPU) + } + if err := ht.updateWCOWContainerCPU(ctx, resources.CPU); err != nil { + return err + } + } + return nil +} + +func (ht *hcsTask) updateLCOWResources(ctx context.Context, data interface{}, annotations map[string]string) error { + resources, ok := data.(*specs.LinuxResources) + if !ok || resources == nil { + return errors.New("must have resources be non-nil and type *LinuxResources when updating a lcow container") + } + settings := guestrequest.LCOWContainerConstraints{ + Linux: *resources, + } + return ht.requestUpdateContainer(ctx, "", settings) +} + +func (ht *hcsTask) requestUpdateContainer(ctx context.Context, resourcePath string, settings interface{}) error { + var modification interface{} + if ht.isWCOW { + modification = &hcsschema.ModifySettingRequest{ + ResourcePath: resourcePath, + RequestType: requesttype.Update, + Settings: settings, + } + } else { + modification = guestrequest.GuestRequest{ + ResourceType: guestrequest.ResourceTypeContainerConstraints, + RequestType: requesttype.Update, + Settings: settings, + } + } + return ht.c.Modify(ctx, modification) +} diff --git a/cmd/containerd-shim-runhcs-v1/task_test.go b/cmd/containerd-shim-runhcs-v1/task_test.go index 1e6268f7d5..18ed758639 100644 --- a/cmd/containerd-shim-runhcs-v1/task_test.go +++ b/cmd/containerd-shim-runhcs-v1/task_test.go @@ -10,6 +10,7 @@ import ( v1 "github.com/containerd/cgroups/stats/v1" "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/runtime/v2/task" + "github.com/containerd/typeurl" specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" ) @@ -91,6 +92,14 @@ func (tst *testShimTask) DumpGuestStacks(ctx context.Context) string { return "" } +func (tst *testShimTask) Update(ctx context.Context, req *task.UpdateTaskRequest) error { + data, err := typeurl.UnmarshalAny(req.Resources) + if err != nil { + return errors.Wrapf(err, "failed to unmarshal resources for container %s update request", req.ID) + } + return verifyTaskUpdateResourcesType(data) +} + func (tst *testShimTask) Share(ctx context.Context, req *shimdiag.ShareRequest) error { return errors.New("not implemented") } diff --git a/cmd/containerd-shim-runhcs-v1/task_wcow_podsandbox.go b/cmd/containerd-shim-runhcs-v1/task_wcow_podsandbox.go index 93422c75af..0877b51ae2 100644 --- a/cmd/containerd-shim-runhcs-v1/task_wcow_podsandbox.go +++ b/cmd/containerd-shim-runhcs-v1/task_wcow_podsandbox.go @@ -16,6 +16,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/runtime" "github.com/containerd/containerd/runtime/v2/task" + "github.com/containerd/typeurl" "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "go.opencensus.io/trace" @@ -257,6 +258,23 @@ func (wpst *wcowPodSandboxTask) DumpGuestStacks(ctx context.Context) string { return "" } +func (wpst *wcowPodSandboxTask) Update(ctx context.Context, req *task.UpdateTaskRequest) error { + if wpst.host == nil { + return errTaskNotIsolated + } + + resources, err := typeurl.UnmarshalAny(req.Resources) + if err != nil { + return errors.Wrapf(err, "failed to unmarshal resources for container %s update request", req.ID) + } + + if err := verifyTaskUpdateResourcesType(resources); err != nil { + return err + } + + return wpst.host.UpdateConstraints(ctx, resources, req.Annotations) +} + func (wpst *wcowPodSandboxTask) Share(ctx context.Context, req *shimdiag.ShareRequest) error { if wpst.host == nil { return errTaskNotIsolated diff --git a/internal/gcs/container.go b/internal/gcs/container.go index de4758435c..6ec5b8b84f 100644 --- a/internal/gcs/container.go +++ b/internal/gcs/container.go @@ -14,9 +14,7 @@ import ( "go.opencensus.io/trace" ) -const ( - hrComputeSystemDoesNotExist = 0xc037010e -) +const hrComputeSystemDoesNotExist = 0xc037010e // Container implements the cow.Container interface for containers // created via GuestConnection. diff --git a/internal/gcs/guestconnection.go b/internal/gcs/guestconnection.go index 02c08e4eb9..f01b97939b 100644 --- a/internal/gcs/guestconnection.go +++ b/internal/gcs/guestconnection.go @@ -193,25 +193,6 @@ func (gc *GuestConnection) DeleteContainerState(ctx context.Context, cid string) return gc.brdg.RPC(ctx, rpcDeleteContainerState, &req, &resp, false) } -func (gc *GuestConnection) UpdateContainer(ctx context.Context, cid string, resources interface{}) (err error) { - ctx, span := trace.StartSpan(ctx, "gcs::GuestConnection::UpdateContainer") - defer span.End() - defer func() { oc.SetSpanStatus(span, err) }() - span.AddAttributes(trace.StringAttribute("cid", cid)) - - resourcesJSON, err := json.Marshal(resources) - if err != nil { - return err - } - - req := updateContainerRequest{ - requestBase: makeRequest(ctx, cid), - Resources: string(resourcesJSON), - } - var resp responseBase - return gc.brdg.RPC(ctx, rpcUpdateContainer, &req, &resp, false) -} - // Close terminates the guest connection. It is undefined to call any other // methods on the connection after this is called. func (gc *GuestConnection) Close() error { diff --git a/internal/gcs/protocol.go b/internal/gcs/protocol.go index 5a32a2c1ce..d559a86364 100644 --- a/internal/gcs/protocol.go +++ b/internal/gcs/protocol.go @@ -361,8 +361,3 @@ type containerGetPropertiesResponseV2 struct { responseBase Properties containerPropertiesV2 } - -type updateContainerRequest struct { - requestBase - Resources string -} diff --git a/internal/guestrequest/types.go b/internal/guestrequest/types.go index 1b32adb42a..bfe83eab44 100644 --- a/internal/guestrequest/types.go +++ b/internal/guestrequest/types.go @@ -2,6 +2,7 @@ package guestrequest import ( hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/opencontainers/runtime-spec/specs-go" ) // Arguably, many of these (at least CombinedLayers) should have been generated @@ -67,18 +68,24 @@ type LCOWNetworkAdapter struct { EncapOverhead uint16 `json:",omitempty"` } +type LCOWContainerConstraints struct { + Windows specs.WindowsResources `json:",omitempty"` + Linux specs.LinuxResources `json:",omitempty"` +} + type ResourceType string const ( // These are constants for v2 schema modify guest requests. - ResourceTypeMappedDirectory ResourceType = "MappedDirectory" - ResourceTypeMappedVirtualDisk ResourceType = "MappedVirtualDisk" - ResourceTypeNetwork ResourceType = "Network" - ResourceTypeNetworkNamespace ResourceType = "NetworkNamespace" - ResourceTypeCombinedLayers ResourceType = "CombinedLayers" - ResourceTypeVPMemDevice ResourceType = "VPMemDevice" - ResourceTypeVPCIDevice ResourceType = "VPCIDevice" - ResourceTypeHvSocket ResourceType = "HvSocket" + ResourceTypeMappedDirectory ResourceType = "MappedDirectory" + ResourceTypeMappedVirtualDisk ResourceType = "MappedVirtualDisk" + ResourceTypeNetwork ResourceType = "Network" + ResourceTypeNetworkNamespace ResourceType = "NetworkNamespace" + ResourceTypeCombinedLayers ResourceType = "CombinedLayers" + ResourceTypeVPMemDevice ResourceType = "VPMemDevice" + ResourceTypeVPCIDevice ResourceType = "VPCIDevice" + ResourceTypeContainerConstraints ResourceType = "ContainerConstraints" + ResourceTypeHvSocket ResourceType = "HvSocket" ) // GuestRequest is for modify commands passed to the guest. diff --git a/internal/hcs/resourcepaths/silo.go b/internal/hcs/resourcepaths/silo.go index f426d32a1e..531e7c0b5d 100644 --- a/internal/hcs/resourcepaths/silo.go +++ b/internal/hcs/resourcepaths/silo.go @@ -8,4 +8,5 @@ const ( SiloMappedPipeResourcePath string = "Container/MappedPipes" SiloMemoryResourcePath string = "Container/Memory/SizeInMB" SiloRegistryFlushStatePath string = "Container/RegistryFlushState" + SiloProcessorResourcePath string = "Container/Processor" ) diff --git a/internal/hcsoci/hcsdoc_wcow.go b/internal/hcsoci/hcsdoc_wcow.go index 12ddf1aade..7423668dda 100644 --- a/internal/hcsoci/hcsdoc_wcow.go +++ b/internal/hcsoci/hcsdoc_wcow.go @@ -119,14 +119,7 @@ func ConvertCPULimits(ctx context.Context, cid string, spec *specs.Spec, maxCPUC if cpuNumSet > 1 { return 0, 0, 0, fmt.Errorf("invalid spec - Windows Container CPU Count: '%d', Limit: '%d', and Weight: '%d' are mutually exclusive", cpuCount, cpuLimit, cpuWeight) } else if cpuNumSet == 1 { - if cpuCount > maxCPUCount { - log.G(ctx).WithFields(logrus.Fields{ - "cid": cid, - "requested": cpuCount, - "assigned": maxCPUCount, - }).Warn("Changing user requested CPUCount to current number of processors") - cpuCount = maxCPUCount - } + cpuCount = NormalizeProcessorCount(ctx, cid, cpuCount, maxCPUCount) } return cpuCount, cpuLimit, cpuWeight, nil } diff --git a/internal/hcsoci/resources.go b/internal/hcsoci/resources.go new file mode 100644 index 0000000000..46ad55660f --- /dev/null +++ b/internal/hcsoci/resources.go @@ -0,0 +1,35 @@ +package hcsoci + +import ( + "context" + + "github.com/Microsoft/hcsshim/internal/log" + "github.com/sirupsen/logrus" +) + +// NormalizeProcessorCount returns the `Min(requested, logical CPU count)`. +func NormalizeProcessorCount(ctx context.Context, cid string, requestedCount, hostCount int32) int32 { + if requestedCount > hostCount { + log.G(ctx).WithFields(logrus.Fields{ + "id": cid, + "requested count": requestedCount, + "assigned count": hostCount, + }).Warn("Changing user requested cpu count to current number of processors on the host") + return hostCount + } else { + return requestedCount + } +} + +// NormalizeMemorySize returns the requested memory size in MB aligned up to an even number +func NormalizeMemorySize(ctx context.Context, cid string, requestedSizeMB uint64) uint64 { + actualMB := (requestedSizeMB + 1) &^ 1 // align up to an even number + if requestedSizeMB != actualMB { + log.G(ctx).WithFields(logrus.Fields{ + "id": cid, + "requestedMB": requestedSizeMB, + "actualMB": actualMB, + }).Warn("Changing user requested MemorySizeInMB to align to 2MB") + } + return actualMB +} diff --git a/internal/jobcontainers/oci.go b/internal/jobcontainers/oci.go index d78adfe43a..95dfe68a40 100644 --- a/internal/jobcontainers/oci.go +++ b/internal/jobcontainers/oci.go @@ -12,23 +12,10 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" ) +const processorWeightMax = 10000 + // This file contains helpers for converting parts of the oci spec to useful // structures/limits to be applied to a job object. -func calculateJobCPUWeight(processorWeight uint32) uint32 { - if processorWeight == 0 { - return 0 - } - return 1 + uint32((8*processorWeight)/jobobject.CPUWeightMax) -} - -func calculateJobCPURate(hostProcs uint32, processorCount uint32) uint32 { - rate := (processorCount * 10000) / hostProcs - if rate == 0 { - return 1 - } - return rate -} - func getUserTokenInheritAnnotation(annotations map[string]string) bool { val, ok := annotations[oci.AnnotationHostProcessInheritUser] return ok && val == "true" @@ -70,3 +57,25 @@ func specToLimits(ctx context.Context, cid string, s *specs.Spec) (*jobobject.Jo MemoryLimitInBytes: memLimitMB * 1024 * 1024, }, nil } + +// calculateJobCPUWeight converts processor cpu weight to job object cpu weight. +// +// `processorWeight` is the processor cpu weight to convert. +func calculateJobCPUWeight(processorWeight uint32) uint32 { + if processorWeight == 0 { + return 0 + } + return 1 + uint32((8*processorWeight)/processorWeightMax) +} + +// calculateJobCPURate converts processor cpu count to job object cpu rate. +// +// `hostProcs` is the total host's processor count. +// `processorCount` is the processor count to convert to cpu rate. +func calculateJobCPURate(hostProcs uint32, processorCount uint32) uint32 { + rate := (processorCount * 10000) / hostProcs + if rate == 0 { + return 1 + } + return rate +} diff --git a/internal/jobobject/jobobject.go b/internal/jobobject/jobobject.go index fe8992bc71..28edf1d6c4 100644 --- a/internal/jobobject/jobobject.go +++ b/internal/jobobject/jobobject.go @@ -46,10 +46,10 @@ const ( // Processor resource controls const ( - CPULimitMin = 1 - CPULimitMax = 10000 - CPUWeightMin = 1 - CPUWeightMax = 9 + cpuLimitMin = 1 + cpuLimitMax = 10000 + cpuWeightMin = 1 + cpuWeightMax = 9 ) var ( diff --git a/internal/jobobject/limits.go b/internal/jobobject/limits.go index 1690f603fd..5d4c90d241 100644 --- a/internal/jobobject/limits.go +++ b/internal/jobobject/limits.go @@ -89,13 +89,13 @@ func (job *JobObject) SetCPULimit(rateControlType CPURateControlType, rateContro } switch rateControlType { case WeightBased: - if rateControlValue < CPUWeightMin || rateControlValue > CPUWeightMax { + if rateControlValue < cpuWeightMin || rateControlValue > cpuWeightMax { return fmt.Errorf("processor weight value of `%d` is invalid", rateControlValue) } cpuInfo.ControlFlags |= winapi.JOB_OBJECT_CPU_RATE_CONTROL_ENABLE | winapi.JOB_OBJECT_CPU_RATE_CONTROL_WEIGHT_BASED cpuInfo.Value = rateControlValue case RateBased: - if rateControlValue < CPULimitMin || rateControlValue > CPULimitMax { + if rateControlValue < cpuLimitMin || rateControlValue > cpuLimitMax { return fmt.Errorf("processor rate of `%d` is invalid", rateControlValue) } cpuInfo.ControlFlags |= winapi.JOB_OBJECT_CPU_RATE_CONTROL_ENABLE | winapi.JOB_OBJECT_CPU_RATE_CONTROL_HARD_CAP diff --git a/internal/uvm/update.go b/internal/uvm/update.go deleted file mode 100644 index ee576767df..0000000000 --- a/internal/uvm/update.go +++ /dev/null @@ -1,12 +0,0 @@ -package uvm - -import ( - "context" -) - -func (uvm *UtilityVM) UpdateContainer(ctx context.Context, cid string, resources interface{}) error { - if uvm.gc == nil || !uvm.guestCaps.UpdateContainerSupported { - return nil - } - return uvm.gc.UpdateContainer(ctx, cid, resources) -} diff --git a/internal/uvm/update_uvm.go b/internal/uvm/update_uvm.go new file mode 100644 index 0000000000..635c81ae81 --- /dev/null +++ b/internal/uvm/update_uvm.go @@ -0,0 +1,56 @@ +package uvm + +import ( + "context" + + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + specs "github.com/opencontainers/runtime-spec/specs-go" +) + +func (uvm *UtilityVM) UpdateConstraints(ctx context.Context, data interface{}, annotations map[string]string) error { + var memoryLimitInBytes *uint64 + var processorLimits *hcsschema.ProcessorLimits + + switch resources := data.(type) { + case *specs.WindowsResources: + if resources.Memory != nil { + memoryLimitInBytes = resources.Memory.Limit + } + if resources.CPU != nil { + processorLimits := &hcsschema.ProcessorLimits{} + if resources.CPU.Maximum != nil { + processorLimits.Limit = uint64(*resources.CPU.Maximum) + } + if resources.CPU.Shares != nil { + processorLimits.Weight = uint64(*resources.CPU.Shares) + } + } + case *specs.LinuxResources: + if resources.Memory != nil { + mem := uint64(*resources.Memory.Limit) + memoryLimitInBytes = &mem + } + if resources.CPU != nil { + processorLimits := &hcsschema.ProcessorLimits{} + if resources.CPU.Quota != nil { + processorLimits.Limit = uint64(*resources.CPU.Quota) + } + if resources.CPU.Shares != nil { + processorLimits.Weight = uint64(*resources.CPU.Shares) + } + } + } + + if memoryLimitInBytes != nil { + if err := uvm.UpdateMemory(ctx, *memoryLimitInBytes); err != nil { + return err + } + } + if processorLimits != nil { + if err := uvm.UpdateCPULimits(ctx, processorLimits); err != nil { + return err + } + } + + return nil +} diff --git a/test/cri-containerd/container_update_test.go b/test/cri-containerd/container_update_test.go new file mode 100644 index 0000000000..94ecbfd1c8 --- /dev/null +++ b/test/cri-containerd/container_update_test.go @@ -0,0 +1,379 @@ +// +build functional + +package cri_containerd + +import ( + "context" + "fmt" + "testing" + + "github.com/Microsoft/hcsshim/osversion" + testutilities "github.com/Microsoft/hcsshim/test/functional/utilities" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" +) + +const processorWeightMax = 10000 + +func calculateJobCPUWeight(processorWeight uint32) uint32 { + if processorWeight == 0 { + return 0 + } + return 1 + uint32((8*processorWeight)/processorWeightMax) +} + +func calculateJobCPURate(hostProcs uint32, processorCount uint32) uint32 { + rate := (processorCount * 10000) / hostProcs + if rate == 0 { + return 1 + } + return rate +} + +func Test_Container_UpdateResources_CPUShare(t *testing.T) { + testutilities.RequiresBuild(t, osversion.V20H2) + type config struct { + name string + requiredFeatures []string + runtimeHandler string + sandboxImage string + containerImage string + cmd []string + } + tests := []config{ + { + name: "WCOW_Process", + requiredFeatures: []string{featureWCOWProcess}, + runtimeHandler: wcowProcessRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + containerImage: imageWindowsNanoserver, + cmd: []string{"cmd", "/c", "ping", "-t", "127.0.0.1"}, + }, + { + name: "WCOW_Hypervisor", + requiredFeatures: []string{featureWCOWHypervisor}, + runtimeHandler: wcowHypervisorRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + containerImage: imageWindowsNanoserver, + cmd: []string{"cmd", "/c", "ping", "-t", "127.0.0.1"}, + }, + { + name: "LCOW", + requiredFeatures: []string{featureLCOW}, + runtimeHandler: lcowRuntimeHandler, + sandboxImage: imageLcowK8sPause, + containerImage: imageLcowAlpine, + cmd: []string{"top"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + if test.runtimeHandler == lcowRuntimeHandler { + pullRequiredLcowImages(t, []string{test.sandboxImage}) + } else if test.runtimeHandler == wcowHypervisorRuntimeHandler { + pullRequiredImages(t, []string{test.sandboxImage}) + } + + podRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name(), + Namespace: testNamespace, + }, + }, + RuntimeHandler: test.runtimeHandler, + } + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podID := runPodSandbox(t, client, ctx, podRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + containerRequest := &runtime.CreateContainerRequest{ + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: t.Name() + "-Container", + }, + Image: &runtime.ImageSpec{ + Image: test.containerImage, + }, + Command: test.cmd, + }, + PodSandboxId: podID, + SandboxConfig: podRequest.Config, + } + + containerID := createContainer(t, client, ctx, containerRequest) + defer removeContainer(t, client, ctx, containerID) + + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + + // make request to increase cpu shares + updateReq := &runtime.UpdateContainerResourcesRequest{ + ContainerId: containerID, + } + + var expected uint32 = 5000 + if test.runtimeHandler == lcowRuntimeHandler { + updateReq.Linux = &runtime.LinuxContainerResources{ + CpuShares: int64(expected), + } + } else { + updateReq.Windows = &runtime.WindowsContainerResources{ + CpuShares: int64(expected), + } + } + + if _, err := client.UpdateContainerResources(ctx, updateReq); err != nil { + t.Fatalf("updating container resources for %s with %v", containerID, err) + } + + if test.runtimeHandler == lcowRuntimeHandler { + checkLCOWResourceLimit(t, ctx, client, containerID, "/sys/fs/cgroup/cpu/cpu.shares", uint64(expected)) + } else { + targetShimName := "k8s.io-" + podID + jobExpectedValue := calculateJobCPUWeight(expected) + checkWCOWResourceLimit(t, ctx, test.runtimeHandler, targetShimName, containerID, "cpu-weight", uint64(jobExpectedValue)) + } + }) + } +} + +func Test_Container_UpdateResources_CPUShare_NotRunning(t *testing.T) { + type config struct { + name string + requiredFeatures []string + runtimeHandler string + sandboxImage string + containerImage string + cmd []string + } + tests := []config{ + { + name: "WCOW_Process", + requiredFeatures: []string{featureWCOWProcess}, + runtimeHandler: wcowProcessRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + containerImage: imageWindowsNanoserver, + cmd: []string{"cmd", "/c", "ping", "-t", "127.0.0.1"}, + }, + { + name: "WCOW_Hypervisor", + requiredFeatures: []string{featureWCOWHypervisor}, + runtimeHandler: wcowHypervisorRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + containerImage: imageWindowsNanoserver, + cmd: []string{"cmd", "/c", "ping", "-t", "127.0.0.1"}, + }, + { + name: "LCOW", + requiredFeatures: []string{featureLCOW}, + runtimeHandler: lcowRuntimeHandler, + sandboxImage: imageLcowK8sPause, + containerImage: imageLcowAlpine, + cmd: []string{"top"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + if test.runtimeHandler == lcowRuntimeHandler { + pullRequiredLcowImages(t, []string{test.sandboxImage}) + } else if test.runtimeHandler == wcowHypervisorRuntimeHandler { + pullRequiredImages(t, []string{test.sandboxImage}) + } + + podRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name(), + Namespace: testNamespace, + }, + }, + RuntimeHandler: test.runtimeHandler, + } + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podID := runPodSandbox(t, client, ctx, podRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + containerRequest := &runtime.CreateContainerRequest{ + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: t.Name() + "-Container", + }, + Image: &runtime.ImageSpec{ + Image: test.containerImage, + }, + Command: test.cmd, + }, + PodSandboxId: podID, + SandboxConfig: podRequest.Config, + } + + containerID := createContainer(t, client, ctx, containerRequest) + defer removeContainer(t, client, ctx, containerID) + + // make request to increase cpu shares == cpu weight + updateReq := &runtime.UpdateContainerResourcesRequest{ + ContainerId: containerID, + } + + var expected uint32 = 5000 + if test.runtimeHandler == lcowRuntimeHandler { + updateReq.Linux = &runtime.LinuxContainerResources{ + CpuShares: int64(expected), + } + } else { + updateReq.Windows = &runtime.WindowsContainerResources{ + CpuShares: int64(expected), + } + } + + if _, err := client.UpdateContainerResources(ctx, updateReq); err != nil { + t.Fatalf("updating container resources for %s with %v", containerID, err) + } + + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + + if test.runtimeHandler == lcowRuntimeHandler { + checkLCOWResourceLimit(t, ctx, client, containerID, "/sys/fs/cgroup/cpu/cpu.shares", uint64(expected)) + } else { + targetShimName := "k8s.io-" + podID + jobExpectedValue := calculateJobCPUWeight(expected) + checkWCOWResourceLimit(t, ctx, test.runtimeHandler, targetShimName, containerID, "cpu-weight", uint64(jobExpectedValue)) + } + }) + } +} + +func Test_Container_UpdateResources_Memory(t *testing.T) { + type config struct { + name string + requiredFeatures []string + runtimeHandler string + sandboxImage string + containerImage string + cmd []string + } + tests := []config{ + { + name: "WCOW_Process", + requiredFeatures: []string{featureWCOWProcess}, + runtimeHandler: wcowProcessRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + containerImage: imageWindowsNanoserver, + cmd: []string{"cmd", "/c", "ping", "-t", "127.0.0.1"}, + }, + { + name: "WCOW_Hypervisor", + requiredFeatures: []string{featureWCOWHypervisor}, + runtimeHandler: wcowHypervisorRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + containerImage: imageWindowsNanoserver, + cmd: []string{"cmd", "/c", "ping", "-t", "127.0.0.1"}, + }, + { + name: "LCOW", + requiredFeatures: []string{featureLCOW}, + runtimeHandler: lcowRuntimeHandler, + sandboxImage: imageLcowK8sPause, + containerImage: imageLcowAlpine, + cmd: []string{"top"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + if test.runtimeHandler == lcowRuntimeHandler { + pullRequiredLcowImages(t, []string{test.sandboxImage}) + } else if test.runtimeHandler == wcowHypervisorRuntimeHandler { + pullRequiredImages(t, []string{test.sandboxImage}) + } + + podRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name(), + Namespace: testNamespace, + }, + }, + RuntimeHandler: test.runtimeHandler, + } + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podID := runPodSandbox(t, client, ctx, podRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + var startingMemorySize int64 = 768 * 1024 * 1024 + containerRequest := &runtime.CreateContainerRequest{ + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: t.Name() + "-Container", + }, + Image: &runtime.ImageSpec{ + Image: test.containerImage, + }, + Command: test.cmd, + Annotations: map[string]string{ + "io.microsoft.container.memory.sizeinmb": fmt.Sprintf("%d", startingMemorySize), // 768MB + }, + }, + PodSandboxId: podID, + SandboxConfig: podRequest.Config, + } + + containerID := createContainer(t, client, ctx, containerRequest) + defer removeContainer(t, client, ctx, containerID) + + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + + // make request for cpu shares + updateReq := &runtime.UpdateContainerResourcesRequest{ + ContainerId: containerID, + } + + newMemorySize := startingMemorySize / 2 + if test.runtimeHandler == lcowRuntimeHandler { + updateReq.Linux = &runtime.LinuxContainerResources{ + MemoryLimitInBytes: newMemorySize, + } + } else { + updateReq.Windows = &runtime.WindowsContainerResources{ + MemoryLimitInBytes: newMemorySize, + } + } + + if _, err := client.UpdateContainerResources(ctx, updateReq); err != nil { + t.Fatalf("updating container resources for %s with %v", containerID, err) + } + + if test.runtimeHandler == lcowRuntimeHandler { + checkLCOWResourceLimit(t, ctx, client, containerID, "/sys/fs/cgroup/memory/memory.limit_in_bytes", uint64(newMemorySize)) + } else { + targetShimName := "k8s.io-" + podID + checkWCOWResourceLimit(t, ctx, test.runtimeHandler, targetShimName, containerID, "memory-limit", uint64(newMemorySize)) + } + }) + } +} diff --git a/test/cri-containerd/exec.go b/test/cri-containerd/exec.go index 657177aa5a..16346aa38a 100644 --- a/test/cri-containerd/exec.go +++ b/test/cri-containerd/exec.go @@ -32,7 +32,7 @@ func execRequest(t *testing.T, client runtime.RuntimeServiceClient, ctx context. // `stdinBuf` is an optional parameter to specify an io.Reader that can be used as stdin for the executed program. // `stdoutBuf` is an optional parameter to specify an io.Writer that can be used as stdout for the executed program. // `stderrBuf` is an optional parameter to specify an io.Writer that can be used as stderr for the executed program. -func execInHost(t *testing.T, ctx context.Context, client shimdiag.ShimDiagService, args []string, stdinBuf *io.Reader, stdoutBuf, stderrBuf *io.Writer) (_ int32, err error) { +func execInHost(ctx context.Context, client shimdiag.ShimDiagService, args []string, stdinBuf io.Reader, stdoutBuf, stderrBuf io.Writer) (_ int32, err error) { var ( stdin = "" stdout = "" diff --git a/test/cri-containerd/main.go b/test/cri-containerd/main.go index ab281178a6..d8d92ba58d 100644 --- a/test/cri-containerd/main.go +++ b/test/cri-containerd/main.go @@ -36,9 +36,11 @@ const ( wcowHypervisor18362RuntimeHandler = "runhcs-wcow-hypervisor-18362" wcowHypervisor19041RuntimeHandler = "runhcs-wcow-hypervisor-19041" - testDeviceUtilFilePath = "C:\\ContainerPlat\\device-util.exe" - testDriversPath = "C:\\ContainerPlat\\testdrivers" - testGPUBootFiles = "C:\\ContainerPlat\\LinuxBootFiles\\nvidiagpu" + testDeviceUtilFilePath = "C:\\ContainerPlat\\device-util.exe" + testJobObjectUtilFilePath = "C:\\ContainerPlat\\jobobject-util.exe" + + testDriversPath = "C:\\ContainerPlat\\testdrivers" + testGPUBootFiles = "C:\\ContainerPlat\\LinuxBootFiles\\nvidiagpu" lcowRuntimeHandler = "runhcs-lcow" imageLcowK8sPause = "k8s.gcr.io/pause:3.1" diff --git a/test/cri-containerd/pod_update_test.go b/test/cri-containerd/pod_update_test.go new file mode 100644 index 0000000000..9bedd9f20c --- /dev/null +++ b/test/cri-containerd/pod_update_test.go @@ -0,0 +1,234 @@ +// +build functional + +package cri_containerd + +import ( + "context" + "fmt" + "testing" + + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" +) + +func Test_Pod_UpdateResources_Memory(t *testing.T) { + type config struct { + name string + requiredFeatures []string + runtimeHandler string + sandboxImage string + } + tests := []config{ + { + name: "WCOW_Hypervisor", + requiredFeatures: []string{featureWCOWHypervisor}, + runtimeHandler: wcowHypervisorRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + }, + { + name: "LCOW", + requiredFeatures: []string{featureLCOW}, + runtimeHandler: lcowRuntimeHandler, + sandboxImage: imageLcowK8sPause, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + if test.runtimeHandler == lcowRuntimeHandler { + pullRequiredLcowImages(t, []string{test.sandboxImage}) + } else { + pullRequiredImages(t, []string{test.sandboxImage}) + } + var startingMemorySize int64 = 768 * 1024 * 1024 + podRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name(), + Namespace: testNamespace, + }, + Annotations: map[string]string{ + "io.microsoft.container.memory.sizeinmb": fmt.Sprintf("%d", startingMemorySize), + }, + }, + RuntimeHandler: test.runtimeHandler, + } + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podID := runPodSandbox(t, client, ctx, podRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + // make request for shrinking memory size + newMemorySize := startingMemorySize / 2 + updateReq := &runtime.UpdateContainerResourcesRequest{ + ContainerId: podID, + } + + if test.runtimeHandler == lcowRuntimeHandler { + updateReq.Linux = &runtime.LinuxContainerResources{ + MemoryLimitInBytes: newMemorySize, + } + } else { + updateReq.Windows = &runtime.WindowsContainerResources{ + MemoryLimitInBytes: newMemorySize, + } + } + + if _, err := client.UpdateContainerResources(ctx, updateReq); err != nil { + t.Fatalf("updating container resources for %s with %v", podID, err) + } + }) + } +} + +func Test_Pod_UpdateResources_Memory_PA(t *testing.T) { + type config struct { + name string + requiredFeatures []string + runtimeHandler string + sandboxImage string + } + tests := []config{ + { + name: "WCOW_Hypervisor", + requiredFeatures: []string{featureWCOWHypervisor}, + runtimeHandler: wcowHypervisorRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + }, + { + name: "LCOW", + requiredFeatures: []string{featureLCOW}, + runtimeHandler: lcowRuntimeHandler, + sandboxImage: imageLcowK8sPause, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + if test.runtimeHandler == lcowRuntimeHandler { + pullRequiredLcowImages(t, []string{test.sandboxImage}) + } else { + pullRequiredImages(t, []string{test.sandboxImage}) + } + var startingMemorySize int64 = 200 * 1024 * 1024 + podRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name(), + Namespace: testNamespace, + }, + Annotations: map[string]string{ + "io.microsoft.virtualmachine.fullyphysicallybacked": "true", + "io.microsoft.container.memory.sizeinmb": fmt.Sprintf("%d", startingMemorySize), + }, + }, + RuntimeHandler: test.runtimeHandler, + } + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podID := runPodSandbox(t, client, ctx, podRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + // make request for shrinking memory size + newMemorySize := startingMemorySize / 2 + updateReq := &runtime.UpdateContainerResourcesRequest{ + ContainerId: podID, + } + + if test.runtimeHandler == lcowRuntimeHandler { + updateReq.Linux = &runtime.LinuxContainerResources{ + MemoryLimitInBytes: newMemorySize, + } + } else { + updateReq.Windows = &runtime.WindowsContainerResources{ + MemoryLimitInBytes: newMemorySize, + } + } + + if _, err := client.UpdateContainerResources(ctx, updateReq); err != nil { + t.Fatalf("updating container resources for %s with %v", podID, err) + } + }) + } +} + +func Test_Pod_UpdateResources_CPUShares(t *testing.T) { + type config struct { + name string + requiredFeatures []string + runtimeHandler string + sandboxImage string + } + tests := []config{ + { + name: "WCOW_Hypervisor", + requiredFeatures: []string{featureWCOWHypervisor}, + runtimeHandler: wcowHypervisorRuntimeHandler, + sandboxImage: imageWindowsNanoserver, + }, + { + name: "LCOW", + requiredFeatures: []string{featureLCOW}, + runtimeHandler: lcowRuntimeHandler, + sandboxImage: imageLcowK8sPause, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + if test.runtimeHandler == lcowRuntimeHandler { + pullRequiredLcowImages(t, []string{test.sandboxImage}) + } else { + pullRequiredImages(t, []string{test.sandboxImage}) + } + podRequest := &runtime.RunPodSandboxRequest{ + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: t.Name(), + Namespace: testNamespace, + }, + }, + RuntimeHandler: test.runtimeHandler, + } + + client := newTestRuntimeClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + podID := runPodSandbox(t, client, ctx, podRequest) + defer removePodSandbox(t, client, ctx, podID) + defer stopPodSandbox(t, client, ctx, podID) + + updateReq := &runtime.UpdateContainerResourcesRequest{ + ContainerId: podID, + } + + if test.runtimeHandler == lcowRuntimeHandler { + updateReq.Linux = &runtime.LinuxContainerResources{ + CpuShares: 2000, + } + } else { + updateReq.Windows = &runtime.WindowsContainerResources{ + CpuShares: 2000, + } + } + + if _, err := client.UpdateContainerResources(ctx, updateReq); err != nil { + t.Fatalf("updating container resources for %s with %v", podID, err) + } + }) + } +} diff --git a/test/cri-containerd/share.go b/test/cri-containerd/share.go new file mode 100644 index 0000000000..7b6a33708c --- /dev/null +++ b/test/cri-containerd/share.go @@ -0,0 +1,22 @@ +// +build functional + +package cri_containerd + +import ( + "context" + + "github.com/Microsoft/hcsshim/internal/shimdiag" +) + +func shareInUVM(ctx context.Context, client shimdiag.ShimDiagService, hostPath, uvmPath string, readOnly bool) error { + req := &shimdiag.ShareRequest{ + HostPath: hostPath, + UvmPath: uvmPath, + ReadOnly: readOnly, + } + _, err := client.DiagShare(ctx, req) + if err != nil { + return err + } + return nil +} diff --git a/test/cri-containerd/update_utilities.go b/test/cri-containerd/update_utilities.go new file mode 100644 index 0000000000..24e8551444 --- /dev/null +++ b/test/cri-containerd/update_utilities.go @@ -0,0 +1,95 @@ +// +build functional + +package cri_containerd + +import ( + "bufio" + "bytes" + "context" + "strconv" + "strings" + "testing" + + "github.com/Microsoft/hcsshim/internal/shimdiag" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" +) + +const podJobObjectUtilPath = "C:\\jobobject-util.exe" + +func containerJobObjectName(id string) string { + return "\\Container_" + id +} + +func createJobObjectsGetUtilArgs(ctx context.Context, cid, toolPath string, options []string) []string { + args := []string{"cmd", "/c", toolPath, "get"} + args = append(args, options...) + args = append(args, containerJobObjectName(cid)) + return args +} + +func checkLCOWResourceLimit(t *testing.T, ctx context.Context, client runtime.RuntimeServiceClient, cid, path string, expected uint64) { + cmd := []string{"cat", path} + containerExecReq := &runtime.ExecSyncRequest{ + ContainerId: cid, + Cmd: cmd, + Timeout: 20, + } + r := execSync(t, client, ctx, containerExecReq) + if r.ExitCode != 0 { + t.Fatalf("failed with exit code %d to cat path: %s", r.ExitCode, r.Stderr) + } + output := strings.TrimSpace(string(r.Stdout)) + bytesActual, err := strconv.ParseUint(output, 10, 0) + if err != nil { + t.Fatalf("could not parse output %s: %s", output, err) + } + if bytesActual != expected { + t.Fatalf("expected to have a memory limit of %v, instead got %v", expected, bytesActual) + } +} + +func checkWCOWResourceLimit(t *testing.T, ctx context.Context, runtimeHandler, shimName, cid, query string, expected uint64) { + shim, err := shimdiag.GetShim(shimName) + if err != nil { + t.Fatalf("failed to find shim %v: %v", shimName, err) + } + shimClient := shimdiag.NewShimDiagClient(shim) + + // share the test utility in so we can query the job object + guestPath := "" + if runtimeHandler == wcowProcessRuntimeHandler { + guestPath = testJobObjectUtilFilePath + } else { + guestPath = podJobObjectUtilPath + if err := shareInUVM(ctx, shimClient, testJobObjectUtilFilePath, guestPath, false); err != nil { + t.Fatalf("failed to share test utility into pod: %v", err) + } + } + + // query the job object + options := []string{"--use-nt", "--" + query} + args := createJobObjectsGetUtilArgs(ctx, cid, guestPath, options) + + buf := &bytes.Buffer{} + bw := bufio.NewWriter(buf) + bufErr := &bytes.Buffer{} + bwErr := bufio.NewWriter(bufErr) + + exitCode, err := execInHost(ctx, shimClient, args, nil, bw, bwErr) + if err != nil { + t.Fatalf("failed to exec request in the host with: %v and %v", err, bufErr.String()) + } + if exitCode != 0 { + t.Fatalf("exec request in host failed with exit code %v: %v", exitCode, bufErr.String()) + } + + // validate the results + value := strings.TrimSpace(strings.TrimPrefix(buf.String(), query+": ")) + limitActual, err := strconv.ParseUint(value, 10, 0) + if err != nil { + t.Fatalf("could not parse output %s: %s", buf.String(), err) + } + if limitActual != expected { + t.Fatalf("expected to have a limit of %v, instead got %v", expected, limitActual) + } +} diff --git a/test/functional/wcow_test.go b/test/functional/wcow_test.go index a51dcb0a54..b806835bad 100644 --- a/test/functional/wcow_test.go +++ b/test/functional/wcow_test.go @@ -12,10 +12,10 @@ import ( "github.com/Microsoft/hcsshim" "github.com/Microsoft/hcsshim/internal/cow" + "github.com/Microsoft/hcsshim/internal/hcs/schema1" "github.com/Microsoft/hcsshim/internal/hcsoci" layerspkg "github.com/Microsoft/hcsshim/internal/layers" "github.com/Microsoft/hcsshim/internal/resources" - "github.com/Microsoft/hcsshim/internal/hcs/schema1" "github.com/Microsoft/hcsshim/internal/schemaversion" "github.com/Microsoft/hcsshim/internal/uvm" "github.com/Microsoft/hcsshim/internal/uvmfolder" diff --git a/test/internal/schemaversion_test.go b/test/internal/schemaversion_test.go index 99beaca45d..cb7ecf241a 100644 --- a/test/internal/schemaversion_test.go +++ b/test/internal/schemaversion_test.go @@ -4,7 +4,7 @@ import ( "io/ioutil" "testing" - "github.com/Microsoft/hcsshim/internal/hcs/schema2" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/schemaversion" "github.com/Microsoft/hcsshim/osversion" _ "github.com/Microsoft/hcsshim/test/functional/manifest" diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/container.go b/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/container.go index de4758435c..6ec5b8b84f 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/container.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/container.go @@ -14,9 +14,7 @@ import ( "go.opencensus.io/trace" ) -const ( - hrComputeSystemDoesNotExist = 0xc037010e -) +const hrComputeSystemDoesNotExist = 0xc037010e // Container implements the cow.Container interface for containers // created via GuestConnection. diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/guestconnection.go b/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/guestconnection.go index 02c08e4eb9..f01b97939b 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/guestconnection.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/guestconnection.go @@ -193,25 +193,6 @@ func (gc *GuestConnection) DeleteContainerState(ctx context.Context, cid string) return gc.brdg.RPC(ctx, rpcDeleteContainerState, &req, &resp, false) } -func (gc *GuestConnection) UpdateContainer(ctx context.Context, cid string, resources interface{}) (err error) { - ctx, span := trace.StartSpan(ctx, "gcs::GuestConnection::UpdateContainer") - defer span.End() - defer func() { oc.SetSpanStatus(span, err) }() - span.AddAttributes(trace.StringAttribute("cid", cid)) - - resourcesJSON, err := json.Marshal(resources) - if err != nil { - return err - } - - req := updateContainerRequest{ - requestBase: makeRequest(ctx, cid), - Resources: string(resourcesJSON), - } - var resp responseBase - return gc.brdg.RPC(ctx, rpcUpdateContainer, &req, &resp, false) -} - // Close terminates the guest connection. It is undefined to call any other // methods on the connection after this is called. func (gc *GuestConnection) Close() error { diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/protocol.go b/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/protocol.go index 5a32a2c1ce..d559a86364 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/protocol.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/gcs/protocol.go @@ -361,8 +361,3 @@ type containerGetPropertiesResponseV2 struct { responseBase Properties containerPropertiesV2 } - -type updateContainerRequest struct { - requestBase - Resources string -} diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/guestrequest/types.go b/test/vendor/github.com/Microsoft/hcsshim/internal/guestrequest/types.go index 1b32adb42a..bfe83eab44 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/guestrequest/types.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/guestrequest/types.go @@ -2,6 +2,7 @@ package guestrequest import ( hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/opencontainers/runtime-spec/specs-go" ) // Arguably, many of these (at least CombinedLayers) should have been generated @@ -67,18 +68,24 @@ type LCOWNetworkAdapter struct { EncapOverhead uint16 `json:",omitempty"` } +type LCOWContainerConstraints struct { + Windows specs.WindowsResources `json:",omitempty"` + Linux specs.LinuxResources `json:",omitempty"` +} + type ResourceType string const ( // These are constants for v2 schema modify guest requests. - ResourceTypeMappedDirectory ResourceType = "MappedDirectory" - ResourceTypeMappedVirtualDisk ResourceType = "MappedVirtualDisk" - ResourceTypeNetwork ResourceType = "Network" - ResourceTypeNetworkNamespace ResourceType = "NetworkNamespace" - ResourceTypeCombinedLayers ResourceType = "CombinedLayers" - ResourceTypeVPMemDevice ResourceType = "VPMemDevice" - ResourceTypeVPCIDevice ResourceType = "VPCIDevice" - ResourceTypeHvSocket ResourceType = "HvSocket" + ResourceTypeMappedDirectory ResourceType = "MappedDirectory" + ResourceTypeMappedVirtualDisk ResourceType = "MappedVirtualDisk" + ResourceTypeNetwork ResourceType = "Network" + ResourceTypeNetworkNamespace ResourceType = "NetworkNamespace" + ResourceTypeCombinedLayers ResourceType = "CombinedLayers" + ResourceTypeVPMemDevice ResourceType = "VPMemDevice" + ResourceTypeVPCIDevice ResourceType = "VPCIDevice" + ResourceTypeContainerConstraints ResourceType = "ContainerConstraints" + ResourceTypeHvSocket ResourceType = "HvSocket" ) // GuestRequest is for modify commands passed to the guest. diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/resourcepaths/silo.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/resourcepaths/silo.go index f426d32a1e..531e7c0b5d 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/resourcepaths/silo.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcs/resourcepaths/silo.go @@ -8,4 +8,5 @@ const ( SiloMappedPipeResourcePath string = "Container/MappedPipes" SiloMemoryResourcePath string = "Container/Memory/SizeInMB" SiloRegistryFlushStatePath string = "Container/RegistryFlushState" + SiloProcessorResourcePath string = "Container/Processor" ) diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/hcsdoc_wcow.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/hcsdoc_wcow.go index 12ddf1aade..7423668dda 100644 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/hcsdoc_wcow.go +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/hcsdoc_wcow.go @@ -119,14 +119,7 @@ func ConvertCPULimits(ctx context.Context, cid string, spec *specs.Spec, maxCPUC if cpuNumSet > 1 { return 0, 0, 0, fmt.Errorf("invalid spec - Windows Container CPU Count: '%d', Limit: '%d', and Weight: '%d' are mutually exclusive", cpuCount, cpuLimit, cpuWeight) } else if cpuNumSet == 1 { - if cpuCount > maxCPUCount { - log.G(ctx).WithFields(logrus.Fields{ - "cid": cid, - "requested": cpuCount, - "assigned": maxCPUCount, - }).Warn("Changing user requested CPUCount to current number of processors") - cpuCount = maxCPUCount - } + cpuCount = NormalizeProcessorCount(ctx, cid, cpuCount, maxCPUCount) } return cpuCount, cpuLimit, cpuWeight, nil } diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go new file mode 100644 index 0000000000..46ad55660f --- /dev/null +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/hcsoci/resources.go @@ -0,0 +1,35 @@ +package hcsoci + +import ( + "context" + + "github.com/Microsoft/hcsshim/internal/log" + "github.com/sirupsen/logrus" +) + +// NormalizeProcessorCount returns the `Min(requested, logical CPU count)`. +func NormalizeProcessorCount(ctx context.Context, cid string, requestedCount, hostCount int32) int32 { + if requestedCount > hostCount { + log.G(ctx).WithFields(logrus.Fields{ + "id": cid, + "requested count": requestedCount, + "assigned count": hostCount, + }).Warn("Changing user requested cpu count to current number of processors on the host") + return hostCount + } else { + return requestedCount + } +} + +// NormalizeMemorySize returns the requested memory size in MB aligned up to an even number +func NormalizeMemorySize(ctx context.Context, cid string, requestedSizeMB uint64) uint64 { + actualMB := (requestedSizeMB + 1) &^ 1 // align up to an even number + if requestedSizeMB != actualMB { + log.G(ctx).WithFields(logrus.Fields{ + "id": cid, + "requestedMB": requestedSizeMB, + "actualMB": actualMB, + }).Warn("Changing user requested MemorySizeInMB to align to 2MB") + } + return actualMB +} diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update.go deleted file mode 100644 index ee576767df..0000000000 --- a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update.go +++ /dev/null @@ -1,12 +0,0 @@ -package uvm - -import ( - "context" -) - -func (uvm *UtilityVM) UpdateContainer(ctx context.Context, cid string, resources interface{}) error { - if uvm.gc == nil || !uvm.guestCaps.UpdateContainerSupported { - return nil - } - return uvm.gc.UpdateContainer(ctx, cid, resources) -} diff --git a/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update_uvm.go b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update_uvm.go new file mode 100644 index 0000000000..635c81ae81 --- /dev/null +++ b/test/vendor/github.com/Microsoft/hcsshim/internal/uvm/update_uvm.go @@ -0,0 +1,56 @@ +package uvm + +import ( + "context" + + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + specs "github.com/opencontainers/runtime-spec/specs-go" +) + +func (uvm *UtilityVM) UpdateConstraints(ctx context.Context, data interface{}, annotations map[string]string) error { + var memoryLimitInBytes *uint64 + var processorLimits *hcsschema.ProcessorLimits + + switch resources := data.(type) { + case *specs.WindowsResources: + if resources.Memory != nil { + memoryLimitInBytes = resources.Memory.Limit + } + if resources.CPU != nil { + processorLimits := &hcsschema.ProcessorLimits{} + if resources.CPU.Maximum != nil { + processorLimits.Limit = uint64(*resources.CPU.Maximum) + } + if resources.CPU.Shares != nil { + processorLimits.Weight = uint64(*resources.CPU.Shares) + } + } + case *specs.LinuxResources: + if resources.Memory != nil { + mem := uint64(*resources.Memory.Limit) + memoryLimitInBytes = &mem + } + if resources.CPU != nil { + processorLimits := &hcsschema.ProcessorLimits{} + if resources.CPU.Quota != nil { + processorLimits.Limit = uint64(*resources.CPU.Quota) + } + if resources.CPU.Shares != nil { + processorLimits.Weight = uint64(*resources.CPU.Shares) + } + } + } + + if memoryLimitInBytes != nil { + if err := uvm.UpdateMemory(ctx, *memoryLimitInBytes); err != nil { + return err + } + } + if processorLimits != nil { + if err := uvm.UpdateCPULimits(ctx, processorLimits); err != nil { + return err + } + } + + return nil +}