From 7d8710824cba0c5e90a049a5af22ca1bb7c1e693 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 27 Aug 2020 21:08:33 -0700 Subject: [PATCH] Fix startup variables not being properly updated on server reboot; closes pterodactyl/panel#2255 --- environment/config.go | 36 +++++++++++++++------ environment/docker/environment.go | 22 +++++++++---- environment/environment.go | 3 ++ router/router_server.go | 4 ++- server/configuration.go | 14 +++++++++ server/filesystem.go | 2 +- server/filesystem_unarchive.go | 4 +-- server/loader.go | 10 ++++-- server/power.go | 15 ++++++--- server/server.go | 8 ++--- server/update.go | 52 ++++++++++++++++++++----------- 11 files changed, 119 insertions(+), 51 deletions(-) diff --git a/environment/config.go b/environment/config.go index 06c7197..83a318b 100644 --- a/environment/config.go +++ b/environment/config.go @@ -4,7 +4,7 @@ import ( "sync" ) -type configurationSettings struct { +type Settings struct { Mounts []Mount Allocations Allocations Limits Limits @@ -16,20 +16,35 @@ type Configuration struct { mu sync.RWMutex environmentVariables []string - settings configurationSettings + settings Settings } -func NewConfiguration(m []Mount, a Allocations, l Limits, envVars []string) *Configuration { +// Returns a new environment configuration with the given settings and environment variables +// defined within it. +func NewConfiguration(s Settings, envVars []string) *Configuration { return &Configuration{ environmentVariables: envVars, - settings: configurationSettings{ - Mounts: m, - Allocations: a, - Limits: l, - }, + settings: s, } } +// Updates the settings struct for this environment on the fly. This allows modified servers to +// automatically push those changes to the environment. +func (c *Configuration) SetSettings(s Settings) { + c.mu.Lock() + c.settings = s + c.mu.Unlock() +} + +// Updates the environment variables associated with this environment by replacing the entire +// array of them with a new one. +func (c *Configuration) SetEnvironmentVariables(ev []string) { + c.mu.Lock() + c.environmentVariables = ev + c.mu.Unlock() +} + +// Returns the limits assigned to this environment. func (c *Configuration) Limits() Limits { c.mu.RLock() defer c.mu.RUnlock() @@ -37,6 +52,7 @@ func (c *Configuration) Limits() Limits { return c.settings.Limits } +// Rturns the allocations associated with this environment. func (c *Configuration) Allocations() Allocations { c.mu.RLock() defer c.mu.RUnlock() @@ -44,6 +60,7 @@ func (c *Configuration) Allocations() Allocations { return c.settings.Allocations } +// Returns all of the mounts associated with this environment. func (c *Configuration) Mounts() []Mount { c.mu.RLock() defer c.mu.RUnlock() @@ -51,9 +68,10 @@ func (c *Configuration) Mounts() []Mount { return c.settings.Mounts } +// Returns the environment variables associated with this instance. func (c *Configuration) EnvironmentVariables() []string { c.mu.RLock() defer c.mu.RUnlock() return c.environmentVariables -} \ No newline at end of file +} diff --git a/environment/docker/environment.go b/environment/docker/environment.go index df3180d..cc155fb 100644 --- a/environment/docker/environment.go +++ b/environment/docker/environment.go @@ -70,12 +70,6 @@ func New(id string, m *Metadata, c *environment.Configuration) (*Environment, er return e, nil } -func (e *Environment) SetStopConfiguration(c *api.ProcessStopConfiguration) { - e.mu.Lock() - e.meta.Stop = c - e.mu.Unlock() -} - func (e *Environment) Type() string { return "docker" } @@ -166,3 +160,19 @@ func (e *Environment) ExitState() (uint32, bool, error) { return uint32(c.State.ExitCode), c.State.OOMKilled, nil } + +// Returns the environment configuration allowing a process to make modifications of the +// environment on the fly. +func (e *Environment) Config() *environment.Configuration { + e.mu.RLock() + defer e.mu.RUnlock() + + return e.Configuration +} + +// Sets the stop configuration for the environment. +func (e *Environment) SetStopConfiguration(c *api.ProcessStopConfiguration) { + e.mu.Lock() + e.meta.Stop = c + e.mu.Unlock() +} \ No newline at end of file diff --git a/environment/environment.go b/environment/environment.go index c80d5d4..82e4db1 100644 --- a/environment/environment.go +++ b/environment/environment.go @@ -24,6 +24,9 @@ type ProcessEnvironment interface { // Returns the name of the environment. Type() string + // Returns the environment configuration to the caller. + Config() *Configuration + // Returns an event emitter instance that can be hooked into to listen for different // events that are fired by the environment. This should not allow someone to publish // events, only subscribe to them. diff --git a/router/router_server.go b/router/router_server.go index 814d3f9..ca77e43 100644 --- a/router/router_server.go +++ b/router/router_server.go @@ -134,11 +134,13 @@ func patchServer(c *gin.Context) { buf := bytes.Buffer{} buf.ReadFrom(c.Request.Body) - if err := s.UpdateDataStructure(buf.Bytes(), true); err != nil { + if err := s.UpdateDataStructure(buf.Bytes()); err != nil { TrackedServerError(err, s).AbortWithServerError(c) return } + s.SyncWithEnvironment() + c.Status(http.StatusNoContent) } diff --git a/server/configuration.go b/server/configuration.go index a40ed9c..e20c55a 100644 --- a/server/configuration.go +++ b/server/configuration.go @@ -43,6 +43,20 @@ func (s *Server) Config() *Configuration { return &s.cfg } +func (s *Server) DiskSpace() int64 { + s.cfg.mu.RLock() + defer s.cfg.mu.RUnlock() + + return s.cfg.Build.DiskSpace +} + +func (s *Server) MemoryLimit() int64 { + s.cfg.mu.RLock() + defer s.cfg.mu.RUnlock() + + return s.cfg.Build.MemoryLimit +} + func (c *Configuration) GetUuid() string { c.mu.RLock() defer c.mu.RUnlock() diff --git a/server/filesystem.go b/server/filesystem.go index 6effbc3..911b702 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -221,7 +221,7 @@ func (fs *Filesystem) HasSpaceAvailable() bool { // been allocated. fs.Server.Proc().SetDisk(size) - space := fs.Server.Build().DiskSpace + space := fs.Server.DiskSpace() // If space is -1 or 0 just return true, means they're allowed unlimited. // // Technically we could skip disk space calculation because we don't need to check if the server exceeds it's limit diff --git a/server/filesystem_unarchive.go b/server/filesystem_unarchive.go index c3b648f..c5cd06f 100644 --- a/server/filesystem_unarchive.go +++ b/server/filesystem_unarchive.go @@ -19,7 +19,7 @@ import ( func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (bool, error) { // Don't waste time trying to determine this if we know the server will have the space for // it since there is no limit. - if fs.Server.Build().DiskSpace <= 0 { + if fs.Server.DiskSpace() <= 0 { return true, nil } @@ -58,7 +58,7 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b wg.Wait() - return ((dirSize + size) / 1000.0 / 1000.0) <= fs.Server.Build().DiskSpace, cErr + return ((dirSize + size) / 1000.0 / 1000.0) <= fs.Server.DiskSpace(), cErr } // Decompress a file in a given directory by using the archiver tool to infer the file diff --git a/server/loader.go b/server/loader.go index 36c92a7..8250655 100644 --- a/server/loader.go +++ b/server/loader.go @@ -93,7 +93,7 @@ func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { } s.cfg = cfg - if err := s.UpdateDataStructure(data.Settings, false); err != nil { + if err := s.UpdateDataStructure(data.Settings); err != nil { return nil, err } @@ -103,7 +103,13 @@ func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { // Right now we only support a Docker based environment, so I'm going to hard code // this logic in. When we're ready to support other environment we'll need to make // some modifications here obviously. - envCfg := environment.NewConfiguration(s.Mounts(), s.cfg.Allocations, s.cfg.Build, s.GetEnvironmentVariables()) + settings := environment.Settings{ + Mounts: s.Mounts(), + Allocations: s.cfg.Allocations, + Limits: s.cfg.Build, + } + + envCfg := environment.NewConfiguration(settings, s.GetEnvironmentVariables()) meta := docker.Metadata{ Image: s.Config().Container.Image, } diff --git a/server/power.go b/server/power.go index 9557f04..a7017c6 100644 --- a/server/power.go +++ b/server/power.go @@ -126,16 +126,21 @@ func (s *Server) HandlePowerAction(action PowerAction, waitSeconds ...int) error // Execute a few functions before actually calling the environment start commands. This ensures // that everything is ready to go for environment booting, and that the server can even be started. func (s *Server) onBeforeStart() error { - // Disallow start & restart if the server is suspended. - if s.IsSuspended() { - return new(suspendedError) - } - s.Log().Info("syncing server configuration with panel") if err := s.Sync(); err != nil { return errors.Wrap(err, "unable to sync server data from Panel instance") } + // Disallow start & restart if the server is suspended. Do this check after performing a sync + // action with the Panel to ensure that we have the most up-to-date information for that server. + if s.IsSuspended() { + return new(suspendedError) + } + + // Ensure we sync the server information with the environment so that any new environment variables + // and process resource limits are correctly applied. + s.SyncWithEnvironment() + if !s.Filesystem.HasSpaceAvailable() { return errors.New("cannot start server, not enough disk space available") } diff --git a/server/server.go b/server/server.go index b55399d..46fc2db 100644 --- a/server/server.go +++ b/server/server.go @@ -77,7 +77,7 @@ func (s *Server) GetEnvironmentVariables() []string { var out = []string{ fmt.Sprintf("TZ=%s", zone), fmt.Sprintf("STARTUP=%s", s.Config().Invocation), - fmt.Sprintf("SERVER_MEMORY=%d", s.Build().MemoryLimit), + fmt.Sprintf("SERVER_MEMORY=%d", s.MemoryLimit()), fmt.Sprintf("SERVER_IP=%s", s.Config().Allocations.DefaultMapping.Ip), fmt.Sprintf("SERVER_PORT=%d", s.Config().Allocations.DefaultMapping.Port), } @@ -125,7 +125,7 @@ func (s *Server) Sync() error { func (s *Server) SyncWithConfiguration(cfg *api.ServerConfigurationResponse) error { // Update the data structure and persist it to the disk. - if err := s.UpdateDataStructure(cfg.Settings, false); err != nil { + if err := s.UpdateDataStructure(cfg.Settings); err != nil { return errors.WithStack(err) } @@ -177,10 +177,6 @@ func (s *Server) IsSuspended() bool { return s.Config().Suspended } -func (s *Server) Build() *environment.Limits { - return &s.Config().Build -} - func (s *Server) ProcessConfiguration() *api.ProcessConfiguration { s.RLock() defer s.RUnlock() diff --git a/server/update.go b/server/update.go index c9a6d8c..aed61dd 100644 --- a/server/update.go +++ b/server/update.go @@ -15,7 +15,7 @@ import ( // The server will be marked as requiring a rebuild on the next boot sequence, // it is up to the specific environment to determine what needs to happen when // that is the case. -func (s *Server) UpdateDataStructure(data []byte, background bool) error { +func (s *Server) UpdateDataStructure(data []byte) error { src := new(Configuration) if err := json.Unmarshal(data, src); err != nil { return errors.WithStack(err) @@ -97,36 +97,50 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { // Update the configuration once we have a lock on the configuration object. s.cfg = c - if background { - go s.runBackgroundActions() - } - return nil } -// Runs through different actions once a server's configuration has been persisted -// to the disk. This function does not return anything as any failures should be logged -// but have no effect on actually updating the server itself. +// Updates the environment for the server to match any of the changed data. This pushes new settings and +// environment variables to the environment. In addition, the in-situ update method is called on the +// environment which will allow environments that make use of it (such as Docker) to immediately apply +// some settings without having to wait on a server to restart. // -// These tasks run in independent threads where relevant to speed up any updates -// that need to happen. -func (s *Server) runBackgroundActions() { - // Check if the s is now suspended, and if so and the process is not terminated - // yet, do it immediately. - if s.IsSuspended() && s.GetState() != environment.ProcessOfflineState { - s.Log().Info("server suspended with running process state, terminating now") +// This functionality allows a server's resources limits to be modified on the fly and have them apply +// right away allowing for dynamic resource allocation and responses to abusive server processes. +func (s *Server) SyncWithEnvironment() { + s.Log().Debug("syncing server settings with environment") - if err := s.Environment.WaitForStop(10, true); err != nil { - s.Log().WithField("error", err).Warn("failed to terminate server environment after suspension") - } - } + // Update the environment settings using the new information from this server. + s.Environment.Config().SetSettings(environment.Settings{ + Mounts: s.Mounts(), + Allocations: s.Config().Allocations, + Limits: s.Config().Build, + }) + + // If build limits are changed, environment variables also change. Plus, any modifications to + // the startup command also need to be properly propagated to this environment. + // + // @see https://github.com/pterodactyl/panel/issues/2255 + s.Environment.Config().SetEnvironmentVariables(s.GetEnvironmentVariables()) if !s.IsSuspended() { // Update the environment in place, allowing memory and CPU usage to be adjusted // on the fly without the user needing to reboot (theoretically). s.Log().Info("performing server limit modification on-the-fly") if err := s.Environment.InSituUpdate(); err != nil { + // This is not a failure, the process is still running fine and will fix itself on the + // next boot, or fail out entirely in a more logical position. s.Log().WithField("error", err).Warn("failed to perform on-the-fly update of the server environment") } + } else { + // Checks if the server is now in a suspended state. If so and a server process is currently running it + // will be gracefully stopped (and terminated if it refuses to stop). + s.Log().Info("server suspended with running process state, terminating now") + + go func (s *Server) { + if err := s.Environment.WaitForStop(60, true); err != nil { + s.Log().WithField("error", err).Warn("failed to terminate server environment after suspension") + } + }(s) } }