Skip to content
This repository has been archived by the owner on Jul 28, 2021. It is now read-only.

Change the update container bridge call to use the modify call instead #391

Merged
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
6 changes: 5 additions & 1 deletion internal/runtime/hcsv2/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
31 changes: 29 additions & 2 deletions internal/runtime/hcsv2/uvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use %q and get rid of the quotes

}
}

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
Expand Down
1 change: 0 additions & 1 deletion service/gcs/bridge/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
31 changes: 1 addition & 30 deletions service/gcs/bridge/bridge_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ var capabilities = prot.GcsCapabilities{
SignalProcessSupported: true,
DumpStacksSupported: true,
DeleteContainerStateSupported: true,
UpdateContainerSupported: true,
},
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
24 changes: 13 additions & 11 deletions service/gcs/prot/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 7 additions & 3 deletions service/gcs/runtime/runc/runc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion service/gcs/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down