scripts: allow GCEWORKER_USER env to override ssh user #63645
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @cucaroach)
scripts/gceworker.sh, line 11 at r1 (raw file):
export CLOUDSDK_COMPUTE_ZONE=${GCEWORKER_ZONE-${CLOUDSDK_COMPUTE_ZONE-us-east1-b}} INSTANCE=${GCEWORKER_NAME-gceworker-$(id -un)} USER=${GCEWORKER_USER:-""}
should the default be $(id - un)? Or does empty USER work fine below (${USER}@${INSTANCE})?
|
scripts/gceworker.sh, line 11 at r1 (raw file): Previously, RaduBerinde wrote…
I thought about it but I didn't want to rock the boat in case id -un differs from what blank gives you, in particular I suspect on linux you can just set USER=blak and gcloud will pick that up and use it. I tested that specifying @instance with no user name works. Come to think of it I should probably not use USER as my variable name so that USER=blak gceworker.sh can work. I'll rename it to SSH_USER. |
Release note: None
cucaroach
force-pushed
the
gceworker-user-override
branch
from
658fe6e to
5885e94
Compare
There was a problem hiding this comment.
Reviewable status:
scripts/gceworker.sh, line 11 at r1 (raw file):
Previously, cucaroach (Tommy Reilly) wrote…
I thought about it but I didn't want to rock the boat in case id -un differs from what blank gives you, in particular I suspect on linux you can just set USER=blak and gcloud will pick that up and use it. I tested that specifying @instance with no user name works. Come to think of it I should probably not use USER as my variable name so that USER=blak gceworker.sh can work. I'll rename it to SSH_USER.
But then if you want USER to work, don't you want the default value to be $USER and not "" ?
There was a problem hiding this comment.
Reviewable status:
scripts/gceworker.sh, line 11 at r1 (raw file):
Previously, RaduBerinde wrote…
But then if you want USER to work, don't you want the default value to be
$USERand not""?
Oh, sorry, now I understand. If we pass in the empty one, gcloud would use $USER anyway. Still, may be less subtle if we just do that ourselves.
There was a problem hiding this comment.
Reviewable status:
|
scripts/gceworker.sh, line 11 at r1 (raw file): Previously, RaduBerinde wrote…
Exactly, I didn't want to trump whatever gcloud/ssh are doing by default. |
|
bors r=RaduBerinde |
|
Build succeeded: |
63743: sql: batch write event logs for grant/revoke r=ajwerner a=the-ericwang35 Helps with #41930. Previously, if we ran grant/revoke on multiple tables, we would create event logs for each table and write them one by one, resulting in round trips proportional to the number of tables. This patch addresses this by batch writing the event logs, so that 1 write to the event log table occurs regardless of the number of tables updated. Release note: None 63805: opt,sql: support streaming set ops r=rytaft a=rytaft This commit adds support for planning streaming set operations in the optimizer. Streaming set operations consist of a `UNION`, `INTERSECT`, or `EXCEPT` (including `ALL` variants) in which both inputs are ordered on the desired output ordering, and the set operation merges the inputs to maintain the ordering. The execution engine already supported this type of streaming set operation, so the purpose of this commit was to teach the optimizer about this capability and hook it up to the execution engine. This is also a prerequisite for #56201. Fixes #40797 Informs #56201 Release note (performance improvement): Set operations (`UNION`, `UNION ALL`, `INTERSECT`, `INTERSECT ALL`, `EXCEPT` and `EXCEPT ALL`) can now maintain ordering if both inputs are ordered on the desired output ordering. This can eliminate unnecessary sort operations and improve performance. 63930: scripts/gceworker.sh: always populate username r=ajwerner a=ajwerner In the previous change (#63645) to this file we were to allow injecting a username. The code before this commit, the code would populate the username as "" leading to a leading `@` in the address. The `ssh` family of commands are happy enough with this but the unison command is not. Before this change, we'd see: ``` Fatal error: ill-formed root specification ssh://@gceworker-ajwerner.us-east1-b.cockroach-workers/go/src/github.com/cockroachdb/cockroach ``` I'm content doing something different here like detecting whether we have a username injected instead. I don't have a strong reason to do that but maybe by explicitly making the user the output of `whoami` I'm preventing some other customization. Release note: None 64027: sql: disallow RBR transforms if transitioning indexes are incompatible r=arulajmani a=otan Release note (bug fix): Previously, if a DROP INDEX failed during a REGIONAL BY ROW transition, the index may be re-inserted back into the REGIONAL BY ROW table but would be invalid if it was sharded or partitioned. This is now rectified. 64076: sql: add statement_id to crdb_internal.node_statement_statistics r=arulajmani a=Azhng Currently, crdb_internal.node_transaction_statistics uses the statement_ids column to reference statements in crdb_internal.node_statement_statistics. However, the statement statistics view does not have column that shows statement id. This commit introduce a new statement_id column in the statement statistics view. Release note (sql change): crdb_internal.node_statement_statistics now stores statement_id. Closes: #64072 Co-authored-by: Eric Wang <[email protected]> Co-authored-by: Rebecca Taft <[email protected]> Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Azhng <[email protected]>
fixes #63641