Fix startup variables not being properly updated on server reboot; closes pterodactyl/panel#2255

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

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

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.

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

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

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

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

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

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

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

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