From e803d7c77f581a7c19fcf04cc9aa7137878261cc Mon Sep 17 00:00:00 2001
From: Charlie Voiselle <464492+angrycub@users.noreply.github.com>
Date: Fri, 10 Feb 2023 15:33:16 -0500
Subject: [PATCH] [core] Do not start the plugin loader on non-clients (#16111)

The plugin loader loads task and device driver plugins which are not
used on server nodes.
---
 .changelog/16111.txt          |  3 +++
 command/agent/agent.go        | 16 ++++++++--------
 command/agent/plugins_test.go | 33 +++++++++++++++++++++++++++++++++
 nomad/config.go               |  8 --------
 nomad/testing.go              |  6 ------
 5 files changed, 44 insertions(+), 22 deletions(-)
 create mode 100644 .changelog/16111.txt
 create mode 100644 command/agent/plugins_test.go

diff --git a/.changelog/16111.txt b/.changelog/16111.txt
new file mode 100644
index 000000000..87bb05868
--- /dev/null
+++ b/.changelog/16111.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+core: Non-client nodes will now skip loading plugins
+```
diff --git a/command/agent/agent.go b/command/agent/agent.go
index d5703f721..e8d8eb0f5 100644
--- a/command/agent/agent.go
+++ b/command/agent/agent.go
@@ -142,10 +142,6 @@ func NewAgent(config *Config, logger log.InterceptLogger, logOutput io.Writer, i
 		return nil, fmt.Errorf("Failed to initialize Consul client: %v", err)
 	}
 
-	if err := a.setupPlugins(); err != nil {
-		return nil, err
-	}
-
 	if err := a.setupServer(); err != nil {
 		return nil, err
 	}
@@ -568,10 +564,6 @@ func (a *Agent) finalizeServerConfig(c *nomad.Config) {
 	// Setup the logging
 	c.Logger = a.logger
 	c.LogOutput = a.logOutput
-
-	// Setup the plugin loaders
-	c.PluginLoader = a.pluginLoader
-	c.PluginSingletonLoader = a.pluginSingletonLoader
 	c.AgentShutdown = func() error { return a.Shutdown() }
 }
 
@@ -1001,6 +993,14 @@ func (a *Agent) setupClient() error {
 	if !a.config.Client.Enabled {
 		return nil
 	}
+
+	// Plugin setup must happen before the call to clientConfig, because it
+	// copies the pointers to the plugin loaders from the Agent to the
+	// Client config.
+	if err := a.setupPlugins(); err != nil {
+		return err
+	}
+
 	// Setup the configuration
 	conf, err := a.clientConfig()
 	if err != nil {
diff --git a/command/agent/plugins_test.go b/command/agent/plugins_test.go
new file mode 100644
index 000000000..be8c3d56c
--- /dev/null
+++ b/command/agent/plugins_test.go
@@ -0,0 +1,33 @@
+package agent
+
+import (
+	"testing"
+
+	"github.com/hashicorp/nomad/api"
+	"github.com/shoenig/test/must"
+)
+
+func TestPlugins_WhenNotClientSkip(t *testing.T) {
+	s, _, _ := testServer(t, false, nil)
+	must.Nil(t, s.Agent.pluginSingletonLoader)
+}
+
+func TestPlugins_WhenClientRun(t *testing.T) {
+	s, _, _ := testServer(t, true, nil)
+	must.NotNil(t, s.Agent.pluginSingletonLoader)
+}
+
+func testServer(t *testing.T, runClient bool, cb func(*Config)) (*TestAgent, *api.Client, string) {
+	// Make a new test server
+	a := NewTestAgent(t, t.Name(), func(config *Config) {
+		config.Client.Enabled = runClient
+
+		if cb != nil {
+			cb(config)
+		}
+	})
+	t.Cleanup(a.Shutdown)
+
+	c := a.Client()
+	return a, c, a.HTTPAddr()
+}
diff --git a/nomad/config.go b/nomad/config.go
index 5442e90db..8ccfbff53 100644
--- a/nomad/config.go
+++ b/nomad/config.go
@@ -11,7 +11,6 @@ import (
 	"golang.org/x/exp/slices"
 
 	"github.com/hashicorp/memberlist"
-	"github.com/hashicorp/nomad/helper/pluginutils/loader"
 	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad/deploymentwatcher"
@@ -373,13 +372,6 @@ type Config struct {
 	// and this value is ignored.
 	DefaultSchedulerConfig structs.SchedulerConfiguration `hcl:"default_scheduler_config"`
 
-	// PluginLoader is used to load plugins.
-	PluginLoader loader.PluginCatalog
-
-	// PluginSingletonLoader is a plugin loader that will returns singleton
-	// instances of the plugins.
-	PluginSingletonLoader loader.PluginCatalog
-
 	// RPCHandshakeTimeout is the deadline by which RPC handshakes must
 	// complete. The RPC handshake includes the first byte read as well as
 	// the TLS handshake and subsequent byte read if TLS is enabled.
diff --git a/nomad/testing.go b/nomad/testing.go
index a74aa07db..696e10841 100644
--- a/nomad/testing.go
+++ b/nomad/testing.go
@@ -10,8 +10,6 @@ import (
 
 	"github.com/hashicorp/nomad/ci"
 	"github.com/hashicorp/nomad/command/agent/consul"
-	"github.com/hashicorp/nomad/helper/pluginutils/catalog"
-	"github.com/hashicorp/nomad/helper/pluginutils/singleton"
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/nomad/mock"
 	"github.com/hashicorp/nomad/nomad/structs"
@@ -85,10 +83,6 @@ func TestServerErr(t *testing.T, cb func(*Config)) (*Server, func(), error) {
 	config.ServerHealthInterval = 50 * time.Millisecond
 	config.AutopilotInterval = 100 * time.Millisecond
 
-	// Set the plugin loaders
-	config.PluginLoader = catalog.TestPluginLoader(t)
-	config.PluginSingletonLoader = singleton.NewSingletonLoader(config.Logger, config.PluginLoader)
-
 	// Disable consul autojoining: tests typically join servers directly
 	config.ConsulConfig.ServerAutoJoin = &f