Merge pull request #391 from katiewasnothere/update_to_modify_call

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

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)
+}

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

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)
 	}
 }
 

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
-}

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 {

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
 }

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,