-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
flags.StringVar(&cmdConfig.Client.NodeClass, "node-class", "", "") | ||
|
||
var servers string | ||
flags.StringVar(&servers, "servers", "", "") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Looks good! Just a few comments but I see you're already taking care of them. 👍 |
Thanks for the review 👍 |
Add parsing of client configuration from the commandline
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Allows configuring a nomad client agent from the command line. Solves #187