From 744c9a485dcf9abffdda182d426072a346008e6c Mon Sep 17 00:00:00 2001
From: Mahmood Ali <mahmood@hashicorp.com>
Date: Sun, 12 Jan 2020 15:50:18 -0500
Subject: [PATCH] scheduler: allow configuring default preemption for system
 scheduler

Some operators want a greater control over when preemption is enabled,
especially during an upgrade to limit potential side-effects.
---
 command/agent/agent.go      |  7 +++++++
 command/agent/agent_test.go | 29 +++++++++++++++++++++++++++++
 command/agent/config.go     |  3 +++
 nomad/config.go             |  8 ++++++--
 nomad/leader.go             | 16 +++++++++-------
 5 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/command/agent/agent.go b/command/agent/agent.go
index f8d85f5623a..4d2f7fb903e 100644
--- a/command/agent/agent.go
+++ b/command/agent/agent.go
@@ -322,6 +322,13 @@ func convertServerConfig(agentConfig *Config) (*nomad.Config, error) {
 		return nil, fmt.Errorf("server_service_name must be set when auto_advertise is enabled")
 	}
 
+	// handle system scheduler preemption default
+	if agentConfig.Server.SystemSchedulerPreemptionEnabledDefault != nil {
+		conf.SystemSchedulerPreemptionEnabledDefault = *agentConfig.Server.SystemSchedulerPreemptionEnabledDefault
+	} else {
+		conf.SystemSchedulerPreemptionEnabledDefault = true
+	}
+
 	// Add the Consul and Vault configs
 	conf.ConsulConfig = agentConfig.Consul
 	conf.VaultConfig = agentConfig.Vault
diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go
index 0cdffa24a0f..ffabf8bd008 100644
--- a/command/agent/agent_test.go
+++ b/command/agent/agent_test.go
@@ -1,6 +1,7 @@
 package agent
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -274,6 +275,34 @@ func TestAgent_ServerConfig(t *testing.T) {
 	}
 }
 
+func TestAgent_ServerConfig_SchedulerFlags(t *testing.T) {
+	cases := []struct {
+		input    *bool
+		expected bool
+	}{
+		{nil, true},
+		{helper.BoolToPtr(false), false},
+		{helper.BoolToPtr(true), true},
+	}
+
+	for _, c := range cases {
+		t.Run(fmt.Sprintf("case: %v", c.input), func(t *testing.T) {
+			conf := DefaultConfig()
+			conf.Server.SystemSchedulerPreemptionEnabledDefault = c.input
+
+			a := &Agent{config: conf}
+			conf.AdvertiseAddrs.Serf = "127.0.0.1:4000"
+			conf.AdvertiseAddrs.RPC = "127.0.0.1:4001"
+			conf.AdvertiseAddrs.HTTP = "10.10.11.1:4005"
+			conf.ACL.Enabled = true
+			require.NoError(t, conf.normalizeAddrs())
+
+			out, err := a.serverConfig()
+			require.NoError(t, err)
+			require.Equal(t, c.expected, out.SystemSchedulerPreemptionEnabledDefault)
+		})
+	}
+}
 func TestAgent_ClientConfig(t *testing.T) {
 	t.Parallel()
 	conf := DefaultConfig()
diff --git a/command/agent/config.go b/command/agent/config.go
index 39259abb2c4..c3b8056197a 100644
--- a/command/agent/config.go
+++ b/command/agent/config.go
@@ -444,6 +444,9 @@ type ServerConfig struct {
 	// ServerJoin contains information that is used to attempt to join servers
 	ServerJoin *ServerJoin `hcl:"server_join"`
 
+	// SystemSchedulerPreemptionEnabledDefault is used to determin whether to enable system preemption by default in a new cluster
+	SystemSchedulerPreemptionEnabledDefault *bool `hcl:"system_scheduler_preemption_enabled_default"`
+
 	// ExtraKeysHCL is used by hcl to surface unexpected keys
 	ExtraKeysHCL []string `hcl:",unusedKeys" json:"-"`
 }
diff --git a/nomad/config.go b/nomad/config.go
index 4b441f135a3..67e02a19c33 100644
--- a/nomad/config.go
+++ b/nomad/config.go
@@ -305,6 +305,9 @@ type Config struct {
 	// dead servers.
 	AutopilotInterval time.Duration
 
+	// SystemSchedulerPreemptionEnabledDefault is used to determin whether to enable system preemption by default in a new cluster
+	SystemSchedulerPreemptionEnabledDefault bool
+
 	// PluginLoader is used to load plugins.
 	PluginLoader loader.PluginCatalog
 
@@ -377,8 +380,9 @@ func DefaultConfig() *Config {
 			MaxTrailingLogs:         250,
 			ServerStabilizationTime: 10 * time.Second,
 		},
-		ServerHealthInterval: 2 * time.Second,
-		AutopilotInterval:    10 * time.Second,
+		ServerHealthInterval:                    2 * time.Second,
+		AutopilotInterval:                       10 * time.Second,
+		SystemSchedulerPreemptionEnabledDefault: true,
 	}
 
 	// Enable all known schedulers by default
diff --git a/nomad/leader.go b/nomad/leader.go
index ec406a4b4b1..268b3a6ee95 100644
--- a/nomad/leader.go
+++ b/nomad/leader.go
@@ -45,12 +45,14 @@ var minAutopilotVersion = version.Must(version.NewVersion("0.8.0"))
 var minSchedulerConfigVersion = version.Must(version.NewVersion("0.9.0"))
 
 // Default configuration for scheduler with preemption enabled for system jobs
-var defaultSchedulerConfig = &structs.SchedulerConfiguration{
-	PreemptionConfig: structs.PreemptionConfig{
-		SystemSchedulerEnabled:  true,
-		BatchSchedulerEnabled:   false,
-		ServiceSchedulerEnabled: false,
-	},
+func (s *Server) defaultSchedulerConfig() structs.SchedulerConfiguration {
+	return structs.SchedulerConfiguration{
+		PreemptionConfig: structs.PreemptionConfig{
+			SystemSchedulerEnabled:  s.config.SystemSchedulerPreemptionEnabledDefault,
+			BatchSchedulerEnabled:   false,
+			ServiceSchedulerEnabled: false,
+		},
+	}
 }
 
 // monitorLeadership is used to monitor if we acquire or lose our role
@@ -1319,7 +1321,7 @@ func (s *Server) getOrCreateSchedulerConfig() *structs.SchedulerConfiguration {
 		return nil
 	}
 
-	req := structs.SchedulerSetConfigRequest{Config: *defaultSchedulerConfig}
+	req := structs.SchedulerSetConfigRequest{Config: s.defaultSchedulerConfig()}
 	if _, _, err = s.raftApply(structs.SchedulerConfigRequestType, req); err != nil {
 		s.logger.Named("core").Error("failed to initialize config", "error", err)
 		return nil