From 642f7826db99a41582b2ea1cd875857f50bf6e48 Mon Sep 17 00:00:00 2001 From: Kathryn Baldauf Date: Fri, 22 Jan 2021 18:29:37 -0800 Subject: [PATCH] Change the update container bridge call to use the modify call instead Signed-off-by: Kathryn Baldauf --- internal/runtime/hcsv2/container.go | 6 +++++- internal/runtime/hcsv2/uvm.go | 31 +++++++++++++++++++++++++++-- service/gcs/bridge/bridge.go | 1 - service/gcs/bridge/bridge_v2.go | 31 +---------------------------- service/gcs/prot/protocol.go | 24 ++++++++++++---------- service/gcs/runtime/runc/runc.go | 10 +++++++--- service/gcs/runtime/runtime.go | 2 +- 7 files changed, 56 insertions(+), 49 deletions(-) diff --git a/internal/runtime/hcsv2/container.go b/internal/runtime/hcsv2/container.go index b0590075..51c8d7f7 100644 --- a/internal/runtime/hcsv2/container.go +++ b/internal/runtime/hcsv2/container.go @@ -129,7 +129,7 @@ func (c *Container) Delete(ctx context.Context) error { return c.container.Delete() } -func (c *Container) Update(ctx context.Context, resources string) error { +func (c *Container) Update(ctx context.Context, resources interface{}) error { return c.container.Update(resources) } @@ -172,3 +172,7 @@ func (c *Container) GetStats(ctx context.Context) (*v1.Metrics, error) { return cg.Stat(cgroups.IgnoreNotExist) } + +func (c *Container) modifyContainerConstraints(ctx context.Context, rt prot.ModifyRequestType, cc *prot.ContainerConstraintsV2) (err error) { + return c.Update(ctx, cc.Linux) +} diff --git a/internal/runtime/hcsv2/uvm.go b/internal/runtime/hcsv2/uvm.go index ec47a8fb..cddddbac 100644 --- a/internal/runtime/hcsv2/uvm.go +++ b/internal/runtime/hcsv2/uvm.go @@ -193,7 +193,7 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM return c, nil } -func (h *Host) ModifyHostSettings(ctx context.Context, settings *prot.ModifySettingRequest) error { +func (h *Host) modifyHostSettings(ctx context.Context, containerID string, settings *prot.ModifySettingRequest) error { switch settings.ResourceType { case prot.MrtMappedVirtualDisk: return modifyMappedVirtualDisk(ctx, settings.RequestType, settings.Settings.(*prot.MappedVirtualDiskV2)) @@ -207,9 +207,36 @@ func (h *Host) ModifyHostSettings(ctx context.Context, settings *prot.ModifySett return modifyNetwork(ctx, settings.RequestType, settings.Settings.(*prot.NetworkAdapterV2)) case prot.MrtVPCIDevice: return modifyMappedVPCIDevice(ctx, settings.RequestType, settings.Settings.(*prot.MappedVPCIDeviceV2)) + case prot.MrtContainerConstraints: + c, err := h.GetContainer(containerID) + if err != nil { + return err + } + return c.modifyContainerConstraints(ctx, settings.RequestType, settings.Settings.(*prot.ContainerConstraintsV2)) + default: + return errors.Errorf("the ResourceType \"%s\" is not supported for UVM", settings.ResourceType) + } +} + +func (h *Host) modifyContainerSettings(ctx context.Context, containerID string, settings *prot.ModifySettingRequest) error { + c, err := h.GetContainer(containerID) + if err != nil { + return err + } + + switch settings.ResourceType { + case prot.MrtContainerConstraints: + return c.modifyContainerConstraints(ctx, settings.RequestType, settings.Settings.(*prot.ContainerConstraintsV2)) default: - return errors.Errorf("the ResourceType \"%s\" is not supported", settings.ResourceType) + return errors.Errorf("the ResourceType \"%s\" is not supported for containers", settings.ResourceType) + } +} + +func (h *Host) ModifySettings(ctx context.Context, containerID string, settings *prot.ModifySettingRequest) error { + if containerID == UVMContainerID { + return h.modifyHostSettings(ctx, containerID, settings) } + return h.modifyContainerSettings(ctx, containerID, settings) } // Shutdown terminates this UVM. This is a destructive call and will destroy all diff --git a/service/gcs/bridge/bridge.go b/service/gcs/bridge/bridge.go index f69cafbd..2c52e86a 100644 --- a/service/gcs/bridge/bridge.go +++ b/service/gcs/bridge/bridge.go @@ -215,7 +215,6 @@ func (b *Bridge) AssignHandlers(mux *Mux, host *hcsv2.Host) { mux.HandleFunc(prot.ComputeSystemModifySettingsV1, prot.PvV4, b.modifySettingsV2) mux.HandleFunc(prot.ComputeSystemDumpStacksV1, prot.PvV4, b.dumpStacksV2) mux.HandleFunc(prot.ComputeSystemDeleteContainerStateV1, prot.PvV4, b.deleteContainerStateV2) - mux.HandleFunc(prot.ComputeSystemUpdateContainerV1, prot.PvV4, b.updateContainerV1) } } diff --git a/service/gcs/bridge/bridge_v2.go b/service/gcs/bridge/bridge_v2.go index eda54403..7652f876 100644 --- a/service/gcs/bridge/bridge_v2.go +++ b/service/gcs/bridge/bridge_v2.go @@ -36,7 +36,6 @@ var capabilities = prot.GcsCapabilities{ SignalProcessSupported: true, DumpStacksSupported: true, DeleteContainerStateSupported: true, - UpdateContainerSupported: true, }, } @@ -478,11 +477,7 @@ func (b *Bridge) modifySettingsV2(r *Request) (_ RequestResponse, err error) { return nil, errors.Wrapf(err, "failed to unmarshal JSON in message \"%s\"", r.Message) } - if request.ContainerID != hcsv2.UVMContainerID { - return nil, errors.New("V2 Modify request not supported on anything but UVM") - } - - err = b.hostState.ModifyHostSettings(ctx, request.Request.(*prot.ModifySettingRequest)) + err = b.hostState.ModifySettings(ctx, request.ContainerID, request.Request.(*prot.ModifySettingRequest)) if err != nil { return nil, err } @@ -526,27 +521,3 @@ func (b *Bridge) deleteContainerStateV2(r *Request) (_ RequestResponse, err erro b.hostState.RemoveContainer(request.ContainerID) return &prot.MessageResponseBase{}, nil } - -func (b *Bridge) updateContainerV1(r *Request) (_ RequestResponse, err error) { - ctx, span := trace.StartSpan(r.Context, "opengcs::bridge::updateContainerV1") - defer span.End() - defer func() { oc.SetSpanStatus(span, err) }() - span.AddAttributes(trace.StringAttribute("cid", r.ContainerID)) - - c, err := b.hostState.GetContainer(r.ContainerID) - if err != nil { - return nil, err - } - var request prot.ContainerUpdate - if err := commonutils.UnmarshalJSONWithHresult(r.Message, &request); err != nil { - return nil, errors.Wrapf(err, "failed to unmarshal JSON in message \"%s\"", r.Message) - } - - if len(request.Resources) == 0 { - return nil, errors.New("cannot update container with empty resources") - } - if err := c.Update(ctx, request.Resources); err != nil { - return nil, err - } - return &prot.MessageResponseBase{}, nil -} diff --git a/service/gcs/prot/protocol.go b/service/gcs/prot/protocol.go index d8f0f052..c35031ad 100644 --- a/service/gcs/prot/protocol.go +++ b/service/gcs/prot/protocol.go @@ -104,8 +104,6 @@ const ( ComputeSystemDumpStacksV1 = 0x10100c01 // ComputeSystemDeleteContainerStateV1 is the delete container request. ComputeSystemDeleteContainerStateV1 = 0x10100d01 - // ComputeSystemUpdateContainerV1 is the update container request - ComputeSystemUpdateContainerV1 = 0x10100e01 // ComputeSystemResponseCreateV1 is the create container response. ComputeSystemResponseCreateV1 = 0x20100101 @@ -172,8 +170,6 @@ func (mi MessageIdentifier) String() string { return "ComputeSystemDumpStacksV1" case ComputeSystemDeleteContainerStateV1: return "ComputeSystemDeleteContainerStateV1" - case ComputeSystemUpdateContainerV1: - return "ComputeSystemUpdateContainerV1" case ComputeSystemResponseCreateV1: return "ComputeSystemResponseCreateV1" case ComputeSystemResponseStartV1: @@ -275,7 +271,6 @@ type GcsGuestCapabilities struct { SignalProcessSupported bool `json:",omitempty"` DumpStacksSupported bool `json:",omitempty"` DeleteContainerStateSupported bool `json:",omitempty"` - UpdateContainerSupported bool `json:",omitempty"` } // ocspancontext is the internal JSON representation of the OpenCensus @@ -452,12 +447,6 @@ type ContainerGetProperties struct { Query string } -// ContainerUpdate is the message sent to update a container's resources -type ContainerUpdate struct { - MessageBase - Resources string -} - // PropertyType is the type of property, such as memory or virtual disk, which // is to be modified for the container. type PropertyType string @@ -528,6 +517,8 @@ const ( MrtNetwork = ModifyResourceType("Network") // MrtVPCIDevice is the modify resource type for vpci devices MrtVPCIDevice = ModifyResourceType("VPCIDevice") + // MrtContainerConstraints is the modify resource type for updating container constraints + MrtContainerConstraints = ModifyResourceType("ContainerConstraints") ) // ModifyRequestType is the type of operation to perform on a given modify @@ -621,6 +612,12 @@ func UnmarshalContainerModifySettings(b []byte) (*ContainerModifySettings, error return &request, errors.Wrap(err, "failed to unmarshal settings as MappedVPCIDeviceV2") } msr.Settings = vd + case MrtContainerConstraints: + cc := &ContainerConstraintsV2{} + if err := commonutils.UnmarshalJSONWithHresult(msrRawSettings, cc); err != nil { + return &request, errors.Wrap(err, "failed to unmarshal settings as ContainerConstraintsV2") + } + msr.Settings = cc default: return &request, errors.Errorf("invalid ResourceType '%s'", msr.ResourceType) } @@ -797,6 +794,11 @@ type MappedVPCIDeviceV2 struct { VMBusGUID string `json:",omitempty"` } +type ContainerConstraintsV2 struct { + Windows oci.WindowsResources `json:",omitempty"` + Linux oci.LinuxResources `json:",omitempty"` +} + // VMHostedContainerSettings is the set of settings used to specify the initial // configuration of a container. type VMHostedContainerSettings struct { diff --git a/service/gcs/runtime/runc/runc.go b/service/gcs/runtime/runc/runc.go index 8b69b61b..3899d9cd 100644 --- a/service/gcs/runtime/runc/runc.go +++ b/service/gcs/runtime/runc/runc.go @@ -693,15 +693,19 @@ func (c *container) startProcess(tempProcessDir string, hasTerminal bool, stdioS return &process{c: c, pid: pid, ttyRelay: ttyRelay, pipeRelay: pipeRelay}, nil } -func (c *container) Update(resources string) error { +func (c *container) Update(resources interface{}) error { + jsonResources, err := json.Marshal(resources) + if err != nil { + return err + } logPath := c.r.getLogPath(c.id) args := []string{"update", "--resources", "-", c.id} cmd := createRuncCommand(logPath, args...) - cmd.Stdin = strings.NewReader(resources) + cmd.Stdin = strings.NewReader(string(jsonResources)) out, err := cmd.CombinedOutput() if err != nil { runcErr := getRuncLogError(logPath) - return errors.Wrapf(err, "runc update failed with %v: %s", runcErr, string(out)) + return errors.Wrapf(err, "runc update request %s failed with %v: %s", string(jsonResources), runcErr, string(out)) } return nil } diff --git a/service/gcs/runtime/runtime.go b/service/gcs/runtime/runtime.go index 210c3585..431fec43 100644 --- a/service/gcs/runtime/runtime.go +++ b/service/gcs/runtime/runtime.go @@ -60,7 +60,7 @@ type Container interface { GetState() (*ContainerState, error) GetRunningProcesses() ([]ContainerProcessState, error) GetAllProcesses() ([]ContainerProcessState, error) - Update(resources string) error + Update(resources interface{}) error } // Runtime is the interface defining commands over an OCI container runtime,