Skip to content

Specify restrictions on the target of init #2670

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

Merged
merged 1 commit into from
Mar 12, 2018

Conversation

pbardea
Copy link
Contributor

@pbardea pbardea commented Mar 8, 2018

Explained that the target of the cockroach init command must be one of
the nodes which appear in the --join flag of the cockroach start
command.

Fixes #2589

@pbardea pbardea requested a review from jseldess March 8, 2018 21:13
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@cockroach-teamcity
Copy link
Member

@cockroach-teamcity
Copy link
Member

Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

Thanks, @pbardea! Just one request. Also, I think it'd be best for @bdarnell to review this as well.

@@ -4,7 +4,7 @@ summary: Perform a one-time-only initialization of a CockroachDB cluster.
toc: false
---

<span class="version-tag">New in v1.1:</span> This page explains the `cockroach init` [command](cockroach-commands.html), which you use to perform a one-time initialization of a new multi-node cluster. For a full walk-through of the cluster startup and initialization process, see one of the [Manual Deployment](manual-deployment.html) tutorials.
<span class="version-tag">New in v1.1:</span> This page explains the `cockroach init` [command](cockroach-commands.html), which you use to perform a one-time initialization of a new multi-node cluster. The target of the `cockroach init` command must appear in the `--join` flag of the [`cockroach start`](start-a-cluster.html) command which have been run on the other nodes in the cluster. For a full walk-through of the cluster startup and initialization process, see one of the [Manual Deployment](manual-deployment.html) tutorials.
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment for the v1.1 version of the page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which comment are you referring to on the v1.1 version of the page? I'm not seeing any Github comments on that page.

Also I've gone ahead and fixed TC if that is what it was referring to.

@cockroach-teamcity
Copy link
Member

@@ -4,7 +4,7 @@ summary: Perform a one-time-only initialization of a CockroachDB cluster.
toc: false
---

<span class="version-tag">New in v1.1:</span> This page explains the `cockroach init` [command](cockroach-commands.html), which you use to perform a one-time initialization of a new multi-node cluster. For a full walk-through of the cluster startup and initialization process, see one of the [Manual Deployment](manual-deployment.html) tutorials.
<span class="version-tag">New in v1.1:</span> This page explains the `cockroach init` [command](cockroach-commands.html), which you use to perform a one-time initialization of a new multi-node cluster. The target of the `cockroach init` command must appear in the `--join` flag of the [`cockroach start`](start-a-node.html) command which have been run on the other nodes in the cluster. For a full walk-through of the cluster startup and initialization process, see one of the [Manual Deployment](manual-deployment.html) tutorials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, I seem to have not submitted this comment before. Sorry.

Instead of mentioning this here, I'd suggest updating the --host flag description as follows:

The server host to connect to. This must appear in the --join flag of other nodes.

@bdarnell, does the target host address need to appear in the join flag of all or any of the other nodes?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's --advertise-host, not --host that must appear in --join and be used from client commands.

We recommend using the same --join arguments on all nodes, and while they may diverge over time they should at least be the same at init time.

Putting this message in the first paragraph of the command seems strange to me; I would put it in the docs for the --host flag. I don't think we need to make this warning too prominent since at the time that you're initializing the cluster you shouldn't have too many nodes or a complex configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm missing something, @bdarnell. --advertise-host isn't even an option on client commands like init... Or are you just saying that, when starting a cluster, if you use --advertise-host on nodes, that's what needs to be used in --join as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I got mixed up. I thought you were suggesting that the server-side --host flag should say that this is what should appear in the --join flag.

In retrospect, instead of --advertise-host and --host, we should have had --host and --bind-host. It's the advertised host that matters everywhere but on the node itself.

@pbardea
Copy link
Contributor Author

pbardea commented Mar 12, 2018

Moved the message from the top of the page to the description --host flag, as suggested.

@jseldess
Copy link
Contributor

Thanks, @pbardea. @bdarnell, I've opened a separate issue to update the description of cockroach start --join based on your comments: #2703

@cockroach-teamcity
Copy link
Member

1 similar comment
@cockroach-teamcity
Copy link
Member

@cockroach-teamcity
Copy link
Member

@jseldess jseldess merged commit f932889 into cockroachdb:master Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants