Pass plugin loaders to client and server

CHANGELOG.md

@@ -13,8 +13,8 @@ IMPROVEMENTS:
    parameterized and periodic jobs [[GH-4392](https://github.com/hashicorp/nomad/issues/4392)]
  * vendor: Removed library obsoleted by go 1.8 [[GH-4469](https://github.com/hashicorp/nomad/issues/4469)]
 
-
 BUG FIXES:
+* client: Fix an issue reloading the client config [[GH-4730](https://github.com/hashicorp/nomad/issues/4730)]
 
 # 0.8.5 (September 13, 2018)
 

client/config/config.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
+	"github.com/hashicorp/nomad/plugins/shared/loader"
 	"github.com/hashicorp/nomad/version"
 )
 
@@ -210,6 +211,13 @@ type Config struct {
 	// This period is meant to be long enough for a leader election to take
 	// place, and a small jitter is applied to avoid a thundering herd.
 	RPCHoldTimeout time.Duration
+
+	// 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
 }
 
 func (c *Config) Copy() *Config {

client/testing.go

@@ -8,12 +8,16 @@ import (
 	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/hashicorp/nomad/plugins/shared/catalog"
+	"github.com/hashicorp/nomad/plugins/shared/singleton"
 	"github.com/mitchellh/go-testing-interface"
 )
 
 // TestClient creates an in-memory client for testing purposes.
 func TestClient(t testing.T, cb func(c *config.Config)) *Client {
 	conf := config.DefaultConfig()
+	logger := testlog.HCLogger(t)
+	conf.Logger = logger
 	conf.VaultConfig.Enabled = helper.BoolToPtr(false)
 	conf.DevMode = true
 	conf.Node = &structs.Node{
@@ -32,12 +36,14 @@ func TestClient(t testing.T, cb func(c *config.Config)) *Client {
 	}
 	conf.Options[fingerprint.TightenNetworkTimeoutsConfig] = "true"
 
+	// Set the plugin loaders
+	conf.PluginLoader = catalog.TestPluginLoader(t)
+	conf.PluginSingletonLoader = singleton.NewSingletonLoader(logger, conf.PluginLoader)
+
 	if cb != nil {
 		cb(conf)
 	}
 
-	logger := testlog.HCLogger(t)
-	conf.Logger = logger
 	catalog := consul.NewMockCatalog(logger)
 	mockService := consulApi.NewMockConsulServiceClient(t, logger)
 	client, err := NewClient(conf, catalog, mockService)

command/agent/agent.go

@@ -19,6 +19,7 @@ import (
 	log "github.com/hashicorp/go-hclog"
 	uuidparse "github.com/hashicorp/go-uuid"
 	clientconfig "github.com/hashicorp/nomad/client/config"
+	"github.com/hashicorp/nomad/plugins/shared/loader"
 
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/lib"
@@ -64,10 +65,21 @@ type Agent struct {
 	// consulCatalog is the subset of Consul's Catalog API Nomad uses.
 	consulCatalog consul.CatalogAPI
 
+	// client is the launched Nomad Client. Can be nil if the agent isn't
+	// configured to run a client.
 	client *client.Client
 
+	// server is the launched Nomad Server. Can be nil if the agent isn't
+	// configured to run a server.
 	server *nomad.Server
 
+	// 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
+
 	shutdown     bool
 	shutdownCh   chan struct{}
 	shutdownLock sync.Mutex
@@ -100,7 +112,9 @@ func NewAgent(config *Config, logOutput io.Writer, inmem *metrics.InmemSink) (*A
 		return nil, fmt.Errorf("Failed to initialize Consul client: %v", err)
 	}
 
-	// TODO setup plugin loader
+	if err := a.setupPlugins(); err != nil {
+		return nil, err
+	}
 
 	if err := a.setupServer(); err != nil {
 		return nil, err
@@ -116,14 +130,13 @@ func NewAgent(config *Config, logOutput io.Writer, inmem *metrics.InmemSink) (*A
 }
 
 // convertServerConfig takes an agent config and log output and returns a Nomad
-// Config.
-func convertServerConfig(agentConfig *Config, logger log.Logger, logOutput io.Writer) (*nomad.Config, error) {
+// Config. There may be missing fields that must be set by the agent. To do this
+// call finalizeServerConfig
+func convertServerConfig(agentConfig *Config) (*nomad.Config, error) {
 	conf := agentConfig.NomadConfig
 	if conf == nil {
 		conf = nomad.DefaultConfig()
 	}
-	conf.Logger = logger
-	conf.LogOutput = logOutput
 	conf.DevMode = agentConfig.DevMode
 	conf.Build = agentConfig.Version.VersionNumber()
 	if agentConfig.Region != "" {
@@ -322,61 +335,73 @@ func convertServerConfig(agentConfig *Config, logger log.Logger, logOutput io.Wr
 // serverConfig is used to generate a new server configuration struct
 // for initializing a nomad server.
 func (a *Agent) serverConfig() (*nomad.Config, error) {
-	return convertServerConfig(a.config, a.logger, a.logOutput)
+	c, err := convertServerConfig(a.config)
+	if err != nil {
+		return nil, err
+	}
+
+	a.finalizeServerConfig(c)
+	return c, nil
+}
+
+// finalizeServerConfig sets configuration fields on the server config that are
+// not staticly convertable and are from the agent.
+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
 }
 
-// clientConfig is used to generate a new client configuration struct
-// for initializing a Nomad client.
+// clientConfig is used to generate a new client configuration struct for
+// initializing a Nomad client.
 func (a *Agent) clientConfig() (*clientconfig.Config, error) {
-	// Setup the configuration
-	conf := a.config.ClientConfig
-	if conf == nil {
-		conf = clientconfig.DefaultConfig()
+	c, err := convertClientConfig(a.config)
+	if err != nil {
+		return nil, err
 	}
 
+	if err := a.finalizeClientConfig(c); err != nil {
+		return nil, err
+	}
+
+	return c, nil
+}
+
+// finalizeClientConfig sets configuration fields on the client config that are
+// not staticly convertable and are from the agent.
+func (a *Agent) finalizeClientConfig(c *clientconfig.Config) error {
+	// Setup the logging
+	c.Logger = a.logger
+	c.LogOutput = a.logOutput
+
 	// If we are running a server, append both its bind and advertise address so
 	// we are able to at least talk to the local server even if that isn't
 	// configured explicitly. This handles both running server and client on one
 	// host and -dev mode.
-	conf.Servers = a.config.Client.Servers
 	if a.server != nil {
 		if a.config.AdvertiseAddrs == nil || a.config.AdvertiseAddrs.RPC == "" {
-			return nil, fmt.Errorf("AdvertiseAddrs is nil or empty")
+			return fmt.Errorf("AdvertiseAddrs is nil or empty")
 		} else if a.config.normalizedAddrs == nil || a.config.normalizedAddrs.RPC == "" {
-			return nil, fmt.Errorf("normalizedAddrs is nil or empty")
+			return fmt.Errorf("normalizedAddrs is nil or empty")
 		}
 
-		conf.Servers = append(conf.Servers,
+		c.Servers = append(c.Servers,
 			a.config.normalizedAddrs.RPC,
 			a.config.AdvertiseAddrs.RPC)
 	}
 
-	conf.Logger = a.logger
-	conf.LogOutput = a.logOutput
-	conf.LogLevel = a.config.LogLevel
-	conf.DevMode = a.config.DevMode
-	if a.config.Region != "" {
-		conf.Region = a.config.Region
-	}
-	if a.config.DataDir != "" {
-		conf.StateDir = filepath.Join(a.config.DataDir, "client")
-		conf.AllocDir = filepath.Join(a.config.DataDir, "alloc")
-	}
-	if a.config.Client.StateDir != "" {
-		conf.StateDir = a.config.Client.StateDir
-	}
-	if a.config.Client.AllocDir != "" {
-		conf.AllocDir = a.config.Client.AllocDir
-	}
-	if a.config.Client.NetworkInterface != "" {
-		conf.NetworkInterface = a.config.Client.NetworkInterface
-	}
-	conf.ChrootEnv = a.config.Client.ChrootEnv
-	conf.Options = a.config.Client.Options
-	// Logging deprecation messages about consul related configuration in client
+	// Setup the plugin loaders
+	c.PluginLoader = a.pluginLoader
+	c.PluginSingletonLoader = a.pluginSingletonLoader
+
+	// Log deprecation messages about Consul related configuration in client
 	// options
 	var invalidConsulKeys []string
-	for key := range conf.Options {
+	for key := range c.Options {
 		if strings.HasPrefix(key, "consul") {
 			invalidConsulKeys = append(invalidConsulKeys, fmt.Sprintf("options.%s", key))
 		}
@@ -388,84 +413,118 @@ func (a *Agent) clientConfig() (*clientconfig.Config, error) {
 		to configure Nomad to work with Consul.`)
 	}
 
-	if a.config.Client.NetworkSpeed != 0 {
-		conf.NetworkSpeed = a.config.Client.NetworkSpeed
+	return nil
+}
+
+// convertClientConfig takes an agent config and log output and returns a client
+// Config. There may be missing fields that must be set by the agent. To do this
+// call finalizeServerConfig
+func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
+	// Setup the configuration
+	conf := agentConfig.ClientConfig
+	if conf == nil {
+		conf = clientconfig.DefaultConfig()
 	}
-	if a.config.Client.CpuCompute != 0 {
-		conf.CpuCompute = a.config.Client.CpuCompute
+
+	conf.Servers = agentConfig.Client.Servers
+	conf.LogLevel = agentConfig.LogLevel
+	conf.DevMode = agentConfig.DevMode
+	if agentConfig.Region != "" {
+		conf.Region = agentConfig.Region
 	}
-	if a.config.Client.MemoryMB != 0 {
-		conf.MemoryMB = a.config.Client.MemoryMB
+	if agentConfig.DataDir != "" {
+		conf.StateDir = filepath.Join(agentConfig.DataDir, "client")
+		conf.AllocDir = filepath.Join(agentConfig.DataDir, "alloc")
 	}
-	if a.config.Client.MaxKillTimeout != "" {
-		dur, err := time.ParseDuration(a.config.Client.MaxKillTimeout)
+	if agentConfig.Client.StateDir != "" {
+		conf.StateDir = agentConfig.Client.StateDir
+	}
+	if agentConfig.Client.AllocDir != "" {
+		conf.AllocDir = agentConfig.Client.AllocDir
+	}
+	if agentConfig.Client.NetworkInterface != "" {
+		conf.NetworkInterface = agentConfig.Client.NetworkInterface
+	}
+	conf.ChrootEnv = agentConfig.Client.ChrootEnv
+	conf.Options = agentConfig.Client.Options
+	if agentConfig.Client.NetworkSpeed != 0 {
+		conf.NetworkSpeed = agentConfig.Client.NetworkSpeed
+	}
+	if agentConfig.Client.CpuCompute != 0 {
+		conf.CpuCompute = agentConfig.Client.CpuCompute
+	}
+	if agentConfig.Client.MemoryMB != 0 {
+		conf.MemoryMB = agentConfig.Client.MemoryMB
+	}
+	if agentConfig.Client.MaxKillTimeout != "" {
+		dur, err := time.ParseDuration(agentConfig.Client.MaxKillTimeout)
 		if err != nil {
 			return nil, fmt.Errorf("Error parsing max kill timeout: %s", err)
 		}
 		conf.MaxKillTimeout = dur
 	}
-	conf.ClientMaxPort = uint(a.config.Client.ClientMaxPort)
-	conf.ClientMinPort = uint(a.config.Client.ClientMinPort)
+	conf.ClientMaxPort = uint(agentConfig.Client.ClientMaxPort)
+	conf.ClientMinPort = uint(agentConfig.Client.ClientMinPort)
 
 	// Setup the node
 	conf.Node = new(structs.Node)
-	conf.Node.Datacenter = a.config.Datacenter
-	conf.Node.Name = a.config.NodeName
-	conf.Node.Meta = a.config.Client.Meta
-	conf.Node.NodeClass = a.config.Client.NodeClass
+	conf.Node.Datacenter = agentConfig.Datacenter
+	conf.Node.Name = agentConfig.NodeName
+	conf.Node.Meta = agentConfig.Client.Meta
+	conf.Node.NodeClass = agentConfig.Client.NodeClass
 
 	// Set up the HTTP advertise address
-	conf.Node.HTTPAddr = a.config.AdvertiseAddrs.HTTP
+	conf.Node.HTTPAddr = agentConfig.AdvertiseAddrs.HTTP
 
 	// Reserve resources on the node.
 	r := conf.Node.Reserved
 	if r == nil {
 		r = new(structs.Resources)
 		conf.Node.Reserved = r
 	}
-	r.CPU = a.config.Client.Reserved.CPU
-	r.MemoryMB = a.config.Client.Reserved.MemoryMB
-	r.DiskMB = a.config.Client.Reserved.DiskMB
-	r.IOPS = a.config.Client.Reserved.IOPS
-	conf.GloballyReservedPorts = a.config.Client.Reserved.ParsedReservedPorts
+	r.CPU = agentConfig.Client.Reserved.CPU
+	r.MemoryMB = agentConfig.Client.Reserved.MemoryMB
+	r.DiskMB = agentConfig.Client.Reserved.DiskMB
+	r.IOPS = agentConfig.Client.Reserved.IOPS
+	conf.GloballyReservedPorts = agentConfig.Client.Reserved.ParsedReservedPorts
 
-	conf.Version = a.config.Version
+	conf.Version = agentConfig.Version
 
-	if *a.config.Consul.AutoAdvertise && a.config.Consul.ClientServiceName == "" {
+	if *agentConfig.Consul.AutoAdvertise && agentConfig.Consul.ClientServiceName == "" {
 		return nil, fmt.Errorf("client_service_name must be set when auto_advertise is enabled")
 	}
 
-	conf.ConsulConfig = a.config.Consul
-	conf.VaultConfig = a.config.Vault
+	conf.ConsulConfig = agentConfig.Consul
+	conf.VaultConfig = agentConfig.Vault
 
 	// Set up Telemetry configuration
-	conf.StatsCollectionInterval = a.config.Telemetry.collectionInterval
-	conf.PublishNodeMetrics = a.config.Telemetry.PublishNodeMetrics
-	conf.PublishAllocationMetrics = a.config.Telemetry.PublishAllocationMetrics
-	conf.DisableTaggedMetrics = a.config.Telemetry.DisableTaggedMetrics
-	conf.BackwardsCompatibleMetrics = a.config.Telemetry.BackwardsCompatibleMetrics
+	conf.StatsCollectionInterval = agentConfig.Telemetry.collectionInterval
+	conf.PublishNodeMetrics = agentConfig.Telemetry.PublishNodeMetrics
+	conf.PublishAllocationMetrics = agentConfig.Telemetry.PublishAllocationMetrics
+	conf.DisableTaggedMetrics = agentConfig.Telemetry.DisableTaggedMetrics
+	conf.BackwardsCompatibleMetrics = agentConfig.Telemetry.BackwardsCompatibleMetrics
 
 	// Set the TLS related configs
-	conf.TLSConfig = a.config.TLSConfig
+	conf.TLSConfig = agentConfig.TLSConfig
 	conf.Node.TLSEnabled = conf.TLSConfig.EnableHTTP
 
 	// Set the GC related configs
-	conf.GCInterval = a.config.Client.GCInterval
-	conf.GCParallelDestroys = a.config.Client.GCParallelDestroys
-	conf.GCDiskUsageThreshold = a.config.Client.GCDiskUsageThreshold
-	conf.GCInodeUsageThreshold = a.config.Client.GCInodeUsageThreshold
-	conf.GCMaxAllocs = a.config.Client.GCMaxAllocs
-	if a.config.Client.NoHostUUID != nil {
-		conf.NoHostUUID = *a.config.Client.NoHostUUID
+	conf.GCInterval = agentConfig.Client.GCInterval
+	conf.GCParallelDestroys = agentConfig.Client.GCParallelDestroys
+	conf.GCDiskUsageThreshold = agentConfig.Client.GCDiskUsageThreshold
+	conf.GCInodeUsageThreshold = agentConfig.Client.GCInodeUsageThreshold
+	conf.GCMaxAllocs = agentConfig.Client.GCMaxAllocs
+	if agentConfig.Client.NoHostUUID != nil {
+		conf.NoHostUUID = *agentConfig.Client.NoHostUUID
 	} else {
 		// Default no_host_uuid to true
 		conf.NoHostUUID = true
 	}
 
 	// Setup the ACLs
-	conf.ACLEnabled = a.config.ACL.Enabled
-	conf.ACLTokenTTL = a.config.ACL.TokenTTL
-	conf.ACLPolicyTTL = a.config.ACL.PolicyTTL
+	conf.ACLEnabled = agentConfig.ACL.Enabled
+	conf.ACLTokenTTL = agentConfig.ACL.TokenTTL
+	conf.ACLPolicyTTL = agentConfig.ACL.PolicyTTL
 
 	return conf, nil
 }