-
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 cluster join command line options and configuration options #527
Conversation
Thanks for the PR. Can you also update the website documentation under |
I updated |
@@ -358,14 +388,6 @@ func (a *Config) Merge(b *Config) *Config { | |||
result.AdvertiseAddrs = result.AdvertiseAddrs.Merge(b.AdvertiseAddrs) | |||
} | |||
|
|||
// Apply the Atlas configuration |
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.
Why was this removed?
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.
Bad git'ing on my part.
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.
👍
Looks great! That's probably the only spot in the docs that needs an update. |
I'm looking forward to having this PR merged to make cluster bootstrapping more automatic. |
if cmdConfig.Server.RetryInterval != "" { | ||
dur, err := time.ParseDuration(cmdConfig.Server.RetryInterval) | ||
if err != nil { | ||
c.Ui.Error(fmt.Sprintf("Error: %s", err)) |
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.
Can we add some extra information in the error message to indicate which field Nomad was not about to parse? I think that would help the user to quickly debug what's wrong in the configuration file.
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 updated the error message to Error parsing retry interval
Merging this PR into a branch to do more testing and understand how the UX works. |
@@ -396,12 +416,16 @@ func (c *Command) Run(args []string) int { | |||
// Enable log streaming | |||
logGate.Flush() | |||
|
|||
// Start retry join process | |||
errCh := make(chan struct{}) |
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.
Can we add the retryJoinErr channel to the Command struct so we don't have to pass the channel into the functions
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.
Added to the Command struct
Thanks @ChrisAubuchon |
Add cluster join command line options and configuration options
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. |
-rejoin
,-join
,-retry-join
,-retry-max
and-retry-interval
command line optionsstart_join
,retry_join
,retry_max
,retry_interval
andrejoin_after_leave
server configuration options