-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
roachprod: introduce --skip-init to roachprod start
#53113
roachprod: introduce --skip-init to roachprod start
#53113
Conversation
0b66948
to
56d1154
Compare
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.
LGTM
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei, @irfansharif, and @nvanbenschoten)
pkg/cmd/roachprod/main.go, line 1046 at r1 (raw file):
The "init" command bootstraps the cluster (using "cockroach init"). It also sets default cluster settings. It's intended to be used in conjunction with 'roachprod start --skip-init'.
nit: spell out even more that roachprod start
by defaults inits
pkg/cmd/roachprod/install/cockroach.go, line 168 at r1 (raw file):
if shouldInit { fmt.Printf("%s: initializing cluster\n", h.c.Name) initOut, err := h.initializeCluster(nodeIdx)
can't we make this guy call Init()
?
56d1154
to
a09b9c1
Compare
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.
TFTR!
bors r+
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei and @nvanbenschoten)
pkg/cmd/roachprod/main.go, line 1046 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
nit: spell out even more that
roachprod start
by defaults inits
Done.
pkg/cmd/roachprod/install/cockroach.go, line 168 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
can't we make this guy call
Init()
?
Yea, but the workhorse is pretty much within initializeCluster
; everything else is error handling + printing output. And I didn't want to tie initialization and setting cluster settings into its own wrapper because reasons.
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
Huh, I thought we changed bors to go back to the old behavior. bors r+ |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
GitHub status checks took too long to complete, so bors is giving up. You can adjust bors configuration to have it wait longer if you like. |
👎 Rejected by PR status |
Example-ORM failures look unrelated, and seems like @taroface has been seeing them too. Re-running. Seems due to cockroachdb/examples-orms#116. |
..and `roachprod init`. I attempted to originally introduce this flag in \cockroachdb#51329, and ultimately abandoned it. I still think it's a good idea to have such a thing, especially given now we're writing integration tests that want to control `init` behaviour. It's much easier to write them with this --skip-init flag than it is to work around roachprod's magical auto-init behavior. To do what's skipped when using --skip-init, we introduce a `roachprod init` sub command. Release note: None
a09b9c1
to
b0fe435
Compare
bors r+ |
Build succeeded: |
+cc @knz this may be of passing interest to you (or maybe not). |
..and
roachprod init
. I attempted to originally introduce this flag in#51329, and ultimately abandoned it. I still think it's a good idea to
have such a thing, especially given now we're writing integration tests
that want to control
init
behaviour. It's much easier to write themwith this --skip-init flag than it is to work around roachprod's magical
auto-init behavior.
To do what's skipped when using --skip-init, we introduce a
roachprod init
sub command.Release note: None