From 850f1650438bc2306cc175e941511939c05c851c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 26 Sep 2018 13:38:46 -0700 Subject: [PATCH 1/4] Internal plugin catalog --- plugins/shared/catalog/catalog.go | 53 +++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 plugins/shared/catalog/catalog.go diff --git a/plugins/shared/catalog/catalog.go b/plugins/shared/catalog/catalog.go new file mode 100644 index 00000000000..36bb7c6c0db --- /dev/null +++ b/plugins/shared/catalog/catalog.go @@ -0,0 +1,53 @@ +// Package catalog is used to register internal plugins such that they can be +// loaded. +package catalog + +import ( + "sync" + + "github.com/hashicorp/nomad/plugins/shared/loader" +) + +var ( + // catalog is the set of registered internal plugins + catalog = map[loader.PluginID]*Registration{} + mu sync.Mutex +) + +// Registration is the registration of an internal plugin +type Registration struct { + Config *loader.InternalPluginConfig + ConfigLoader ConfigFromOptions +} + +// ConfigFromOptions is used to retrieve a plugin config when passed a node's +// option map. This allows upgrade pathing from the old configuration format to +// the new config format. +type ConfigFromOptions func(options map[string]string) (config map[string]interface{}, err error) + +// Register is used to register an internal plugin. +func Register(id loader.PluginID, config *loader.InternalPluginConfig) { + mu.Lock() + defer mu.Unlock() + catalog[id] = &Registration{ + Config: config, + } +} + +// RegisterDeferredConfig is used to register an internal plugin that sets its +// config using the client's option map. +func RegisterDeferredConfig(id loader.PluginID, config *loader.InternalPluginConfig, configLoader ConfigFromOptions) { + mu.Lock() + defer mu.Unlock() + catalog[id] = &Registration{ + Config: config, + ConfigLoader: configLoader, + } +} + +// Catalog returns the catalog of internal plugins +func Catalog() map[loader.PluginID]*Registration { + mu.Lock() + defer mu.Unlock() + return catalog +} From bd4cfbcce9ce3b69664757983aaa5d986b6a523e Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 26 Sep 2018 13:39:09 -0700 Subject: [PATCH 2/4] Plugin loader initialization --- command/agent/agent.go | 15 +++++ command/agent/plugins.go | 82 ++++++++++++++++++++++++++++ plugins/shared/loader/init.go | 13 ++++- plugins/shared/loader/loader_test.go | 69 +++++++++++++++++++++++ 4 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 command/agent/plugins.go diff --git a/command/agent/agent.go b/command/agent/agent.go index b7016e7451a..30202b84beb 100644 --- a/command/agent/agent.go +++ b/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 @@ -101,6 +113,9 @@ func NewAgent(config *Config, logOutput io.Writer, inmem *metrics.InmemSink) (*A } // TODO setup plugin loader + if err := a.setupPlugins(); err != nil { + return nil, err + } if err := a.setupServer(); err != nil { return nil, err diff --git a/command/agent/plugins.go b/command/agent/plugins.go new file mode 100644 index 00000000000..a0226246aad --- /dev/null +++ b/command/agent/plugins.go @@ -0,0 +1,82 @@ +package agent + +import ( + "fmt" + + "github.com/hashicorp/nomad/plugins/shared/catalog" + "github.com/hashicorp/nomad/plugins/shared/loader" + "github.com/hashicorp/nomad/plugins/shared/singleton" +) + +// setupPlugins is used to setup the plugin loaders. +func (a *Agent) setupPlugins() error { + // Get our internal plugins + internal, err := a.internalPluginConfigs() + if err != nil { + return err + } + + // Build the plugin loader + config := &loader.PluginLoaderConfig{ + Logger: a.logger, + PluginDir: a.config.PluginDir, + Configs: a.config.Plugins, + InternalPlugins: internal, + } + l, err := loader.NewPluginLoader(config) + if err != nil { + return fmt.Errorf("failed to create plugin loader: %v", err) + } + a.pluginLoader = l + + // Wrap the loader to get our singleton loader + a.pluginSingletonLoader = singleton.NewSingletonLoader(a.logger, l) + + for k, plugins := range a.pluginLoader.Catalog() { + for _, p := range plugins { + a.logger.Info("detected plugin", "name", p.Name, "type", k, "plugin_version", p.PluginVersion) + } + } + + return nil +} + +func (a *Agent) internalPluginConfigs() (map[loader.PluginID]*loader.InternalPluginConfig, error) { + // Get the registered plugins + catalog := catalog.Catalog() + + // Create our map of plugins + internal := make(map[loader.PluginID]*loader.InternalPluginConfig, len(catalog)) + + // Grab the client options map if we can + var options map[string]string + if a.config != nil && a.config.Client != nil { + options = a.config.Client.Options + } + + for id, reg := range catalog { + if reg.Config == nil { + a.logger.Warn("skipping loading internal plugin because it is missing its configuration", "plugin", id) + continue + } + + pluginConfig := reg.Config.Config + if reg.ConfigLoader != nil { + pc, err := reg.ConfigLoader(options) + if err != nil { + return nil, fmt.Errorf("failed to retrieve config for internal plugin %v: %v", id, err) + } + + pluginConfig = pc + + // TODO We should log the config to warn users about upgrade pathing + } + + internal[id] = &loader.InternalPluginConfig{ + Factory: reg.Config.Factory, + Config: pluginConfig, + } + } + + return internal, nil +} diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 3972b72e81b..5ca4dca48dc 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -61,8 +61,11 @@ func validateConfig(config *PluginLoaderConfig) error { // init initializes the plugin loader by compiling both internal and external // plugins and selecting the highest versioned version of any given plugin. func (l *PluginLoader) init(config *PluginLoaderConfig) error { + // Create a mapping of name to config + configMap := configMap(config.Configs) + // Initialize the internal plugins - internal, err := l.initInternal(config.InternalPlugins) + internal, err := l.initInternal(config.InternalPlugins, configMap) if err != nil { return fmt.Errorf("failed to fingerprint internal plugins: %v", err) } @@ -74,7 +77,6 @@ func (l *PluginLoader) init(config *PluginLoaderConfig) error { } // Fingerprint the passed plugins - configMap := configMap(config.Configs) external, err := l.fingerprintPlugins(plugins, configMap) if err != nil { return fmt.Errorf("failed to fingerprint plugins: %v", err) @@ -92,7 +94,7 @@ func (l *PluginLoader) init(config *PluginLoaderConfig) error { } // initInternal initializes internal plugins. -func (l *PluginLoader) initInternal(plugins map[PluginID]*InternalPluginConfig) (map[PluginID]*pluginInfo, error) { +func (l *PluginLoader) initInternal(plugins map[PluginID]*InternalPluginConfig, configs map[string]*config.PluginConfig) (map[PluginID]*pluginInfo, error) { var mErr multierror.Error fingerprinted := make(map[PluginID]*pluginInfo, len(plugins)) for k, config := range plugins { @@ -109,6 +111,11 @@ func (l *PluginLoader) initInternal(plugins map[PluginID]*InternalPluginConfig) config: config.Config, } + // Try to retrieve a user specified config + if userConfig, ok := configs[k.Name]; ok && userConfig.Config != nil { + info.config = userConfig.Config + } + // Fingerprint base info i, err := base.PluginInfo() if err != nil { diff --git a/plugins/shared/loader/loader_test.go b/plugins/shared/loader/loader_test.go index 982be796851..3bc4d53a01e 100644 --- a/plugins/shared/loader/loader_test.go +++ b/plugins/shared/loader/loader_test.go @@ -427,6 +427,75 @@ func TestPluginLoader_Internal_Config(t *testing.T) { require.EqualValues(expected, detected) } +// Tests that an external config can override the config of an internal plugin +func TestPluginLoader_Internal_ExternalConfig(t *testing.T) { + t.Parallel() + require := require.New(t) + + // Create the harness + h := newHarness(t, nil) + defer h.cleanup() + + plugin := "mock-device" + pluginVersion := "v0.0.1" + + id := PluginID{ + Name: plugin, + PluginType: base.PluginTypeDevice, + } + expectedConfig := map[string]interface{}{ + "foo": "2", + "bar": "3", + } + + logger := testlog.HCLogger(t) + logger.SetLevel(log.Trace) + lconfig := &PluginLoaderConfig{ + Logger: logger, + PluginDir: h.pluginDir(), + InternalPlugins: map[PluginID]*InternalPluginConfig{ + id: { + Factory: mockFactory(plugin, base.PluginTypeDevice, pluginVersion, true), + Config: map[string]interface{}{ + "foo": "1", + "bar": "2", + }, + }, + }, + Configs: []*config.PluginConfig{ + { + Name: plugin, + Config: expectedConfig, + }, + }, + } + + l, err := NewPluginLoader(lconfig) + require.NoError(err) + + // Get the catalog and assert we have the two plugins + c := l.Catalog() + require.Len(c, 1) + require.Contains(c, base.PluginTypeDevice) + detected := c[base.PluginTypeDevice] + require.Len(detected, 1) + + expected := []*base.PluginInfoResponse{ + { + Name: plugin, + Type: base.PluginTypeDevice, + PluginVersion: pluginVersion, + PluginApiVersion: "v0.1.0", + }, + } + require.EqualValues(expected, detected) + + // Check the config + loaded, ok := l.plugins[id] + require.True(ok) + require.EqualValues(expectedConfig, loaded.config) +} + // Pass a config but make sure it is fatal func TestPluginLoader_Internal_Config_Bad(t *testing.T) { t.Parallel() From 66c487ab5ff9f2d550c2094df6d56527503e29f6 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 26 Sep 2018 18:14:36 -0700 Subject: [PATCH 3/4] Fix client reloading and pass the plugin loaders to server and client --- CHANGELOG.md | 2 +- client/config/config.go | 8 ++ client/testing.go | 10 +- command/agent/agent.go | 204 ++++++++++++++++++------------ command/agent/command.go | 27 ++-- nomad/config.go | 8 ++ nomad/testing.go | 6 + plugins/shared/catalog/testing.go | 65 ++++++++++ plugins/shared/loader/init.go | 11 +- 9 files changed, 248 insertions(+), 93 deletions(-) create mode 100644 plugins/shared/catalog/testing.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 480919904d4..2c9e04c7e67 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/client/config/config.go b/client/config/config.go index bbc0c2a6a9f..8a9961eed11 100644 --- a/client/config/config.go +++ b/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 { diff --git a/client/testing.go b/client/testing.go index cde5e8b8010..a67caa0dd9f 100644 --- a/client/testing.go +++ b/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) diff --git a/command/agent/agent.go b/command/agent/agent.go index 30202b84beb..d6980468bb6 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -112,7 +112,6 @@ 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 } @@ -131,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 != "" { @@ -337,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)) } @@ -403,34 +413,68 @@ 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 agentConfig.DataDir != "" { + conf.StateDir = filepath.Join(agentConfig.DataDir, "client") + conf.AllocDir = filepath.Join(agentConfig.DataDir, "alloc") } - if a.config.Client.MemoryMB != 0 { - conf.MemoryMB = a.config.Client.MemoryMB + if agentConfig.Client.StateDir != "" { + conf.StateDir = agentConfig.Client.StateDir } - if a.config.Client.MaxKillTimeout != "" { - dur, err := time.ParseDuration(a.config.Client.MaxKillTimeout) + 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 @@ -438,49 +482,49 @@ func (a *Agent) clientConfig() (*clientconfig.Config, error) { 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 } diff --git a/command/agent/command.go b/command/agent/command.go index e15555c33e1..5c049be5025 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -751,25 +751,36 @@ func (c *Command) handleReload() { if s := c.agent.Server(); s != nil { c.agent.logger.Debug("starting reload of server config") - sconf, err := convertServerConfig(newConf, c.agent.logger, c.logOutput) + sconf, err := convertServerConfig(newConf) if err != nil { c.agent.logger.Error("failed to convert server config", "error", err) return - } else { - if err := s.Reload(sconf); err != nil { - c.agent.logger.Error("reloading server config failed", "error", err) - return - } + } + + // Finalize the config to get the agent objects injected in + c.agent.finalizeServerConfig(sconf) + + // Reload the config + if err := s.Reload(sconf); err != nil { + c.agent.logger.Error("reloading server config failed", "error", err) + return } } if s := c.agent.Client(); s != nil { - clientConfig, err := c.agent.clientConfig() c.agent.logger.Debug("starting reload of client config") + clientConfig, err := convertClientConfig(newConf) if err != nil { - c.agent.logger.Error("reloading client config failed", "error", err) + c.agent.logger.Error("failed to convert client config", "error", err) return } + + // Finalize the config to get the agent objects injected in + if err := c.agent.finalizeClientConfig(clientConfig); err != nil { + c.agent.logger.Error("failed to finalize client config", "error", err) + return + } + if err := c.agent.Client().Reload(clientConfig); err != nil { c.agent.logger.Error("reloading client config failed", "error", err) return diff --git a/nomad/config.go b/nomad/config.go index 2e0b49ed7fb..c712e5833e1 100644 --- a/nomad/config.go +++ b/nomad/config.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" + "github.com/hashicorp/nomad/plugins/shared/loader" "github.com/hashicorp/nomad/scheduler" "github.com/hashicorp/raft" "github.com/hashicorp/serf/serf" @@ -295,6 +296,13 @@ type Config struct { // autopilot tasks, such as promoting eligible non-voters and removing // dead servers. AutopilotInterval 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 } // CheckVersion is used to check if the ProtocolVersion is valid diff --git a/nomad/testing.go b/nomad/testing.go index 9696aa386fe..7d0f9fcd079 100644 --- a/nomad/testing.go +++ b/nomad/testing.go @@ -12,6 +12,8 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/mock" "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" ) @@ -69,6 +71,10 @@ func TestServer(t testing.T, cb func(*Config)) *Server { 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) + // Invoke the callback if any if cb != nil { cb(config) diff --git a/plugins/shared/catalog/testing.go b/plugins/shared/catalog/testing.go new file mode 100644 index 00000000000..59bcc6cf183 --- /dev/null +++ b/plugins/shared/catalog/testing.go @@ -0,0 +1,65 @@ +package catalog + +import ( + "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/nomad/structs/config" + "github.com/hashicorp/nomad/plugins/shared/loader" + "github.com/mitchellh/go-testing-interface" +) + +// TestPluginLoader returns a plugin loader populated only with internal plugins +func TestPluginLoader(t testing.T) loader.PluginCatalog { + return TestPluginLoaderWithOptions(t, "", nil, nil) +} + +// TestPluginLoaderWithOptions allows configuring the plugin loader fully. +func TestPluginLoaderWithOptions(t testing.T, + pluginDir string, + options map[string]string, + configs []*config.PluginConfig) loader.PluginCatalog { + + // Get a logger + logger := testlog.HCLogger(t) + + // Get the registered plugins + catalog := Catalog() + + // Create our map of plugins + internal := make(map[loader.PluginID]*loader.InternalPluginConfig, len(catalog)) + + for id, reg := range catalog { + if reg.Config == nil { + logger.Warn("skipping loading internal plugin because it is missing its configuration", "plugin", id) + continue + } + + pluginConfig := reg.Config.Config + if reg.ConfigLoader != nil { + pc, err := reg.ConfigLoader(options) + if err != nil { + t.Fatalf("failed to retrieve config for internal plugin %v: %v", id, err) + } + + pluginConfig = pc + } + + internal[id] = &loader.InternalPluginConfig{ + Factory: reg.Config.Factory, + Config: pluginConfig, + } + } + + // Build the plugin loader + config := &loader.PluginLoaderConfig{ + Logger: logger, + PluginDir: "", + Configs: configs, + InternalPlugins: internal, + } + l, err := loader.NewPluginLoader(config) + if err != nil { + t.Fatalf("failed to create plugin loader: %v", err) + } + + return l +} diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 5ca4dca48dc..1f9a48f545e 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -32,8 +32,6 @@ func validateConfig(config *PluginLoaderConfig) error { return fmt.Errorf("nil config passed") } else if config.Logger == nil { multierror.Append(&mErr, fmt.Errorf("nil logger passed")) - } else if config.PluginDir == "" { - multierror.Append(&mErr, fmt.Errorf("invalid plugin dir %q passed", config.PluginDir)) } // Validate that all plugins have a binary name @@ -149,9 +147,18 @@ func (l *PluginLoader) initInternal(plugins map[PluginID]*InternalPluginConfig, // scan scans the plugin directory and retrieves potentially eligible binaries func (l *PluginLoader) scan() ([]os.FileInfo, error) { + if l.pluginDir == "" { + return nil, nil + } + // Capture the list of binaries in the plugins folder f, err := os.Open(l.pluginDir) if err != nil { + // There are no plugins to scan + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("failed to open plugin directory %q: %v", l.pluginDir, err) } files, err := f.Readdirnames(-1) From e62683d1cdd2561bdc089c0c4ab581a522799ead Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 27 Sep 2018 09:34:58 -0700 Subject: [PATCH 4/4] extra logging --- command/agent/plugins.go | 2 +- plugins/shared/loader/init.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/command/agent/plugins.go b/command/agent/plugins.go index a0226246aad..6637d69f97a 100644 --- a/command/agent/plugins.go +++ b/command/agent/plugins.go @@ -56,7 +56,7 @@ func (a *Agent) internalPluginConfigs() (map[loader.PluginID]*loader.InternalPlu for id, reg := range catalog { if reg.Config == nil { - a.logger.Warn("skipping loading internal plugin because it is missing its configuration", "plugin", id) + a.logger.Error("skipping loading internal plugin because it is missing its configuration", "plugin", id) continue } diff --git a/plugins/shared/loader/init.go b/plugins/shared/loader/init.go index 1f9a48f545e..da15623e3b7 100644 --- a/plugins/shared/loader/init.go +++ b/plugins/shared/loader/init.go @@ -156,6 +156,7 @@ func (l *PluginLoader) scan() ([]os.FileInfo, error) { if err != nil { // There are no plugins to scan if os.IsNotExist(err) { + l.logger.Debug("skipping external plugins since plugin_dir doesn't exist") return nil, nil }