roachprod: dismiss signup banner for nodes started by roachprod#56632
Conversation
dhartunian
left a comment
There was a problem hiding this comment.
can you shorten the first commit line so it fits in the PR description?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @dhartunian, @irfansharif, and @nkodali)
pkg/cmd/roachprod/install/cockroach.go, line 388 at r1 (raw file):
export ROACHPROD=%[3]d%[4]s; GOTRACEBACK=crash COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING=1 %[5]s \ COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true \
nit: not a blocker for me but is there any value to adding this var to whatever feeds h.getEnvVars() below? It gets inserted right into this spot: [5].
I don't know what the motivation is for putting other env vars in here directly or not.
47a4119 to
b9af963
Compare
Previously, when starting a node using roachprod, the db console ui would display a release notes signup banner. This banner is not useful for users of roachprod. We introduced a way to toggle this via env var in cockroachdb#56437. This commit sets that env var, COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true for `cockroach start*` commands issued from roachprod. Resolves: cockroachdb#46998 See also: cockroachdb#56437 Release note: none
b9af963 to
8611b71
Compare
nkodali
left a comment
There was a problem hiding this comment.
Shortened commit msg first line.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @irfansharif)
pkg/cmd/roachprod/install/cockroach.go, line 388 at r1 (raw file):
Previously, dhartunian (David Hartunian) wrote…
nit: not a blocker for me but is there any value to adding this var to whatever feeds
h.getEnvVars()below? It gets inserted right into this spot:[5].I don't know what the motivation is for putting other env vars in here directly or not.
Good catch. I initially thought h.getEnvVars() only got args from os.Environ to forward any env vars set on a roachprod command to the cockroach start command. But looks like it's also getting default args from main.nodeEnv via SyncedCluster.Env. Added this to main.nodeEnv.
dhartunian
left a comment
There was a problem hiding this comment.
just update the PR title/message to match commit :) that's one unfortunate consequence of editing commit messages in revisions.
Reviewed 2 of 2 files at r3.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @dhartunian)
|
bors r=dhartunian,irfansharif |
|
Build succeeded: |
Previously, when starting a node using roachprod, the db console
ui would display a release notes signup banner. This banner is not
useful for users of roachprod. We introduced a way to toggle this
via env var in #56437. This commit sets that
env var, COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true for
cockroach start*commands issued from roachprod.Resolves: #46998
See also: #56437
Release note: none