diff --git a/cmd/podman/containers/update.go b/cmd/podman/containers/update.go index 2929f37a71..429bd96821 100644 --- a/cmd/podman/containers/update.go +++ b/cmd/podman/containers/update.go @@ -113,6 +113,16 @@ func GetChangedHealthCheckConfiguration(cmd *cobra.Command, vals *entities.Conta return updateHealthCheckConfig } +func GetChangedDeviceLimits(s *specgen.SpecGenerator) *define.UpdateContainerDevicesLimits { + updateDevicesLimits := define.UpdateContainerDevicesLimits{} + updateDevicesLimits.SetBlkIOWeightDevice(s.WeightDevice) + updateDevicesLimits.SetDeviceReadBPs(s.ThrottleReadBpsDevice) + updateDevicesLimits.SetDeviceWriteBPs(s.ThrottleWriteBpsDevice) + updateDevicesLimits.SetDeviceReadIOPs(s.ThrottleReadIOPSDevice) + updateDevicesLimits.SetDeviceWriteIOPs(s.ThrottleWriteIOPSDevice) + return &updateDevicesLimits +} + func update(cmd *cobra.Command, args []string) error { var err error // use a specgen since this is the easiest way to hold resource info @@ -124,33 +134,35 @@ func update(cmd *cobra.Command, args []string) error { return err } - if updateOpts.Restart != "" { - policy, retries, err := util.ParseRestartPolicy(updateOpts.Restart) - if err != nil { - return err - } - s.RestartPolicy = policy - if policy == define.RestartPolicyOnFailure { - s.RestartRetries = &retries - } - } - - // we need to pass the whole specgen since throttle devices are parsed later due to cross compat. s.ResourceLimits, err = specgenutil.GetResources(s, &updateOpts) if err != nil { return err } - healthCheckConfig := GetChangedHealthCheckConfiguration(cmd, &updateOpts) - if err != nil { - return err + if s.ResourceLimits == nil { + s.ResourceLimits = &specs.LinuxResources{} } + healthCheckConfig := GetChangedHealthCheckConfiguration(cmd, &updateOpts) + opts := &entities.ContainerUpdateOptions{ NameOrID: strings.TrimPrefix(args[0], "/"), - Specgen: s, + Resources: s.ResourceLimits, ChangedHealthCheckConfiguration: &healthCheckConfig, + DevicesLimits: GetChangedDeviceLimits(s), + } + + if cmd.Flags().Changed("restart") { + policy, retries, err := util.ParseRestartPolicy(updateOpts.Restart) + if err != nil { + return err + } + opts.RestartPolicy = &policy + if policy == define.RestartPolicyOnFailure { + opts.RestartRetries = &retries + } } + rep, err := registry.ContainerEngine().ContainerUpdate(context.Background(), opts) if err != nil { return err diff --git a/libpod/define/container.go b/libpod/define/container.go index 5dd5465dfe..56cd225adb 100644 --- a/libpod/define/container.go +++ b/libpod/define/container.go @@ -2,6 +2,8 @@ package define import ( "fmt" + + "github.com/opencontainers/runtime-spec/specs-go" ) // Valid restart policy types. @@ -64,3 +66,111 @@ const ( // a Job kube yaml spec K8sKindJob = "job" ) + +type WeightDevice struct { + Path string + Weight uint16 +} + +type ThrottleDevice struct { + Path string + Rate uint64 +} + +type UpdateContainerDevicesLimits struct { + // Block IO weight (relative device weight) in the form: + // ```[{"Path": "device_path", "Weight": weight}]``` + BlkIOWeightDevice []WeightDevice `json:",omitempty"` + // Limit read rate (bytes per second) from a device, in the form: + // ```[{"Path": "device_path", "Rate": rate}]``` + DeviceReadBPs []ThrottleDevice `json:",omitempty"` + // Limit write rate (bytes per second) to a device, in the form: + // ```[{"Path": "device_path", "Rate": rate}]``` + DeviceWriteBPs []ThrottleDevice `json:",omitempty"` + // Limit read rate (IO per second) from a device, in the form: + // ```[{"Path": "device_path", "Rate": rate}]``` + DeviceReadIOPs []ThrottleDevice `json:",omitempty"` + // Limit write rate (IO per second) to a device, in the form: + // ```[{"Path": "device_path", "Rate": rate}]``` + DeviceWriteIOPs []ThrottleDevice `json:",omitempty"` +} + +func (d *WeightDevice) addToLinuxWeightDevice(wd map[string]specs.LinuxWeightDevice) { + wd[d.Path] = specs.LinuxWeightDevice{ + Weight: &d.Weight, + LeafWeight: nil, + } +} + +func LinuxWeightDeviceToWeightDevice(path string, d specs.LinuxWeightDevice) WeightDevice { + return WeightDevice{Path: path, Weight: *d.Weight} +} + +func (d *ThrottleDevice) addToLinuxThrottleDevice(td map[string]specs.LinuxThrottleDevice) { + td[d.Path] = specs.LinuxThrottleDevice{Rate: d.Rate} +} + +func LinuxThrottleDevicesToThrottleDevices(path string, d specs.LinuxThrottleDevice) ThrottleDevice { + return ThrottleDevice{Path: path, Rate: d.Rate} +} + +func (u *UpdateContainerDevicesLimits) SetBlkIOWeightDevice(wd map[string]specs.LinuxWeightDevice) { + for path, dev := range wd { + u.BlkIOWeightDevice = append(u.BlkIOWeightDevice, LinuxWeightDeviceToWeightDevice(path, dev)) + } +} + +func copyLinuxThrottleDevicesFromMapToThrottleDevicesArray(source map[string]specs.LinuxThrottleDevice, dest []ThrottleDevice) []ThrottleDevice { + for path, dev := range source { + dest = append(dest, LinuxThrottleDevicesToThrottleDevices(path, dev)) + } + return dest +} + +func (u *UpdateContainerDevicesLimits) SetDeviceReadBPs(td map[string]specs.LinuxThrottleDevice) { + u.DeviceReadBPs = copyLinuxThrottleDevicesFromMapToThrottleDevicesArray(td, u.DeviceReadBPs) +} + +func (u *UpdateContainerDevicesLimits) SetDeviceWriteBPs(td map[string]specs.LinuxThrottleDevice) { + u.DeviceWriteBPs = copyLinuxThrottleDevicesFromMapToThrottleDevicesArray(td, u.DeviceWriteBPs) +} + +func (u *UpdateContainerDevicesLimits) SetDeviceReadIOPs(td map[string]specs.LinuxThrottleDevice) { + u.DeviceReadIOPs = copyLinuxThrottleDevicesFromMapToThrottleDevicesArray(td, u.DeviceReadIOPs) +} + +func (u *UpdateContainerDevicesLimits) SetDeviceWriteIOPs(td map[string]specs.LinuxThrottleDevice) { + u.DeviceWriteIOPs = copyLinuxThrottleDevicesFromMapToThrottleDevicesArray(td, u.DeviceWriteIOPs) +} + +func (u *UpdateContainerDevicesLimits) GetMapOfLinuxWeightDevice() map[string]specs.LinuxWeightDevice { + wd := make(map[string]specs.LinuxWeightDevice) + for _, dev := range u.BlkIOWeightDevice { + dev.addToLinuxWeightDevice(wd) + } + return wd +} + +func getMapOfLinuxThrottleDevices(source []ThrottleDevice) map[string]specs.LinuxThrottleDevice { + td := make(map[string]specs.LinuxThrottleDevice) + for _, dev := range source { + dev.addToLinuxThrottleDevice(td) + } + return td +} + +func (u *UpdateContainerDevicesLimits) GetMapOfDeviceReadBPs() map[string]specs.LinuxThrottleDevice { + return getMapOfLinuxThrottleDevices(u.DeviceReadBPs) +} + +func (u *UpdateContainerDevicesLimits) GetMapOfDeviceWriteBPs() map[string]specs.LinuxThrottleDevice { + return getMapOfLinuxThrottleDevices(u.DeviceWriteBPs) +} + +func (u *UpdateContainerDevicesLimits) GetMapOfDeviceReadIOPs() map[string]specs.LinuxThrottleDevice { + return getMapOfLinuxThrottleDevices(u.DeviceReadIOPs) +} + +func (u *UpdateContainerDevicesLimits) GetMapOfDeviceWriteIOPs() map[string]specs.LinuxThrottleDevice { + return getMapOfLinuxThrottleDevices(u.DeviceWriteIOPs) +} diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index d9ec050dc9..bd6162c2ce 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -18,6 +18,7 @@ import ( api "github.com/containers/podman/v5/pkg/api/types" "github.com/containers/podman/v5/pkg/domain/entities" "github.com/containers/podman/v5/pkg/domain/infra/abi" + "github.com/containers/podman/v5/pkg/specgenutil" "github.com/containers/podman/v5/pkg/util" "github.com/gorilla/schema" "github.com/sirupsen/logrus" @@ -447,7 +448,14 @@ func UpdateContainer(w http.ResponseWriter, r *http.Request) { utils.Error(w, http.StatusInternalServerError, fmt.Errorf("decode(): %w", err)) return } - err = ctr.Update(&options.LinuxResources, restartPolicy, restartRetries, &options.UpdateHealthCheckConfig) + + resourceLimits, err := specgenutil.UpdateMajorAndMinorNumbers(&options.LinuxResources, &options.UpdateContainerDevicesLimits) + if err != nil { + utils.InternalServerError(w, err) + return + } + + err = ctr.Update(resourceLimits, restartPolicy, restartRetries, &options.UpdateHealthCheckConfig) if err != nil { utils.InternalServerError(w, err) return diff --git a/pkg/api/handlers/swagger/responses.go b/pkg/api/handlers/swagger/responses.go index f3fe133342..18b11efdf2 100644 --- a/pkg/api/handlers/swagger/responses.go +++ b/pkg/api/handlers/swagger/responses.go @@ -324,9 +324,13 @@ type containerCreateResponse struct { Body entities.ContainerCreateResponse } +// Update container +// swagger:response type containerUpdateResponse struct { // in:body - ID string + Body struct { + ID string + } } // Wait container diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index d461344a35..938094ab66 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -76,6 +76,7 @@ type LibpodContainersRmReport struct { type UpdateEntities struct { specs.LinuxResources define.UpdateHealthCheckConfig + define.UpdateContainerDevicesLimits } type Info struct { diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 3adfb2a90f..0ca1c3bec7 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -1804,9 +1804,8 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // produces: // - application/json // responses: - // responses: - // 201: - // $ref: "#/responses/containerUpdateResponse" + // 201: + // $ref: "#/responses/containerUpdateResponse" // 400: // $ref: "#/responses/badParamError" // 404: diff --git a/pkg/bindings/containers/update.go b/pkg/bindings/containers/update.go index 8091158029..786802434e 100644 --- a/pkg/bindings/containers/update.go +++ b/pkg/bindings/containers/update.go @@ -20,15 +20,16 @@ func Update(ctx context.Context, options *types.ContainerUpdateOptions) (string, } params := url.Values{} - if options.Specgen.RestartPolicy != "" { - params.Set("restartPolicy", options.Specgen.RestartPolicy) - if options.Specgen.RestartRetries != nil { - params.Set("restartRetries", strconv.Itoa(int(*options.Specgen.RestartRetries))) + if options.RestartPolicy != nil { + params.Set("restartPolicy", *options.RestartPolicy) + if options.RestartRetries != nil { + params.Set("restartRetries", strconv.Itoa(int(*options.RestartRetries))) } } updateEntities := &handlers.UpdateEntities{ - LinuxResources: *options.Specgen.ResourceLimits, - UpdateHealthCheckConfig: *options.ChangedHealthCheckConfiguration, + LinuxResources: *options.Resources, + UpdateHealthCheckConfig: *options.ChangedHealthCheckConfiguration, + UpdateContainerDevicesLimits: *options.DevicesLimits, } requestData, err := jsoniter.MarshalToString(updateEntities) if err != nil { diff --git a/pkg/domain/entities/types/containers.go b/pkg/domain/entities/types/containers.go index 0e1f563931..ea01a4e666 100644 --- a/pkg/domain/entities/types/containers.go +++ b/pkg/domain/entities/types/containers.go @@ -3,6 +3,7 @@ package types import ( "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/pkg/specgen" + "github.com/opencontainers/runtime-spec/specs-go" ) type ContainerCopyFunc func() error @@ -36,7 +37,55 @@ type ContainerStatsReport struct { } type ContainerUpdateOptions struct { - NameOrID string + NameOrID string + // This individual items of Specgen are used to update container configuration: + // - ResourceLimits + // - WeightDevice + // - ThrottleReadBpsDevice + // - ThrottleWriteBpsDevice + // - ThrottleReadIOPSDevice + // - ThrottleWriteIOPSDevice + // - RestartPolicy + // - RestartRetries + // + // Deprecated: Specgen should not be used to change the container configuration. + // Specgen is processed first, so values will be overwritten by values from other fields. + // + // To change configuration use other fields in ContainerUpdateOptions struct: + // - Resources to change resource configuration + // - DevicesLimits to Limit device + // - RestartPolicy to change restart policy + // - RestartRetries to change restart retries Specgen *specgen.SpecGenerator + Resources *specs.LinuxResources + DevicesLimits *define.UpdateContainerDevicesLimits ChangedHealthCheckConfiguration *define.UpdateHealthCheckConfig + RestartPolicy *string + RestartRetries *uint +} + +func (u *ContainerUpdateOptions) ProcessSpecgen() { + if u.Specgen == nil { + return + } + + if u.Resources == nil { + u.Resources = u.Specgen.ResourceLimits + } + + if u.DevicesLimits == nil { + u.DevicesLimits = new(define.UpdateContainerDevicesLimits) + u.DevicesLimits.SetBlkIOWeightDevice(u.Specgen.WeightDevice) + u.DevicesLimits.SetDeviceReadBPs(u.Specgen.ThrottleReadBpsDevice) + u.DevicesLimits.SetDeviceWriteBPs(u.Specgen.ThrottleWriteBpsDevice) + u.DevicesLimits.SetDeviceReadIOPs(u.Specgen.ThrottleReadIOPSDevice) + u.DevicesLimits.SetDeviceWriteIOPs(u.Specgen.ThrottleWriteIOPSDevice) + } + + if u.RestartPolicy == nil { + u.RestartPolicy = &u.Specgen.RestartPolicy + } + if u.RestartRetries == nil { + u.RestartRetries = u.Specgen.RestartRetries + } } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index db8f9f3c0c..5b9481d694 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1792,14 +1792,7 @@ func (ic *ContainerEngine) ContainerClone(ctx context.Context, ctrCloneOpts enti // ContainerUpdate finds and updates the given container's cgroup config with the specified options func (ic *ContainerEngine) ContainerUpdate(ctx context.Context, updateOptions *entities.ContainerUpdateOptions) (string, error) { - err := specgen.WeightDevices(updateOptions.Specgen) - if err != nil { - return "", err - } - err = specgen.FinishThrottleDevices(updateOptions.Specgen) - if err != nil { - return "", err - } + updateOptions.ProcessSpecgen() containers, err := getContainers(ic.Libpod, getContainersOptions{names: []string{updateOptions.NameOrID}}) if err != nil { return "", err @@ -1809,12 +1802,12 @@ func (ic *ContainerEngine) ContainerUpdate(ctx context.Context, updateOptions *e } container := containers[0].Container - var restartPolicy *string - if updateOptions.Specgen.RestartPolicy != "" { - restartPolicy = &updateOptions.Specgen.RestartPolicy + resourceLimits, err := specgenutil.UpdateMajorAndMinorNumbers(updateOptions.Resources, updateOptions.DevicesLimits) + if err != nil { + return "", err } - if err = container.Update(updateOptions.Specgen.ResourceLimits, restartPolicy, updateOptions.Specgen.RestartRetries, updateOptions.ChangedHealthCheckConfiguration); err != nil { + if err = container.Update(resourceLimits, updateOptions.RestartPolicy, updateOptions.RestartRetries, updateOptions.ChangedHealthCheckConfiguration); err != nil { return "", err } return containers[0].ID(), nil diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 89c4486735..daee6a8219 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -1061,13 +1061,6 @@ func (ic *ContainerEngine) ContainerClone(ctx context.Context, ctrCloneOpts enti // ContainerUpdate finds and updates the given container's cgroup config with the specified options func (ic *ContainerEngine) ContainerUpdate(ctx context.Context, updateOptions *entities.ContainerUpdateOptions) (string, error) { - err := specgen.WeightDevices(updateOptions.Specgen) - if err != nil { - return "", err - } - err = specgen.FinishThrottleDevices(updateOptions.Specgen) - if err != nil { - return "", err - } + updateOptions.ProcessSpecgen() return containers.Update(ic.ClientCtx, updateOptions) } diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index 79a8f66ac1..74ad6829b9 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -1297,3 +1297,27 @@ func GetResources(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions) } return s.ResourceLimits, nil } + +func UpdateMajorAndMinorNumbers(resources *specs.LinuxResources, devicesLimits *define.UpdateContainerDevicesLimits) (*specs.LinuxResources, error) { + spec := specgen.SpecGenerator{} + spec.ResourceLimits = &specs.LinuxResources{} + if resources != nil { + spec.ResourceLimits = resources + } + + spec.WeightDevice = devicesLimits.GetMapOfLinuxWeightDevice() + spec.ThrottleReadBpsDevice = devicesLimits.GetMapOfDeviceReadBPs() + spec.ThrottleWriteBpsDevice = devicesLimits.GetMapOfDeviceWriteBPs() + spec.ThrottleReadIOPSDevice = devicesLimits.GetMapOfDeviceReadIOPs() + spec.ThrottleWriteIOPSDevice = devicesLimits.GetMapOfDeviceWriteIOPs() + + err := specgen.WeightDevices(&spec) + if err != nil { + return nil, err + } + err = specgen.FinishThrottleDevices(&spec) + if err != nil { + return nil, err + } + return spec.ResourceLimits, nil +} diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index c2737f006a..5c82ea2c4e 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -686,19 +686,57 @@ t GET libpod/containers/$cname/json 200 \ if root; then podman run -dt --name=updateCtr alpine - echo '{"Memory":{"Limit":500000}, "CPU":{"Shares":123}}' >${TMPD}/update.json + echo '{ + "Memory":{"Limit":500000}, + "CPU":{"Shares":123}, + "DeviceReadBPs": [{ "Path": "/dev/zero", "Rate": 10485760 }], + "DeviceWriteBPs": [{ "Path": "/dev/zero", "Rate": 31457280 }], + "DeviceReadIOPs": [{ "Path": "/dev/zero", "Rate": 2000 }], + "DeviceWriteIOPs": [{ "Path": "/dev/zero", "Rate": 4000 }] + }' >${TMPD}/update.json t POST libpod/containers/updateCtr/update ${TMPD}/update.json 201 cgroupPath=/sys/fs/cgroup/cpu.weight # 002 is the byte length cpu_weight_expect=$'\001\0025' - # Verify + # Verify CPU weight echo '{ "AttachStdout":true,"Cmd":["cat", "'$cgroupPath'"]}' >${TMPD}/exec.json t POST containers/updateCtr/exec ${TMPD}/exec.json 201 .Id~[0-9a-f]\\{64\\} eid=$(jq -r '.Id' <<<"$output") t POST exec/$eid/start 200 $cpu_weight_expect + BlkioDeviceReadBps_expected='[ + { + "Path": "/dev/zero", + "Rate": 10485760 + } +]' + BlkioDeviceWriteBPs_expected='[ + { + "Path": "/dev/zero", + "Rate": 31457280 + } +]' + BlkioDeviceReadIOPs_expected='[ + { + "Path": "/dev/zero", + "Rate": 2000 + } +]' + BlkioDeviceWriteIOPs_expected='[ + { + "Path": "/dev/zero", + "Rate": 4000 + } +]' + # Verify Device limits + t GET containers/updateCtr/json 200 \ + .HostConfig.BlkioDeviceReadBps="$BlkioDeviceReadBps_expected" \ + .HostConfig.BlkioDeviceWriteBps="$BlkioDeviceWriteBPs_expected" \ + .HostConfig.BlkioDeviceReadIOps="$BlkioDeviceReadIOPs_expected" \ + .HostConfig.BlkioDeviceWriteIOps="$BlkioDeviceWriteIOPs_expected" \ + # Now use the compat API echo '{ "Memory": 536870912 }' >${TMPD}/compatupdate.json t POST containers/updateCtr/update ${TMPD}/compatupdate.json 200