Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parsing of client configuration from the commandline #191

Merged
merged 4 commits into from
Oct 2, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 78 additions & 5 deletions command/agent/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ func (c *Command) readConfig() *Config {
// Server-only options
flags.IntVar(&cmdConfig.Server.BootstrapExpect, "bootstrap-expect", 0, "")

// Client-only options
flags.StringVar(&cmdConfig.Client.StateDir, "state-dir", "", "")
flags.StringVar(&cmdConfig.Client.AllocDir, "alloc-dir", "", "")
flags.StringVar(&cmdConfig.Client.NodeID, "node-id", "", "")
flags.StringVar(&cmdConfig.Client.NodeClass, "node-class", "", "")

var servers string
flags.StringVar(&servers, "servers", "", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I see why you did this, as we already have a -server arg. It would be nice if we could think of a reasonable name for a flag that you could specify multiple times, but for my lack of creativity this should suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I couldn't think of anything else either.


var meta []string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we have been putting all of the type declarations at the top so that we don't have to break up these blocks.

flags.Var((*sliceflag.StringFlag)(&meta), "meta", "")

// General options
flags.Var((*sliceflag.StringFlag)(&configPath), "config", "config")
flags.StringVar(&cmdConfig.BindAddr, "bind", "", "")
Expand All @@ -88,6 +100,25 @@ func (c *Command) readConfig() *Config {
return nil
}

// Split the servers.
if servers != "" {
cmdConfig.Client.Servers = strings.Split(servers, ",")
}

// Parse the meta flags.
if len(meta) != 0 {
cmdConfig.Client.Meta = make(map[string]string)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can specify the map length here since we already know it to save a few allocations.

for _, kv := range meta {
parts := strings.Split(kv, "=")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use SplitN here to allow = signs in the value. Might be useful for encoded values and such.

if len(parts) != 2 {
c.Ui.Error(fmt.Sprintf("Error parsing Client.Meta value: %v", kv))
return nil
}

cmdConfig.Client.Meta[parts[0]] = parts[1]
}
}

// Load the configuration
var config *Config
if dev {
Expand Down Expand Up @@ -134,10 +165,26 @@ func (c *Command) readConfig() *Config {
return config
}

// Check that we have a data-dir
if config.DataDir == "" {
// Check for valid modes.
if config.Server.Enabled && config.Client.Enabled {
c.Ui.Error("To run both as a server and client, use -dev mode.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these need to be mutually exclusive? I've used this mode a few times before to see how the client behaves and persists data in a bare minimum set-up. If there's a reason we should actually enforce this then this looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that is true. If you wanted to "dev" mode with persistence you could enable both. I will update.

return nil
} else if !(config.Server.Enabled || config.Client.Enabled) {
c.Ui.Error("Must specify either server, client or dev mode for the agent.")
return nil
}

// Ensure that we have the directories we neet to run.
if config.Server.Enabled && config.DataDir == "" {
c.Ui.Error("Must specify data directory")
return nil
} else if config.Client.Enabled && config.DataDir == "" {
// The config is valid if the top-level data-dir is set or if both
// alloc-dir and state-dir are set.
if config.Client.AllocDir == "" || config.Client.StateDir == "" {
c.Ui.Error("Must specify both the state and alloc dir if data-dir is omitted.")
return nil
}
}

// Check the bootstrap flags
Expand Down Expand Up @@ -588,9 +635,35 @@ Server Options:
Client Options:

-client
Enable client mode for the agent. Client mode enables a given node
to be evaluated for allocations. If client mode is not enabled,
no work will be scheduled to the agent.
Enable client mode for the agent. Client mode enables a given node to be
evaluated for allocations. If client mode is not enabled, no work will be
scheduled to the agent.

-state-dir
The directory used to store state and other persistent data. If not
specified a subdirectory under the "-data-dir" will be used.

-alloc-dir
The directory used to store allocation data such as downloaded artificats as
well as data produced by tasks. If not specified, a subdirectory under the
"-data-dir" will be used.

-servers
A list of known server addresses to connect to given as "host:port" and
delimited by commas.

-node-id
A unique identifier for the node to use. If not provided, a UUID is
generated.

-node-class
Mark this node as a member of a node-class. This can be used to label
similiar node types.

-meta
User specified metadata to associated with the node. Each instance of -meta
parses a single KEY=VALUE pair. Repeat the meta flag for each key/value pair
to be added.

Atlas Options:

Expand Down
16 changes: 14 additions & 2 deletions command/agent/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,28 @@ func TestCommand_Args(t *testing.T) {
tcases := []tcase{
{
[]string{},
"Must specify data directory",
"Must specify either server, client or dev mode for the agent.",
},
{
[]string{"-client", "-server"},
"To run both as a server and client, use -dev mode.",
},
{
[]string{"-data-dir=" + tmpDir, "-bootstrap-expect=1"},
[]string{"-client", "-data-dir=" + tmpDir, "-bootstrap-expect=1"},
"Bootstrap requires server mode to be enabled",
},
{
[]string{"-data-dir=" + tmpDir, "-server", "-bootstrap-expect=1"},
"WARNING: Bootstrap mode enabled!",
},
{
[]string{"-server"},
"Must specify data directory",
},
{
[]string{"-client", "-alloc-dir="},
"Must specify both the state and alloc dir if data-dir is omitted.",
},
}
for _, tc := range tcases {
// Make a new command. We pre-emptively close the shutdownCh
Expand Down