From 8336f6ff2986821ff34b55217047d7de8439180b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 20 Jun 2021 17:21:51 -0700 Subject: [PATCH] Apply container limits to install containers, defaulting to minimums if the server's resources are set too low --- config/config_docker.go | 9 +++++++ environment/docker/container.go | 4 ++-- environment/settings.go | 42 +++++++++++++++++++++++---------- server/install.go | 38 +++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/config/config_docker.go b/config/config_docker.go index 11d4f59..aa8c569 100644 --- a/config/config_docker.go +++ b/config/config_docker.go @@ -61,6 +61,15 @@ type DockerConfiguration struct { // malicious process could create enough processes to cause the host node to run out of // available pids and crash. ContainerPidLimit int64 `default:"256" json:"container_pid_limit" yaml:"container_pid_limit"` + + // InstallLimits defines the limits on the installer containers that prevents a server's + // installation process from unintentionally consuming more resources than expected. This + // is used in conjunction with the server's defined limits. Whichever value is higher will + // take precedence in the install containers. + InstallerLimits struct { + Memory int64 `default:"1024" json:"memory" yaml:"memory"` + Cpu int64 `default:"100" json:"cpu" yaml:"cpu"` + } `json:"installer_limits" yaml:"installer_limits"` } // RegistryConfiguration defines the authentication credentials for a given diff --git a/environment/docker/container.go b/environment/docker/container.go index df66ce4..05dbf30 100644 --- a/environment/docker/container.go +++ b/environment/docker/container.go @@ -132,7 +132,7 @@ func (e *Environment) InSituUpdate() error { // // @see https://github.com/moby/moby/issues/41946 if _, err := e.client.ContainerUpdate(ctx, e.Id, container.UpdateConfig{ - Resources: e.resources(), + Resources: e.Configuration.Limits().AsContainerResources(), }); err != nil { return errors.Wrap(err, "environment/docker: could not update container") } @@ -203,7 +203,7 @@ func (e *Environment) Create() error { // Define resource limits for the container based on the data passed through // from the Panel. - Resources: e.resources(), + Resources: e.Configuration.Limits().AsContainerResources(), DNS: config.Get().Docker.Network.Dns, diff --git a/environment/settings.go b/environment/settings.go index e3e40dd..8ebbfdf 100644 --- a/environment/settings.go +++ b/environment/settings.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/apex/log" + "github.com/docker/docker/api/types/container" "github.com/pterodactyl/wings/config" ) @@ -60,49 +61,66 @@ type Limits struct { // ConvertedCpuLimit converts the CPU limit for a server build into a number // that can be better understood by the Docker environment. If there is no limit // set, return -1 which will indicate to Docker that it has unlimited CPU quota. -func (r *Limits) ConvertedCpuLimit() int64 { - if r.CpuLimit == 0 { +func (l Limits) ConvertedCpuLimit() int64 { + if l.CpuLimit == 0 { return -1 } - return r.CpuLimit * 1000 + return l.CpuLimit * 1000 } // MemoryOverheadMultiplier sets the hard limit for memory usage to be 5% more // than the amount of memory assigned to the server. If the memory limit for the // server is < 4G, use 10%, if less than 2G use 15%. This avoids unexpected // crashes from processes like Java which run over the limit. -func (r *Limits) MemoryOverheadMultiplier() float64 { - if r.MemoryLimit <= 2048 { +func (l Limits) MemoryOverheadMultiplier() float64 { + if l.MemoryLimit <= 2048 { return 1.15 - } else if r.MemoryLimit <= 4096 { + } else if l.MemoryLimit <= 4096 { return 1.10 } return 1.05 } -func (r *Limits) BoundedMemoryLimit() int64 { - return int64(math.Round(float64(r.MemoryLimit) * r.MemoryOverheadMultiplier() * 1_000_000)) +func (l Limits) BoundedMemoryLimit() int64 { + return int64(math.Round(float64(l.MemoryLimit) * l.MemoryOverheadMultiplier() * 1_000_000)) } // ConvertedSwap returns the amount of swap available as a total in bytes. This // is returned as the amount of memory available to the server initially, PLUS // the amount of additional swap to include which is the format used by Docker. -func (r *Limits) ConvertedSwap() int64 { - if r.Swap < 0 { +func (l Limits) ConvertedSwap() int64 { + if l.Swap < 0 { return -1 } - return (r.Swap * 1_000_000) + r.BoundedMemoryLimit() + return (l.Swap * 1_000_000) + l.BoundedMemoryLimit() } // ProcessLimit returns the process limit for a container. This is currently // defined at a system level and not on a per-server basis. -func (r *Limits) ProcessLimit() int64 { +func (l Limits) ProcessLimit() int64 { return config.Get().Docker.ContainerPidLimit } +func (l Limits) AsContainerResources() container.Resources { + pids := l.ProcessLimit() + + return container.Resources{ + Memory: l.BoundedMemoryLimit(), + MemoryReservation: l.MemoryLimit * 1_000_000, + MemorySwap: l.ConvertedSwap(), + CPUQuota: l.ConvertedCpuLimit(), + CPUPeriod: 100_000, + CPUShares: 1024, + BlkioWeight: l.IoWeight, + OomKillDisable: &l.OOMDisabled, + CpusetCpus: l.Threads, + PidsLimit: &pids, + } +} + type Variables map[string]interface{} // Get is an ugly hacky function to handle environment variables that get passed diff --git a/server/install.go b/server/install.go index fc080be..3bfaffc 100644 --- a/server/install.go +++ b/server/install.go @@ -434,6 +434,7 @@ func (ip *InstallationProcess) Execute() (string, error) { ReadOnly: false, }, }, + Resources: ip.resourceLimits(), Tmpfs: map[string]string{ "/tmp": "rw,exec,nosuid,size=" + tmpfsSize + "M", }, @@ -530,6 +531,43 @@ func (ip *InstallationProcess) StreamOutput(ctx context.Context, id string) erro return nil } +// resourceLimits returns the install container specific resource limits. This +// looks at the globally defined install container limits and attempts to use +// the higher of the two (defined limits & server limits). This allows for servers +// with super low limits (e.g. Discord bots with 128Mb of memory) to perform more +// intensive installation processes if needed. +// +// This also avoids a server with limits such as 4GB of memory from accidentally +// consuming 2-5x the defined limits during the install process and causing +// system instability. +func (ip *InstallationProcess) resourceLimits() container.Resources { + limits := config.Get().Docker.InstallerLimits + + // Create a copy of the configuration so we're not accidentally making changes + // to the underlying server build data. + c := *ip.Server.Config() + cfg := c.Build + if cfg.MemoryLimit < limits.Memory { + cfg.MemoryLimit = limits.Memory + } + // Only apply the CPU limit if neither one is currently set to unlimited. If the + // installer CPU limit is unlimited don't even waste time with the logic, just + // set the config to unlimited for this. + if limits.Cpu == 0 { + cfg.CpuLimit = 0 + } else if cfg.CpuLimit != 0 && cfg.CpuLimit < limits.Cpu { + cfg.CpuLimit = limits.Cpu + } + + resources := cfg.AsContainerResources() + // Explicitly remove the PID limits for the installation container. These scripts are + // defined at an administrative level and users can't manually execute things like a + // fork bomb during this process. + resources.PidsLimit = nil + + return resources +} + // SyncInstallState makes a HTTP request to the Panel instance notifying it that // the server has completed the installation process, and what the state of the // server is. A boolean value of "true" means everything was successful, "false"