cli: Add connect command stub#60636
Conversation
|
This change is |
aaron-crl
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @itsbilal and @knz)
pkg/cli/flags.go, line 479 at r1 (raw file):
f := connectCmd.Flags() stringFlag(f, &baseCfg.SSLCertsDir, cliflags.CertsDir) stringFlag(f, &baseCfg.InitToken, cliflags.InitToken)
Do we need to add a join list here too?
itsbilal
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/cli/flags.go, line 479 at r1 (raw file):
Previously, aaron-crl wrote…
Do we need to add a join list here too?
Nope, that'd go into the args functional argument in the runConnect function itself, as it's a positional argument.
aaron-crl
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/cli/flags.go, line 479 at r1 (raw file):
Previously, itsbilal (Bilal Akhtar) wrote…
Nope, that'd go into the
argsfunctional argument in therunConnectfunction itself, as it's a positional argument.
I thought this was a flagged command for start (which was what I was emulating)?
Either way, 
b48d6ef to
91ba8a0
Compare
knz
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1, 1 of 1 files at r2.
Reviewable status:
pkg/cli/flags.go, line 479 at r1 (raw file):
Previously, aaron-crl wrote…
I thought this was a flagged command for
start(which was what I was emulating)?Either way, :lgtm: to get things rolling.
I'll be with aaron on this one and think it will make sense to share the relevant args with start, to ease the teaching of this.
(this will include --join but also, as I surmised yesterday, --listen-addr and --advertise-addr and the corresponding sql/http addrs later. But you can leave the addr part to me)
itsbilal
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/cli/flags.go, line 479 at r1 (raw file):
Previously, aaron-crl wrote…
I thought this was a flagged command for
start(which was what I was emulating)?Either way, :lgtm: to get things rolling.
It's a separate command altogether - not a part of start. But we can structure the underlying functions so they can also be called from runStart easily.
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Don't think this got auto-retried, so: bors r+ |
|
Already running a review |
91ba8a0 to
1487af5
Compare
|
Canceled. |
itsbilal
left a comment
There was a problem hiding this comment.
As this PR got held up by bors anyway, I decided to incorporate the feedback to standardize flags with start.
Reviewable status:
pkg/cli/flags.go, line 479 at r1 (raw file):
Previously, itsbilal (Bilal Akhtar) wrote…
It's a separate command altogether - not a part of
start. But we can structure the underlying functions so they can also be called from runStart easily.
Done. It's now a JoinList now, behind the same type of flags as start. Also added --listen-addr.
aaron-crl
left a comment
There was a problem hiding this comment.
Reviewable status:
…tion security: added helper functions to support automatic certificate generation This doesn't do anything yet but will be updated to trigger off: cockroachdb#60636 when it lands. Release note: None
Adds a new command stub, `connect`, with relevant args of `--certs-dir`, `--init-token`, `--listen-addr`, `--join` (same as start). Implementation code for this command is yet to come, and the command is not hooked up to the outer `cockroach` command yet. Very first part of cockroachdb#60632. Release note: None.
1487af5 to
2b4c93c
Compare
|
Canceled. |
|
bors r=aaron-crl |
|
Build failed (retrying...): |
|
Build succeeded: |
…tion security: added helper functions to support automatic certificate generation This doesn't do anything yet but will be updated to trigger off: cockroachdb#60636 when it lands. Release note: None
…tion security: added helper functions to support automatic certificate generation This doesn't do anything yet but will be updated to trigger off: cockroachdb#60636 when it lands. Release note: None
…tion server: added utility function for bundling init certs server: added init function that uses a recieved bundle to provision a node security: added helper functions to support automatic certificate generation security: added ClientCAKeyPath helper to align with ClientCACertPath This is part of cockroachdb#60632 and provides functions for cockroachdb#60636. Release note: None
…tion server: added utility function for bundling init certs server: added init function that uses a recieved bundle to provision a node security: added helper functions to support automatic certificate generation security: added ClientCAKeyPath helper to align with ClientCACertPath This is part of cockroachdb#60632 and provides functions for cockroachdb#60636. Release note: None
…tion server: added utility function for bundling init certs server: added init function that uses a recieved bundle to provision a node security: added helper functions to support automatic certificate generation security: added ClientCAKeyPath helper to align with ClientCACertPath This is part of cockroachdb#60632 and provides functions for cockroachdb#60636. Release note: None
60705: server: added initial cert utilities for automatic cert generation r=knz a=aaron-crl security: added helper functions to support automatic certificate generation This doesn't do anything yet but will be updated to trigger off: #60636 when it lands. This is part of the work supporting #60632 Release note: None Co-authored-by: Aaron Blum <aaron@cockroachlabs.com>
Adds a new command stub,
connect, with relevant args of--certs-dir,--init-token, and list of peers. Implementationcode for this command is yet to come, and the command is not hooked
up to the outer
cockroachcommand yet.Very first part of #60632.
Release note: None.
Epic: CRDB-6663