From 6ad20ced5bb73e4f162b910dc25255873e26743e Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 26 Sep 2024 16:56:59 +0100 Subject: [PATCH] Move docker resource limit settings from server to agent (#3174) so you can set it per agent and not per server --- cmd/server/flags.go | 32 +-------- cmd/server/setup.go | 8 --- .../30-administration/10-server-config.md | 38 ---------- .../22-backends/10-docker.md | 38 ++++++++++ docs/docs/91-migrations.md | 3 +- pipeline/backend/docker/config.go | 71 +++++++++++++++++++ pipeline/backend/docker/convert.go | 14 ++-- pipeline/backend/docker/convert_test.go | 65 +++++++++-------- pipeline/backend/docker/docker.go | 37 +++------- pipeline/backend/docker/flags.go | 33 +++++++++ pipeline/backend/types/step.go | 6 -- pipeline/frontend/yaml/compiler/compiler.go | 10 --- pipeline/frontend/yaml/compiler/convert.go | 31 -------- pipeline/frontend/yaml/compiler/option.go | 15 ---- .../frontend/yaml/compiler/option_test.go | 19 ----- pipeline/frontend/yaml/linter/linter.go | 3 - pipeline/frontend/yaml/linter/linter_test.go | 4 -- pipeline/frontend/yaml/types/container.go | 18 ++--- .../frontend/yaml/types/container_test.go | 42 ++++------- server/config.go | 1 - server/model/resource_limit.go | 25 ------- server/pipeline/stepbuilder/stepBuilder.go | 1 - 22 files changed, 221 insertions(+), 293 deletions(-) create mode 100644 pipeline/backend/docker/config.go delete mode 100644 server/model/resource_limit.go diff --git a/cmd/server/flags.go b/cmd/server/flags.go index 18023e207..4b5e5a811 100644 --- a/cmd/server/flags.go +++ b/cmd/server/flags.go @@ -295,36 +295,8 @@ var flags = append([]cli.Flag{ Usage: "How many retries of fetching the Woodpecker configuration from a forge are done before we fail", Value: 3, }, - &cli.IntFlag{ - Sources: cli.EnvVars("WOODPECKER_LIMIT_MEM_SWAP"), - Name: "limit-mem-swap", - Usage: "maximum memory used for swap in bytes", - }, - &cli.IntFlag{ - Sources: cli.EnvVars("WOODPECKER_LIMIT_MEM"), - Name: "limit-mem", - Usage: "maximum memory allowed in bytes", - }, - &cli.IntFlag{ - Sources: cli.EnvVars("WOODPECKER_LIMIT_SHM_SIZE"), - Name: "limit-shm-size", - Usage: "docker compose /dev/shm allowed in bytes", - }, - &cli.IntFlag{ - Sources: cli.EnvVars("WOODPECKER_LIMIT_CPU_QUOTA"), - Name: "limit-cpu-quota", - Usage: "impose a cpu quota", - }, - &cli.IntFlag{ - Sources: cli.EnvVars("WOODPECKER_LIMIT_CPU_SHARES"), - Name: "limit-cpu-shares", - Usage: "change the cpu shares", - }, - &cli.StringFlag{ - Sources: cli.EnvVars("WOODPECKER_LIMIT_CPU_SET"), - Name: "limit-cpu-set", - Usage: "set the cpus allowed to execute containers", - }, + // + // generic forge settings // &cli.StringFlag{ Name: "forge-url", diff --git a/cmd/server/setup.go b/cmd/server/setup.go index f221aebc7..b46f1bd5c 100644 --- a/cmd/server/setup.go +++ b/cmd/server/setup.go @@ -178,14 +178,6 @@ func setupEvilGlobals(ctx context.Context, c *cli.Command, s store.Store) error server.Config.Pipeline.DefaultTimeout = c.Int("default-pipeline-timeout") server.Config.Pipeline.MaxTimeout = c.Int("max-pipeline-timeout") - // limits - server.Config.Pipeline.Limits.MemSwapLimit = c.Int("limit-mem-swap") - server.Config.Pipeline.Limits.MemLimit = c.Int("limit-mem") - server.Config.Pipeline.Limits.ShmSize = c.Int("limit-shm-size") - server.Config.Pipeline.Limits.CPUQuota = c.Int("limit-cpu-quota") - server.Config.Pipeline.Limits.CPUShares = c.Int("limit-cpu-shares") - server.Config.Pipeline.Limits.CPUSet = c.String("limit-cpu-set") - // backend options for pipeline compiler server.Config.Pipeline.Proxy.No = c.String("backend-no-proxy") server.Config.Pipeline.Proxy.HTTP = c.String("backend-http-proxy") diff --git a/docs/docs/30-administration/10-server-config.md b/docs/docs/30-administration/10-server-config.md index 02fde63a0..4b3c7c3cb 100644 --- a/docs/docs/30-administration/10-server-config.md +++ b/docs/docs/30-administration/10-server-config.md @@ -476,44 +476,6 @@ Supported variables: --- -### `WOODPECKER_LIMIT_MEM_SWAP` - -> Default: `0` - -The maximum amount of memory a single pipeline container is allowed to swap to disk, configured in bytes. There is no limit if `0`. - -### `WOODPECKER_LIMIT_MEM` - -> Default: `0` - -The maximum amount of memory a single pipeline container can use, configured in bytes. There is no limit if `0`. - -### `WOODPECKER_LIMIT_SHM_SIZE` - -> Default: `0` - -The maximum amount of memory of `/dev/shm` allowed in bytes. There is no limit if `0`. - -### `WOODPECKER_LIMIT_CPU_QUOTA` - -> Default: `0` - -The number of microseconds per CPU period that the container is limited to before throttled. There is no limit if `0`. - -### `WOODPECKER_LIMIT_CPU_SHARES` - -> Default: `0` - -The relative weight vs. other containers. - -### `WOODPECKER_LIMIT_CPU_SET` - -> Default: empty - -Comma-separated list to limit the specific CPUs or cores a pipeline container can use. - -Example: `WOODPECKER_LIMIT_CPU_SET=1,2` - ### `WOODPECKER_CONFIG_SERVICE_ENDPOINT` > Default: empty diff --git a/docs/docs/30-administration/22-backends/10-docker.md b/docs/docs/30-administration/22-backends/10-docker.md index 7e3cfdf69..3e5db5b52 100644 --- a/docs/docs/30-administration/22-backends/10-docker.md +++ b/docs/docs/30-administration/22-backends/10-docker.md @@ -64,3 +64,41 @@ Enable IPv6 for the networks used by pipeline containers (steps). Make sure you List of default volumes separated by comma to be mounted to all pipeline containers (steps). For example to use custom CA certificates installed on host and host timezone use `/etc/ssl/certs:/etc/ssl/certs:ro,/etc/timezone:/etc/timezone`. + +### `WOODPECKER_BACKEND_DOCKER_LIMIT_MEM_SWAP` + +> Default: `0` + +The maximum amount of memory a single pipeline container is allowed to swap to disk, configured in bytes. There is no limit if `0`. + +### `WOODPECKER_BACKEND_DOCKER_LIMIT_MEM` + +> Default: `0` + +The maximum amount of memory a single pipeline container can use, configured in bytes. There is no limit if `0`. + +### `WOODPECKER_BACKEND_DOCKER_LIMIT_SHM_SIZE` + +> Default: `0` + +The maximum amount of memory of `/dev/shm` allowed in bytes. There is no limit if `0`. + +### `WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_QUOTA` + +> Default: `0` + +The number of microseconds per CPU period that the container is limited to before throttled. There is no limit if `0`. + +### `WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_SHARES` + +> Default: `0` + +The relative weight vs. other containers. + +### `WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_SET` + +> Default: empty + +Comma-separated list to limit the specific CPUs or cores a pipeline container can use. + +Example: `WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_SET=1,2` diff --git a/docs/docs/91-migrations.md b/docs/docs/91-migrations.md index 2d1870276..e0cc57c83 100644 --- a/docs/docs/91-migrations.md +++ b/docs/docs/91-migrations.md @@ -4,13 +4,14 @@ Some versions need some changes to the server configuration or the pipeline conf ## `next` -- Set `/woodpecker` as defautl workdir for the **woodpecker-cli** container - Removed built-in environment variables: - `CI_COMMIT_URL` use `CI_PIPELINE_FORGE_URL` - `CI_STEP_FINISHED` as empty during execution - `CI_PIPELINE_FINISHED` as empty during execution - `CI_PIPELINE_STATUS` was always `success` - `CI_STEP_STATUS` was always `success` +- Set `/woodpecker` as defautl workdir for the **woodpecker-cli** container +- Move docker resource limit settings from server into agent configuration - Rename server environment variable `WOODPECKER_ESCALATE` to `WOODPECKER_PLUGINS_PRIVILEGED` - All default privileged plugins (like `woodpeckerci/plugin-docker-buildx`) were removed. Please carefully [re-add those plugins](./30-administration/10-server-config.md#woodpecker_plugins_privileged) you trust and rely on. - `WOODPECKER_DEFAULT_CLONE_IMAGE` got depricated use `WOODPECKER_DEFAULT_CLONE_PLUGIN` diff --git a/pipeline/backend/docker/config.go b/pipeline/backend/docker/config.go new file mode 100644 index 000000000..299ce31b8 --- /dev/null +++ b/pipeline/backend/docker/config.go @@ -0,0 +1,71 @@ +// Copyright 2024 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package docker + +import ( + "fmt" + "strings" + + "github.com/rs/zerolog/log" + "github.com/urfave/cli/v3" +) + +type config struct { + enableIPv6 bool + network string + volumes []string + resourceLimit resourceLimit +} + +type resourceLimit struct { + MemSwapLimit int64 + MemLimit int64 + ShmSize int64 + CPUQuota int64 + CPUShares int64 + CPUSet string +} + +func configFromCli(c *cli.Command) (config, error) { + conf := config{ + enableIPv6: c.Bool("backend-docker-ipv6"), + network: c.String("backend-docker-network"), + resourceLimit: resourceLimit{ + MemSwapLimit: c.Int("backend-docker-limit-mem-swap"), + MemLimit: c.Int("backend-docker-limit-mem"), + ShmSize: c.Int("backend-docker-limit-shm-size"), + CPUQuota: c.Int("backend-docker-limit-cpu-quota"), + CPUShares: c.Int("backend-docker-limit-cpu-shares"), + CPUSet: c.String("backend-docker-limit-cpu-set"), + }, + } + + volumes := strings.Split(c.String("backend-docker-volumes"), ",") + conf.volumes = make([]string, 0, len(volumes)) + // Validate provided volume definitions + for _, v := range volumes { + if v == "" { + continue + } + parts, err := splitVolumeParts(v) + if err != nil { + log.Error().Err(err).Msgf("can not parse volume config") + return conf, fmt.Errorf("invalid volume '%s' provided in WOODPECKER_BACKEND_DOCKER_VOLUMES: %w", v, err) + } + conf.volumes = append(conf.volumes, strings.Join(parts, ":")) + } + + return conf, nil +} diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index d67672274..c08771f00 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -68,20 +68,20 @@ func toContainerName(step *types.Step) string { } // returns a container host configuration. -func toHostConfig(step *types.Step) *container.HostConfig { +func toHostConfig(step *types.Step, conf *config) *container.HostConfig { config := &container.HostConfig{ Resources: container.Resources{ - CPUQuota: step.CPUQuota, - CPUShares: step.CPUShares, - CpusetCpus: step.CPUSet, - Memory: step.MemLimit, - MemorySwap: step.MemSwapLimit, + CPUQuota: conf.resourceLimit.CPUQuota, + CPUShares: conf.resourceLimit.CPUShares, + CpusetCpus: conf.resourceLimit.CPUSet, + Memory: conf.resourceLimit.MemLimit, + MemorySwap: conf.resourceLimit.MemSwapLimit, }, + ShmSize: conf.resourceLimit.ShmSize, LogConfig: container.LogConfig{ Type: "json-file", }, Privileged: step.Privileged, - ShmSize: step.ShmSize, } if len(step.NetworkMode) != 0 { diff --git a/pipeline/backend/docker/convert_test.go b/pipeline/backend/docker/convert_test.go index 998c731b5..4d419b559 100644 --- a/pipeline/backend/docker/convert_test.go +++ b/pipeline/backend/docker/convert_test.go @@ -196,37 +196,44 @@ func TestToConfigSmall(t *testing.T) { } func TestToConfigFull(t *testing.T) { - engine := docker{info: system.Info{OSType: "linux/riscv64"}} + engine := docker{ + info: system.Info{OSType: "linux/riscv64"}, + config: config{ + enableIPv6: true, + resourceLimit: resourceLimit{ + MemSwapLimit: 12, + MemLimit: 13, + ShmSize: 14, + CPUQuota: 15, + CPUShares: 16, + }, + }, + } conf := engine.toConfig(&backend.Step{ - Name: "test", - UUID: "09238932", - Type: backend.StepTypeCommands, - Image: "golang:1.2.3", - Pull: true, - Detached: true, - Privileged: true, - WorkingDir: "/src/abc", - Environment: map[string]string{"TAGS": "sqlite"}, - Commands: []string{"go test", "go vet ./..."}, - ExtraHosts: []backend.HostAlias{{Name: "t", IP: "1.2.3.4"}}, - Volumes: []string{"/cache:/cache"}, - Tmpfs: []string{"/tmp"}, - Devices: []string{"/dev/sdc"}, - Networks: []backend.Conn{{Name: "extra-net", Aliases: []string{"extra.net"}}}, - DNS: []string{"9.9.9.9", "8.8.8.8"}, - DNSSearch: nil, - MemSwapLimit: 12, - MemLimit: 13, - ShmSize: 14, - CPUQuota: 15, - CPUShares: 16, - OnFailure: true, - OnSuccess: true, - Failure: "fail", - AuthConfig: backend.Auth{Username: "user", Password: "123456"}, - NetworkMode: "bridge", - Ports: []backend.Port{{Number: 21}, {Number: 22}}, + Name: "test", + UUID: "09238932", + Type: backend.StepTypeCommands, + Image: "golang:1.2.3", + Pull: true, + Detached: true, + Privileged: true, + WorkingDir: "/src/abc", + Environment: map[string]string{"TAGS": "sqlite"}, + Commands: []string{"go test", "go vet ./..."}, + ExtraHosts: []backend.HostAlias{{Name: "t", IP: "1.2.3.4"}}, + Volumes: []string{"/cache:/cache"}, + Tmpfs: []string{"/tmp"}, + Devices: []string{"/dev/sdc"}, + Networks: []backend.Conn{{Name: "extra-net", Aliases: []string{"extra.net"}}}, + DNS: []string{"9.9.9.9", "8.8.8.8"}, + DNSSearch: nil, + OnFailure: true, + OnSuccess: true, + Failure: "fail", + AuthConfig: backend.Auth{Username: "user", Password: "123456"}, + NetworkMode: "bridge", + Ports: []backend.Port{{Number: 21}, {Number: 22}}, }) assert.NotNil(t, conf) diff --git a/pipeline/backend/docker/docker.go b/pipeline/backend/docker/docker.go index bb5484c31..bc92b56a3 100644 --- a/pipeline/backend/docker/docker.go +++ b/pipeline/backend/docker/docker.go @@ -40,11 +40,9 @@ import ( ) type docker struct { - client client.APIClient - enableIPv6 bool - network string - volumes []string - info system.Info + client client.APIClient + info system.Info + config config } const ( @@ -132,22 +130,9 @@ func (e *docker) Load(ctx context.Context) (*backend.BackendInfo, error) { return nil, err } - e.enableIPv6 = c.Bool("backend-docker-ipv6") - e.network = c.String("backend-docker-network") - - volumes := strings.Split(c.String("backend-docker-volumes"), ",") - e.volumes = make([]string, 0, len(volumes)) - // Validate provided volume definitions - for _, v := range volumes { - if v == "" { - continue - } - parts, err := splitVolumeParts(v) - if err != nil { - log.Error().Err(err).Msgf("invalid volume '%s' provided in WOODPECKER_BACKEND_DOCKER_VOLUMES", v) - continue - } - e.volumes = append(e.volumes, strings.Join(parts, ":")) + e.config, err = configFromCli(c) + if err != nil { + return nil, err } return &backend.BackendInfo{ @@ -175,7 +160,7 @@ func (e *docker) SetupWorkflow(ctx context.Context, conf *backend.Config, taskUU for _, n := range conf.Networks { _, err := e.client.NetworkCreate(ctx, n.Name, network.CreateOptions{ Driver: networkDriver, - EnableIPv6: &e.enableIPv6, + EnableIPv6: &e.config.enableIPv6, }) if err != nil { return err @@ -188,7 +173,7 @@ func (e *docker) StartStep(ctx context.Context, step *backend.Step, taskUUID str log.Trace().Str("taskUUID", taskUUID).Msgf("start step %s", step.Name) config := e.toConfig(step) - hostConfig := toHostConfig(step) + hostConfig := toHostConfig(step, &e.config) containerName := toContainerName(step) // create pull options with encoded authorization credentials. @@ -217,7 +202,7 @@ func (e *docker) StartStep(ctx context.Context, step *backend.Step, taskUUID str } // add default volumes to the host configuration - hostConfig.Binds = utils.DeduplicateStrings(append(hostConfig.Binds, e.volumes...)) + hostConfig.Binds = utils.DeduplicateStrings(append(hostConfig.Binds, e.config.volumes...)) _, err := e.client.ContainerCreate(ctx, config, hostConfig, nil, nil, containerName) if client.IsErrNotFound(err) { @@ -251,8 +236,8 @@ func (e *docker) StartStep(ctx context.Context, step *backend.Step, taskUUID str } // join the container to an existing network - if e.network != "" { - err = e.client.NetworkConnect(ctx, e.network, containerName, &network.EndpointSettings{}) + if e.config.network != "" { + err = e.client.NetworkConnect(ctx, e.config.network, containerName, &network.EndpointSettings{}) if err != nil { return err } diff --git a/pipeline/backend/docker/flags.go b/pipeline/backend/docker/flags.go index 379cf5996..e1ac036e2 100644 --- a/pipeline/backend/docker/flags.go +++ b/pipeline/backend/docker/flags.go @@ -56,4 +56,37 @@ var Flags = []cli.Flag{ Name: "backend-docker-volumes", Usage: "backend docker volumes (comma separated)", }, + // + // resource limit parameters + // + &cli.IntFlag{ + Sources: cli.EnvVars("WOODPECKER_BACKEND_DOCKER_LIMIT_MEM_SWAP", "WOODPECKER_LIMIT_MEM_SWAP"), + Name: "backend-docker-limit-mem-swap", + Usage: "maximum memory used for swap in bytes", + }, + &cli.IntFlag{ + Sources: cli.EnvVars("WOODPECKER_BACKEND_DOCKER_LIMIT_MEM", "WOODPECKER_LIMIT_MEM"), + Name: "backend-docker-limit-mem", + Usage: "maximum memory allowed in bytes", + }, + &cli.IntFlag{ + Sources: cli.EnvVars("WOODPECKER_BACKEND_DOCKER_LIMIT_SHM_SIZE", "WOODPECKER_LIMIT_SHM_SIZE"), + Name: "backend-docker-limit-shm-size", + Usage: "docker /dev/shm allowed in bytes", + }, + &cli.IntFlag{ + Sources: cli.EnvVars("WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_QUOTA", "WOODPECKER_LIMIT_CPU_QUOTA"), + Name: "backend-docker-limit-cpu-quota", + Usage: "impose a cpu quota", + }, + &cli.IntFlag{ + Sources: cli.EnvVars("WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_SHARES", "WOODPECKER_LIMIT_CPU_SHARES"), + Name: "backend-docker-limit-cpu-shares", + Usage: "change the cpu shares", + }, + &cli.StringFlag{ + Sources: cli.EnvVars("WOODPECKER_BACKEND_DOCKER_LIMIT_CPU_SET", "WOODPECKER_LIMIT_CPU_SET"), + Name: "backend-docker-limit-cpu-set", + Usage: "set the cpus allowed to execute containers", + }, } diff --git a/pipeline/backend/types/step.go b/pipeline/backend/types/step.go index 6eb3471b9..122cc6359 100644 --- a/pipeline/backend/types/step.go +++ b/pipeline/backend/types/step.go @@ -34,12 +34,6 @@ type Step struct { Networks []Conn `json:"networks,omitempty"` DNS []string `json:"dns,omitempty"` DNSSearch []string `json:"dns_search,omitempty"` - MemSwapLimit int64 `json:"memswap_limit,omitempty"` - MemLimit int64 `json:"mem_limit,omitempty"` - ShmSize int64 `json:"shm_size,omitempty"` - CPUQuota int64 `json:"cpu_quota,omitempty"` - CPUShares int64 `json:"cpu_shares,omitempty"` - CPUSet string `json:"cpu_set,omitempty"` OnFailure bool `json:"on_failure,omitempty"` OnSuccess bool `json:"on_success,omitempty"` Failure string `json:"failure,omitempty"` diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index b9bdd09cc..68eb1495e 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -81,15 +81,6 @@ func (s *Secret) Match(event string) bool { return false } -type ResourceLimit struct { - MemSwapLimit int64 - MemLimit int64 - ShmSize int64 - CPUQuota int64 - CPUShares int64 - CPUSet string -} - // Compiler compiles the yaml. type Compiler struct { local bool @@ -104,7 +95,6 @@ type Compiler struct { metadata metadata.Metadata registries []Registry secrets map[string]Secret - reslimit ResourceLimit defaultClonePlugin string trustedClonePlugins []string trustedPipeline bool diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index bab841d8a..792e35505 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/pipeline/frontend/yaml/compiler/convert.go @@ -151,31 +151,6 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe } } - memSwapLimit := int64(container.MemSwapLimit) - if c.reslimit.MemSwapLimit != 0 { - memSwapLimit = c.reslimit.MemSwapLimit - } - memLimit := int64(container.MemLimit) - if c.reslimit.MemLimit != 0 { - memLimit = c.reslimit.MemLimit - } - shmSize := int64(container.ShmSize) - if c.reslimit.ShmSize != 0 { - shmSize = c.reslimit.ShmSize - } - cpuQuota := int64(container.CPUQuota) - if c.reslimit.CPUQuota != 0 { - cpuQuota = c.reslimit.CPUQuota - } - cpuShares := int64(container.CPUShares) - if c.reslimit.CPUShares != 0 { - cpuShares = c.reslimit.CPUShares - } - cpuSet := container.CPUSet - if c.reslimit.CPUSet != "" { - cpuSet = c.reslimit.CPUSet - } - var ports []backend_types.Port for _, portDef := range container.Ports { port, err := convertPort(portDef) @@ -214,12 +189,6 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe Networks: networks, DNS: container.DNS, DNSSearch: container.DNSSearch, - MemSwapLimit: memSwapLimit, - MemLimit: memLimit, - ShmSize: shmSize, - CPUQuota: cpuQuota, - CPUShares: cpuShares, - CPUSet: cpuSet, AuthConfig: authConfig, OnSuccess: onSuccess, OnFailure: onFailure, diff --git a/pipeline/frontend/yaml/compiler/option.go b/pipeline/frontend/yaml/compiler/option.go index c25b555ec..96c18f8c2 100644 --- a/pipeline/frontend/yaml/compiler/option.go +++ b/pipeline/frontend/yaml/compiler/option.go @@ -157,21 +157,6 @@ func WithNetworks(networks ...string) Option { } } -// WithResourceLimit configures the compiler with default resource limits that -// are applied each container in the pipeline. -func WithResourceLimit(swap, mem, shmSize, cpuQuota, cpuShares int64, cpuSet string) Option { - return func(compiler *Compiler) { - compiler.reslimit = ResourceLimit{ - MemSwapLimit: swap, - MemLimit: mem, - ShmSize: shmSize, - CPUQuota: cpuQuota, - CPUShares: cpuShares, - CPUSet: cpuSet, - } - } -} - func WithDefaultClonePlugin(cloneImage string) Option { return func(compiler *Compiler) { compiler.defaultClonePlugin = cloneImage diff --git a/pipeline/frontend/yaml/compiler/option_test.go b/pipeline/frontend/yaml/compiler/option_test.go index a865ef254..b4c179d20 100644 --- a/pipeline/frontend/yaml/compiler/option_test.go +++ b/pipeline/frontend/yaml/compiler/option_test.go @@ -67,25 +67,6 @@ func TestWithNetworks(t *testing.T) { assert.Equal(t, "overlay_bar", compiler.networks[1]) } -func TestWithResourceLimit(t *testing.T) { - compiler := New( - WithResourceLimit( - 1, - 2, - 3, - 4, - 5, - "0,2-5", - ), - ) - assert.EqualValues(t, 1, compiler.reslimit.MemSwapLimit) - assert.EqualValues(t, 2, compiler.reslimit.MemLimit) - assert.EqualValues(t, 3, compiler.reslimit.ShmSize) - assert.EqualValues(t, 4, compiler.reslimit.CPUQuota) - assert.EqualValues(t, 5, compiler.reslimit.CPUShares) - assert.Equal(t, "0,2-5", compiler.reslimit.CPUSet) -} - func TestWithPrefix(t *testing.T) { assert.Equal(t, "someprefix_", New(WithPrefix("someprefix_")).prefix) } diff --git a/pipeline/frontend/yaml/linter/linter.go b/pipeline/frontend/yaml/linter/linter.go index 6b88d8e98..f717736ad 100644 --- a/pipeline/frontend/yaml/linter/linter.go +++ b/pipeline/frontend/yaml/linter/linter.go @@ -207,9 +207,6 @@ func (l *Linter) lintTrusted(config *WorkflowConfig, c *types.Container, area st if c.Privileged { errors = append(errors, "Insufficient privileges to use privileged mode") } - if c.ShmSize != 0 { - errors = append(errors, "Insufficient privileges to override shm_size") - } if len(c.DNS) != 0 { errors = append(errors, "Insufficient privileges to use custom dns") } diff --git a/pipeline/frontend/yaml/linter/linter_test.go b/pipeline/frontend/yaml/linter/linter_test.go index 3c0e73417..85815799e 100644 --- a/pipeline/frontend/yaml/linter/linter_test.go +++ b/pipeline/frontend/yaml/linter/linter_test.go @@ -120,10 +120,6 @@ func TestLintErrors(t *testing.T) { from: "steps: { build: { image: golang, privileged: true } }", want: "Insufficient privileges to use privileged mode", }, - { - from: "steps: { build: { image: golang, shm_size: 10gb } }", - want: "Insufficient privileges to override shm_size", - }, { from: "steps: { build: { image: golang, dns: [ 8.8.8.8 ] } }", want: "Insufficient privileges to use custom dns", diff --git a/pipeline/frontend/yaml/types/container.go b/pipeline/frontend/yaml/types/container.go index 9ebc8f12a..ff5d72438 100644 --- a/pipeline/frontend/yaml/types/container.go +++ b/pipeline/frontend/yaml/types/container.go @@ -54,18 +54,12 @@ type ( Privileged bool `yaml:"privileged,omitempty"` // Undocumented - CPUQuota base.StringOrInt `yaml:"cpu_quota,omitempty"` - CPUSet string `yaml:"cpuset,omitempty"` - CPUShares base.StringOrInt `yaml:"cpu_shares,omitempty"` - Devices []string `yaml:"devices,omitempty"` - DNSSearch base.StringOrSlice `yaml:"dns_search,omitempty"` - DNS base.StringOrSlice `yaml:"dns,omitempty"` - ExtraHosts []string `yaml:"extra_hosts,omitempty"` - MemLimit base.MemStringOrInt `yaml:"mem_limit,omitempty"` - MemSwapLimit base.MemStringOrInt `yaml:"memswap_limit,omitempty"` - NetworkMode string `yaml:"network_mode,omitempty"` - ShmSize base.MemStringOrInt `yaml:"shm_size,omitempty"` - Tmpfs []string `yaml:"tmpfs,omitempty"` + Devices []string `yaml:"devices,omitempty"` + DNSSearch base.StringOrSlice `yaml:"dns_search,omitempty"` + DNS base.StringOrSlice `yaml:"dns,omitempty"` + ExtraHosts []string `yaml:"extra_hosts,omitempty"` + NetworkMode string `yaml:"network_mode,omitempty"` + Tmpfs []string `yaml:"tmpfs,omitempty"` } ) diff --git a/pipeline/frontend/yaml/types/container_test.go b/pipeline/frontend/yaml/types/container_test.go index 10de961c7..eaa5ca918 100644 --- a/pipeline/frontend/yaml/types/container_test.go +++ b/pipeline/frontend/yaml/types/container_test.go @@ -30,9 +30,6 @@ image: golang:latest commands: - go build - go test -cpu_quota: 11 -cpuset: 1,2 -cpu_shares: 99 detach: true devices: - /dev/ttyUSB0:/dev/ttyUSB0 @@ -54,9 +51,6 @@ networks: - other-network pull: true privileged: true -shm_size: 1kb -mem_limit: 1kb -memswap_limit: 1kb volumes: - /var/lib/mysql - /opt/data:/var/lib/mysql @@ -78,27 +72,21 @@ ports: func TestUnmarshalContainer(t *testing.T) { want := Container{ - Commands: base.StringOrSlice{"go build", "go test"}, - CPUQuota: base.StringOrInt(11), - CPUSet: "1,2", - CPUShares: base.StringOrInt(99), - Detached: true, - Devices: []string{"/dev/ttyUSB0:/dev/ttyUSB0"}, - Directory: "example/", - DNS: base.StringOrSlice{"8.8.8.8"}, - DNSSearch: base.StringOrSlice{"example.com"}, - Entrypoint: []string{"/bin/sh", "-c"}, - Environment: map[string]any{"RACK_ENV": "development", "SHOW": true}, - ExtraHosts: []string{"somehost:162.242.195.82", "otherhost:50.31.209.229", "ipv6:2001:db8::10"}, - Image: "golang:latest", - MemLimit: base.MemStringOrInt(1024), - MemSwapLimit: base.MemStringOrInt(1024), - Name: "my-build-container", - NetworkMode: "bridge", - Pull: true, - Privileged: true, - ShmSize: base.MemStringOrInt(1024), - Tmpfs: base.StringOrSlice{"/var/lib/test"}, + Commands: base.StringOrSlice{"go build", "go test"}, + Detached: true, + Devices: []string{"/dev/ttyUSB0:/dev/ttyUSB0"}, + Directory: "example/", + DNS: base.StringOrSlice{"8.8.8.8"}, + DNSSearch: base.StringOrSlice{"example.com"}, + Entrypoint: []string{"/bin/sh", "-c"}, + Environment: map[string]any{"RACK_ENV": "development", "SHOW": true}, + ExtraHosts: []string{"somehost:162.242.195.82", "otherhost:50.31.209.229", "ipv6:2001:db8::10"}, + Image: "golang:latest", + Name: "my-build-container", + NetworkMode: "bridge", + Pull: true, + Privileged: true, + Tmpfs: base.StringOrSlice{"/var/lib/test"}, Volumes: Volumes{ Volumes: []*Volume{ {Source: "", Destination: "/var/lib/mysql"}, diff --git a/server/config.go b/server/config.go index 96e9d4326..fb6c42292 100644 --- a/server/config.go +++ b/server/config.go @@ -66,7 +66,6 @@ var Config = struct { DefaultCancelPreviousPipelineEvents []model.WebhookEvent DefaultClonePlugin string TrustedClonePlugins []string - Limits model.ResourceLimit Volumes []string Networks []string PrivilegedPlugins []string diff --git a/server/model/resource_limit.go b/server/model/resource_limit.go deleted file mode 100644 index 90a302993..000000000 --- a/server/model/resource_limit.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2018 Drone.IO Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package model - -// ResourceLimit is the resource limit to set on pipeline steps. -type ResourceLimit struct { - MemSwapLimit int64 - MemLimit int64 - ShmSize int64 - CPUQuota int64 - CPUShares int64 - CPUSet string -} diff --git a/server/pipeline/stepbuilder/stepBuilder.go b/server/pipeline/stepbuilder/stepBuilder.go index a59f4b0ba..8cf437606 100644 --- a/server/pipeline/stepbuilder/stepBuilder.go +++ b/server/pipeline/stepbuilder/stepBuilder.go @@ -270,7 +270,6 @@ func (b *StepBuilder) toInternalRepresentation(parsed *yaml_types.Workflow, envi compiler.WithEnviron(b.Envs), // TODO: server deps should be moved into StepBuilder fields and set on StepBuilder creation compiler.WithEscalated(server.Config.Pipeline.PrivilegedPlugins...), - compiler.WithResourceLimit(server.Config.Pipeline.Limits.MemSwapLimit, server.Config.Pipeline.Limits.MemLimit, server.Config.Pipeline.Limits.ShmSize, server.Config.Pipeline.Limits.CPUQuota, server.Config.Pipeline.Limits.CPUShares, server.Config.Pipeline.Limits.CPUSet), compiler.WithVolumes(server.Config.Pipeline.Volumes...), compiler.WithNetworks(server.Config.Pipeline.Networks...), compiler.WithLocal(false),