From eeaa95ddf991441fc7f3b88238e7380250b09070 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 13 May 2019 11:59:52 -0400 Subject: [PATCH] Use init to handle plugin invocation Currently, nomad "plugin" processes (e.g. executor, logmon, docker_logger) are started as CLI commands to be handled by command CLI framework. Plugin launchers use `discover.NomadBinary()` to identify the binary and start it. This has few downsides: The trivial one is that when running tests, one must re-compile the nomad binary as the tests need to invoke the nomad executable to start plugin. This is frequently overlooked, resulting in puzzlement. The more significant issue with `executor` in particular is in relation to external driver: * Plugin must identify the path of invoking nomad binary, which is not trivial; `discvoer.NomadBinary()` now returns the path to the plugin rather than to nomad, preventing external drivers from launching executors. * The external driver may get a different version of executor than it expects (specially if we make a binary incompatible change in future). This commit addresses both downside by having the plugin invocation handling through an `init()` call, similar to how libcontainer init handler is done in [1] and recommened by libcontainer [2]. `init()` will be invoked and handled properly in tests and external drivers. For external drivers, this change will cause external drivers to launch the executor that's compiled against. There a are a couple of downsides to this approach: * These specific packages (i.e executor, logmon, and dockerlog) need to be careful in use of `init()`, package initializers. Must avoid having command execution rely on any other init in the package. I prefixed files with `z_` (golang processes files in lexical order), but ensured we don't depend on order. * The command handling is spread in multiple packages making it a bit less obvious how plugin starts are handled. [1] drivers/shared/executor/libcontainer_nsenter_linux.go [2] https://github.com/opencontainers/runc/tree/eb4aeed24ffbf8e2d740fafea39d91faa0ee84d0/libcontainer#using-libcontainer --- client/logmon/plugin.go | 4 +- client/logmon/z_logmon_cmd.go | 28 ++++++++ command/commands.go | 16 ----- command/docker_logger_plugin.go | 43 ------------ command/executor_plugin.go | 66 ------------------- command/logmon_plugin.go | 42 ------------ drivers/docker/docklog/plugin.go | 4 +- drivers/docker/docklog/z_docker_logger_cmd.go | 29 ++++++++ drivers/shared/executor/utils.go | 4 +- drivers/shared/executor/z_executor_cmd.go | 51 ++++++++++++++ 10 files changed, 114 insertions(+), 173 deletions(-) create mode 100644 client/logmon/z_logmon_cmd.go delete mode 100644 command/docker_logger_plugin.go delete mode 100644 command/executor_plugin.go delete mode 100644 command/logmon_plugin.go create mode 100644 drivers/docker/docklog/z_docker_logger_cmd.go create mode 100644 drivers/shared/executor/z_executor_cmd.go diff --git a/client/logmon/plugin.go b/client/logmon/plugin.go index 3234806c761..5d97b751d10 100644 --- a/client/logmon/plugin.go +++ b/client/logmon/plugin.go @@ -2,12 +2,12 @@ package logmon import ( "context" + "os" "os/exec" hclog "github.com/hashicorp/go-hclog" plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/client/logmon/proto" - "github.com/hashicorp/nomad/helper/discover" "github.com/hashicorp/nomad/plugins/base" "google.golang.org/grpc" ) @@ -16,7 +16,7 @@ import ( // TODO: Integrate with base plugin loader func LaunchLogMon(logger hclog.Logger, reattachConfig *plugin.ReattachConfig) (LogMon, *plugin.Client, error) { logger = logger.Named("logmon") - bin, err := discover.NomadExecutable() + bin, err := os.Executable() if err != nil { return nil, nil, err } diff --git a/client/logmon/z_logmon_cmd.go b/client/logmon/z_logmon_cmd.go new file mode 100644 index 00000000000..eafc0b8a835 --- /dev/null +++ b/client/logmon/z_logmon_cmd.go @@ -0,0 +1,28 @@ +package logmon + +import ( + "os" + + hclog "github.com/hashicorp/go-hclog" + plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/nomad/plugins/base" +) + +func init() { + if len(os.Args) > 1 && os.Args[1] == "logmon" { + logger := hclog.New(&hclog.LoggerOptions{ + Level: hclog.Trace, + JSONFormat: true, + Name: "logmon", + }) + plugin.Serve(&plugin.ServeConfig{ + HandshakeConfig: base.Handshake, + Plugins: map[string]plugin.Plugin{ + "logmon": NewPlugin(NewLogMon(logger)), + }, + GRPCServer: plugin.DefaultGRPCServer, + Logger: logger, + }) + os.Exit(0) + } +} diff --git a/command/commands.go b/command/commands.go index 1c1666a498c..0eeade107cc 100644 --- a/command/commands.go +++ b/command/commands.go @@ -5,7 +5,6 @@ import ( "os" "github.com/hashicorp/nomad/command/agent" - "github.com/hashicorp/nomad/drivers/docker/docklog" "github.com/hashicorp/nomad/version" colorable "github.com/mattn/go-colorable" "github.com/mitchellh/cli" @@ -237,11 +236,6 @@ func Commands(metaPtr *Meta, agentUi cli.Ui) map[string]cli.CommandFactory { Meta: meta, }, nil }, - docklog.PluginName: func() (cli.Command, error) { - return &DockerLoggerPluginCommand{ - Meta: meta, - }, nil - }, "eval": func() (cli.Command, error) { return &EvalCommand{ Meta: meta, @@ -262,11 +256,6 @@ func Commands(metaPtr *Meta, agentUi cli.Ui) map[string]cli.CommandFactory { Meta: meta, }, nil }, - "executor": func() (cli.Command, error) { - return &ExecutorPluginCommand{ - Meta: meta, - }, nil - }, "fs": func() (cli.Command, error) { return &AllocFSCommand{ Meta: meta, @@ -372,11 +361,6 @@ func Commands(metaPtr *Meta, agentUi cli.Ui) map[string]cli.CommandFactory { Meta: meta, }, nil }, - "logmon": func() (cli.Command, error) { - return &LogMonPluginCommand{ - Meta: meta, - }, nil - }, "logs": func() (cli.Command, error) { return &AllocLogsCommand{ Meta: meta, diff --git a/command/docker_logger_plugin.go b/command/docker_logger_plugin.go deleted file mode 100644 index 707def19f41..00000000000 --- a/command/docker_logger_plugin.go +++ /dev/null @@ -1,43 +0,0 @@ -package command - -import ( - "strings" - - log "github.com/hashicorp/go-hclog" - plugin "github.com/hashicorp/go-plugin" - "github.com/hashicorp/nomad/drivers/docker/docklog" - "github.com/hashicorp/nomad/plugins/base" -) - -type DockerLoggerPluginCommand struct { - Meta -} - -func (e *DockerLoggerPluginCommand) Help() string { - helpText := ` - This is a command used by Nomad internally to launch the docker logger process" - ` - return strings.TrimSpace(helpText) -} - -func (e *DockerLoggerPluginCommand) Synopsis() string { - return "internal - launch a docker logger plugin" -} - -func (e *DockerLoggerPluginCommand) Run(args []string) int { - logger := log.New(&log.LoggerOptions{ - Level: log.Trace, - JSONFormat: true, - Name: docklog.PluginName, - }) - - plugin.Serve(&plugin.ServeConfig{ - HandshakeConfig: base.Handshake, - Plugins: map[string]plugin.Plugin{ - docklog.PluginName: docklog.NewPlugin(docklog.NewDockerLogger(logger)), - }, - GRPCServer: plugin.DefaultGRPCServer, - Logger: logger, - }) - return 0 -} diff --git a/command/executor_plugin.go b/command/executor_plugin.go deleted file mode 100644 index 0e4cfadb421..00000000000 --- a/command/executor_plugin.go +++ /dev/null @@ -1,66 +0,0 @@ -package command - -import ( - "encoding/json" - "os" - "strings" - - hclog "github.com/hashicorp/go-hclog" - log "github.com/hashicorp/go-hclog" - plugin "github.com/hashicorp/go-plugin" - - "github.com/hashicorp/nomad/drivers/shared/executor" - "github.com/hashicorp/nomad/plugins/base" -) - -type ExecutorPluginCommand struct { - Meta -} - -func (e *ExecutorPluginCommand) Help() string { - helpText := ` - This is a command used by Nomad internally to launch an executor plugin" - ` - return strings.TrimSpace(helpText) -} - -func (e *ExecutorPluginCommand) Synopsis() string { - return "internal - launch an executor plugin" -} - -func (e *ExecutorPluginCommand) Run(args []string) int { - if len(args) != 1 { - e.Ui.Error("json configuration not provided") - return 1 - } - - config := args[0] - var executorConfig executor.ExecutorConfig - if err := json.Unmarshal([]byte(config), &executorConfig); err != nil { - return 1 - } - - f, err := os.OpenFile(executorConfig.LogFile, os.O_CREATE|os.O_RDWR|os.O_APPEND, 0666) - if err != nil { - e.Ui.Error(err.Error()) - return 1 - } - - // Create the logger - logger := log.New(&log.LoggerOptions{ - Level: hclog.LevelFromString(executorConfig.LogLevel), - JSONFormat: true, - Output: f, - }) - - plugin.Serve(&plugin.ServeConfig{ - HandshakeConfig: base.Handshake, - Plugins: executor.GetPluginMap( - logger, - executorConfig.FSIsolation, - ), - GRPCServer: plugin.DefaultGRPCServer, - Logger: logger, - }) - return 0 -} diff --git a/command/logmon_plugin.go b/command/logmon_plugin.go deleted file mode 100644 index 4d3bda20c12..00000000000 --- a/command/logmon_plugin.go +++ /dev/null @@ -1,42 +0,0 @@ -package command - -import ( - "strings" - - hclog "github.com/hashicorp/go-hclog" - plugin "github.com/hashicorp/go-plugin" - "github.com/hashicorp/nomad/client/logmon" - "github.com/hashicorp/nomad/plugins/base" -) - -type LogMonPluginCommand struct { - Meta -} - -func (e *LogMonPluginCommand) Help() string { - helpText := ` - This is a command used by Nomad internally to launch the logmon process" - ` - return strings.TrimSpace(helpText) -} - -func (e *LogMonPluginCommand) Synopsis() string { - return "internal - launch a logmon plugin" -} - -func (e *LogMonPluginCommand) Run(args []string) int { - logger := hclog.New(&hclog.LoggerOptions{ - Level: hclog.Trace, - JSONFormat: true, - Name: "logmon", - }) - plugin.Serve(&plugin.ServeConfig{ - HandshakeConfig: base.Handshake, - Plugins: map[string]plugin.Plugin{ - "logmon": logmon.NewPlugin(logmon.NewLogMon(logger)), - }, - GRPCServer: plugin.DefaultGRPCServer, - Logger: logger, - }) - return 0 -} diff --git a/drivers/docker/docklog/plugin.go b/drivers/docker/docklog/plugin.go index 967eeb9cabe..f9feff281cc 100644 --- a/drivers/docker/docklog/plugin.go +++ b/drivers/docker/docklog/plugin.go @@ -2,12 +2,12 @@ package docklog import ( "context" + "os" "os/exec" hclog "github.com/hashicorp/go-hclog" plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/drivers/docker/docklog/proto" - "github.com/hashicorp/nomad/helper/discover" "github.com/hashicorp/nomad/plugins/base" "google.golang.org/grpc" ) @@ -17,7 +17,7 @@ const PluginName = "docker_logger" // LaunchDockerLogger launches an instance of DockerLogger func LaunchDockerLogger(logger hclog.Logger) (DockerLogger, *plugin.Client, error) { logger = logger.Named(PluginName) - bin, err := discover.NomadExecutable() + bin, err := os.Executable() if err != nil { return nil, nil, err } diff --git a/drivers/docker/docklog/z_docker_logger_cmd.go b/drivers/docker/docklog/z_docker_logger_cmd.go new file mode 100644 index 00000000000..dd853a450d1 --- /dev/null +++ b/drivers/docker/docklog/z_docker_logger_cmd.go @@ -0,0 +1,29 @@ +package docklog + +import ( + "os" + + log "github.com/hashicorp/go-hclog" + plugin "github.com/hashicorp/go-plugin" + "github.com/hashicorp/nomad/plugins/base" +) + +func init() { + if len(os.Args) > 1 && os.Args[1] == PluginName { + logger := log.New(&log.LoggerOptions{ + Level: log.Trace, + JSONFormat: true, + Name: PluginName, + }) + + plugin.Serve(&plugin.ServeConfig{ + HandshakeConfig: base.Handshake, + Plugins: map[string]plugin.Plugin{ + PluginName: NewPlugin(NewDockerLogger(logger)), + }, + GRPCServer: plugin.DefaultGRPCServer, + Logger: logger, + }) + os.Exit(0) + } +} diff --git a/drivers/shared/executor/utils.go b/drivers/shared/executor/utils.go index 6c551d17316..565cc80ac14 100644 --- a/drivers/shared/executor/utils.go +++ b/drivers/shared/executor/utils.go @@ -3,13 +3,13 @@ package executor import ( "encoding/json" "fmt" + "os" "os/exec" "github.com/golang/protobuf/ptypes" hclog "github.com/hashicorp/go-hclog" plugin "github.com/hashicorp/go-plugin" "github.com/hashicorp/nomad/drivers/shared/executor/proto" - "github.com/hashicorp/nomad/helper/discover" "github.com/hashicorp/nomad/plugins/base" ) @@ -32,7 +32,7 @@ func CreateExecutor(logger hclog.Logger, driverConfig *base.ClientDriverConfig, if err != nil { return nil, nil, fmt.Errorf("unable to create executor config: %v", err) } - bin, err := discover.NomadExecutable() + bin, err := os.Executable() if err != nil { return nil, nil, fmt.Errorf("unable to find the nomad binary: %v", err) } diff --git a/drivers/shared/executor/z_executor_cmd.go b/drivers/shared/executor/z_executor_cmd.go new file mode 100644 index 00000000000..aa25ef9b072 --- /dev/null +++ b/drivers/shared/executor/z_executor_cmd.go @@ -0,0 +1,51 @@ +package executor + +import ( + "encoding/json" + "os" + + hclog "github.com/hashicorp/go-hclog" + log "github.com/hashicorp/go-hclog" + plugin "github.com/hashicorp/go-plugin" + + "github.com/hashicorp/nomad/plugins/base" +) + +func init() { + if len(os.Args) > 1 && os.Args[1] == "executor" { + if len(os.Args) != 3 { + hclog.L().Error("json configuration not provided") + os.Exit(1) + } + + config := os.Args[2] + var executorConfig ExecutorConfig + if err := json.Unmarshal([]byte(config), &executorConfig); err != nil { + os.Exit(1) + } + + f, err := os.OpenFile(executorConfig.LogFile, os.O_CREATE|os.O_RDWR|os.O_APPEND, 0666) + if err != nil { + hclog.L().Error(err.Error()) + os.Exit(1) + } + + // Create the logger + logger := log.New(&log.LoggerOptions{ + Level: hclog.LevelFromString(executorConfig.LogLevel), + JSONFormat: true, + Output: f, + }) + + plugin.Serve(&plugin.ServeConfig{ + HandshakeConfig: base.Handshake, + Plugins: GetPluginMap( + logger, + executorConfig.FSIsolation, + ), + GRPCServer: plugin.DefaultGRPCServer, + Logger: logger, + }) + os.Exit(0) + } +}