Roachprod azure add machine #111926
Roachprod azure add machine #111926
Conversation
Epic: CC-25185 Release note: None
Epic: CC-25185 Release note: none
|
This change is |
smg260
requested review from
srosenberg and
DarrylWong
and removed request for
a team
smg260
force-pushed
the
roachprod_azure_add_machine
branch
from
cd825aa to
167b2a5
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @DarrylWong and @srosenberg)
pkg/roachprod/vm/azure/flags.go line 36 at r5 (raw file):
Previously, DarrylWong wrote…
random thought/nit: should we note why we use westus2? Personally I'd be pretty confused but maybe finding the commit message is good enough?
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 3 of 3 files at r2, 1 of 1 files at r3, 2 of 2 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status:
pkg/cmd/roachtest/tests/asyncpg.go line 85 at r7 (raw file):
ctx, t, c, node, "update apt-get", `sudo apt-get update`, ); err != nil { t.Fatal(err)
Curious -- did this fix an Azure specific failure?
pkg/roachprod/vm/azure/azure.go line 897 at r3 (raw file):
// The names for these are generated in the form Roachtest_<index>_Inbound genericInbound := []string{"8011", "8081", "9011", "9081-9102", "20011-20016", "27257", "27259-27280", "30258"}
Where were these port ranges obtained from? Probably a good idea to map them to services in comments if this was obtained manually after test runs, otherwise it will be hard to make sense of this list:
genericInbound := []string{
"8011", // nginx
"8081", // foo
}@herkolategan recently added a change to pick a dynamic open port for cockroach. We might have to be more generous with these port ranges with that in mind.
Meta question: do we need to configure the firewall at this level for Azure? AFAIK, we don't have similar configurations for the other clouds. Given that folks might be using roachprod outside of roachtest for their own experimentation, this could be overly restrictive too.
smg260
force-pushed
the
roachprod_azure_add_machine
branch
from
167b2a5 to
21b928b
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/tests/asyncpg.go line 85 at r7 (raw file):
Previously, renatolabs (Renato Costa) wrote…
Curious -- did this fix an Azure specific failure?
It did, as with installing go for a cdc. The repository metadata of the vms may be old / out of date. These updates are all over roachtests. At some point it would be nice to give a more structured way of preparing the environment.
pkg/roachprod/vm/azure/azure.go line 897 at r3 (raw file):
Previously, renatolabs (Renato Costa) wrote…
Where were these port ranges obtained from? Probably a good idea to map them to services in comments if this was obtained manually after test runs, otherwise it will be hard to make sense of this list:
genericInbound := []string{ "8011", // nginx "8081", // foo }@herkolategan recently added a change to pick a dynamic open port for cockroach. We might have to be more generous with these port ranges with that in mind.
Meta question: do we need to configure the firewall at this level for Azure? AFAIK, we don't have similar configurations for the other clouds. Given that folks might be using roachprod outside of roachtest for their own experimentation, this could be overly restrictive too.
When creating the vms, there are default deny all rules in place which are probably configurable, but these rules are only required for processes connecting to the VMs from the internet. This is really just building on what was already there.
The ports are from a number of roachtests, many related to multitenant, especially the 20000s which are dynamic tenant SQL ports. It would be cumbersome to map them all - much of it was parsed from error logs + ide search.
We probably don't need them and I don't think we restricting connections for GCE or AWS, but it never hurts to err on the side of caution. Either way, these rules are easily modified, and we can revisit removing the completely if it makes sense.
I've added an explanatory comment
| // The mapped roachtests are not exhaustive, and at some point will be | ||
| // cumbersome to keep adding exceptions for. We may need to consider removing | ||
| // all rules. | ||
| genericInbound := []string{ |
There was a problem hiding this comment.
workload has another range for prom metrics: 2112-2120 and 33333 for pprof :) (It's in pkg/workload/cli/run.go.)
This adds some more ports required for various roachtests and refactors their creation with a helper function. Epic: CC-25185 Release note: None
`westus` does not support availability zones, but `westus2` does. Over 20 roachtests use az. Epic: CC-25185 Release note: none
This is required so that packages can be found in azure. Epic: CC-25185 Release note: None
smg260
force-pushed
the
roachprod_azure_add_machine
branch
from
21b928b to
e7c247a
Compare
|
TFTR bors r=darrylwong,srosenberg |
|
bors r=darrylwong,srosenberg |
|
Build succeeded: |
This PR makes several azure specific changes to roachtest and roachprod to support more roachtests.
sseries machines forpremium/ultradiskswesttowest2for AZapt-get updatefor asyncpgEpic: CC-25185
Release note: none