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
cli,server: add --virtualized[-empty] flags to init #120813
Conversation
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
} | ||
|
||
initAttempt := func() error { | ||
const defaultApplicationClusterName = "application" |
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.
bikeshed: we can't use default
since it doesn't work as a bare keyword in places where we use a tenant name, but is there anything shorter we could use?
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.
main
?
pkg/server/server.go
Outdated
} | ||
return nil | ||
} | ||
return errors.Errorf("cluster initialization failed. cluster may need to be manually configured") |
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.
ultra minor nit: maybe a ;
or :
separator to avoid the question of capitalizing cluster
pkg/server/serverpb/init.proto
Outdated
} | ||
|
||
enum InitType { | ||
NO_INIT = 0; |
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.
nit: maybe make this DEFAULT
and then add NONE
as a separate option, even if the switch maps them both to the same thing for now?
ultra-nit: wise I'd leave "init" out of the value names, since it'll be in the prefix of the generated name.
96cf9b4
to
7d6e4b0
Compare
This adds the ability to specify that the cluster should be initialized as a virtualized cluster. Currently, this runs a one-time set of SQL initialization steps. One may rightly ask why we don't run the SQL directly from the `init` command. Running the SQL command would 1) Require that the user pass new custom-flags to init if they are using on-standard listen options, and 2) Technically may race with the server startup unless we waiting until the server was listening for connections. Fixes cockroachdb#120262 Release note (ops change): Cluster virtualization is now configured at cluster startup via flags (--virtualized or --virtualized-empty) passed to `cocroach init` rather than via the previously available --config-profile flag passed to `cockroach start`.
7d6e4b0
to
532c95d
Compare
Added a test. |
bors r=dt |
This adds the ability to specify that the cluster should be initialized as a virtualized cluster.
Currently, this runs a one-time set of SQL initialization steps.
One may rightly ask why we don't run the SQL directly from the
init
command. Running the SQL command wouldRequire that the user pass new custom-flags to init if they are
using on-standard listen options, and
Technically may race with the server startup unless we waiting
until the server was listening for connections.
Fixes #120262
Release note (ops change): Cluster virtualization is now configured at cluster startup via flags (--virtualized or --virtualized-empty) passed to
cocroach init
rather than via the previously available --config-profile flag passed tocockroach start
.