-
Notifications
You must be signed in to change notification settings - Fork 887
chore: add terraform for spinning up load test cluster #7504
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
Conversation
a2d6b0e
to
dbcfc64
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.
some suggestions inline, including a lot of places where i think we can shorten names, but I don't feel extremely strongly about them.
I think at some point soon we're going to have to add support for licenses, but that doesn't need to block this PR.
scaletest/terraform/gcp_vpc.tf
Outdated
} | ||
|
||
resource "google_compute_subnetwork" "subnet" { | ||
name = "${var.name}-subnet" |
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.
name = "${var.name}-subnet" | |
name = var.name |
scaletest/terraform/gcp_vpc.tf
Outdated
@@ -0,0 +1,39 @@ | |||
resource "google_compute_network" "vpc" { | |||
project = var.project_id | |||
name = "${var.name}-vpc" |
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.
name = "${var.name}-vpc" | |
name = var.name |
-vpc
is redundant
scaletest/terraform/gcp_db.tf
Outdated
|
||
data "google_compute_global_address" "sql_peering" { | ||
name = "sql-ip-address" | ||
} |
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.
What is this used for? I don't see any references to it
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.
Nice spot, this is stale copy-pasta. Removed.
scaletest/terraform/gcp_db.tf
Outdated
} | ||
|
||
resource "google_sql_database_instance" "db" { | ||
name = "${var.name}-db" |
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.
name = "${var.name}-db" | |
name = var.name |
scaletest/terraform/gcp_db.tf
Outdated
|
||
database_flags { | ||
name = "max_connections" | ||
value = "500" |
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.
experience from v1 scale testing makes me think this should be a variable
scaletest/terraform/gcp_cluster.tf
Outdated
|
||
depends_on = [ | ||
google_container_cluster.primary | ||
] |
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.
Isn't this redundant given line 41?
scaletest/terraform/gcp_cluster.tf
Outdated
} | ||
|
||
resource "google_container_node_pool" "misc" { | ||
name = "${var.name}-node-pool-misc" |
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.
name = "${var.name}-node-pool-misc" | |
name = "${var.name}-misc" |
// These variables control the Coder deployment. | ||
variable "coder_replicas" { | ||
description = "Number of Coder replicas to provision" | ||
default = 1 |
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.
more than one basically requires Coder Enterprise license or tailnet coordination won't work
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.
Good point -- we could possibly use the trial license in this case as I don't see one of these instances being up for more than 30 days.
scaletest/terraform/coder.tf
Outdated
match_expressions { | ||
key = "app.kubernetes.io/name" | ||
operator = "In" | ||
values = ["coder-workspace"] |
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.
generally workspaces >> nodes, so I'm not sure this affinity term helps us
resources { | ||
requests = { | ||
"cpu" = "0.1" | ||
"memory" = "128Mi" |
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.
is this what we observe the agent
using in practice?
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.
Close enough at idle, yes.
Adds terraform configs for spinning up a load test cluster.