Skip to content

cli: default "--join" flag port to 26257#65014

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rauchenstein:connect_port
May 12, 2021
Merged

cli: default "--join" flag port to 26257#65014
craig[bot] merged 1 commit intocockroachdb:masterfrom
rauchenstein:connect_port

Conversation

@rauchenstein
Copy link
Copy Markdown
Contributor

This behavior existed for "start --join" but not for "connect --join".
(Our code didn't set a default, so it defaulted to port 443 for https.)
The change is implemented at the level of flag parsing, so it is now
uniform for all commands using the join flag.

Fixes #61620.

Release note (bug fix): Hosts listed with the "connect --join" command
line flag now default to port 26257 (was 443). This matches the
existing behavior of "start --join".

@rauchenstein rauchenstein added the do-not-merge bors won't merge a PR with this label. label May 11, 2021
@rauchenstein rauchenstein requested a review from a team as a code owner May 11, 2021 20:06
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

:lgtm: thank you!

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rauchenstein)


pkg/cli/flags_test.go, line 909 at r1 (raw file):

		expectedJoin []string
	}{
		{[]string{"connect", "--join=a"}, []string{"a:" + base.DefaultPort}},

since you authored this, the name of the command has changed to be connect init (or alternatively connect join). Two strings. Hence the test failure in CI.

This behavior existed for "start --join" but not for "connect --join".
(Our code didn't set a default, so it defaulted to port 443 for https.)
The change is implemented at the level of flag parsing, so it is now
uniform for all commands using the join flag.

Fixes cockroachdb#61620.

Release note (bug fix): Hosts listed with the "connect --join" command
line flag now default to port 26257 (was 443).  This matches the
existing behavior of "start --join".
@rauchenstein rauchenstein removed the do-not-merge bors won't merge a PR with this label. label May 12, 2021
@rauchenstein rauchenstein changed the title default "--join" flag port to 26257 cli: default "--join" flag port to 26257 May 12, 2021
@rauchenstein
Copy link
Copy Markdown
Contributor Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented May 12, 2021

Build succeeded:

@craig craig bot merged commit bc1862d into cockroachdb:master May 12, 2021
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.

cli: cockroach connect mistakenly uses port 443 if the --join flag did not provide a port number

3 participants