-
Notifications
You must be signed in to change notification settings - Fork 6
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
massdriver-application/azure/scale-sets-docker #88
base: main
Are you sure you want to change the base?
Conversation
…into will/orc-160-azure-vm-tf-module
Marking as "Ready for review" without autoscaling or monitoring. Those should be easy adds, just trying to get a majority of this merged in. I've provisioned it a bunch with Bloby and not-bloby. This uses the latest |
data "azurerm_virtual_network" "main" { | ||
name = local.vnet_name | ||
resource_group_name = local.vnet_resource_group | ||
} | ||
|
||
data "azurerm_subnet" "default" { | ||
# does it have to be called default? | ||
name = "default" | ||
virtual_network_name = data.azurerm_virtual_network.main.name | ||
resource_group_name = data.azurerm_virtual_network.main.resource_group_name | ||
} |
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.
I don't think these are needed, because we're creating a subnet for the VMSS in subnet.tf
, and using auto-cidr to configure it.
What this seems to be doing is using the default
subnet coming from the VNet bundle and using it in ip_configurations
block within VMSS. I think what we want is to use the subnet that gets created in this deployment.
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.
Ahh I know that file subnet.tf
. We use it for the endpoint, we might be able to use it for the VM too.
for_each = var.dns.enable_dns ? [1] : [] | ||
content { | ||
name = var.name | ||
subnet_id = data.azurerm_subnet.default.id |
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.
subnet_id = data.azurerm_subnet.default.id | |
subnet_id = azurerm_subnet.main.id |
Same thing here
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.
Most of the complexity here is the networking. I'd like to follow up on this with another PR if we'd like to adjust.
I designed it this way so ingress comes through the VM network. We can draw all the lines and figure out all the networking if you want.
resource_group_name = azurerm_resource_group.main.name | ||
location = azurerm_resource_group.main.location | ||
disable_password_authentication = false | ||
admin_username = "placeholder" |
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.
I think we expose this field in postgres and mysql bundles to the customer to select their own password
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.
But we don't want people to log into the VM, we won't expose this to the user. 👍
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.
Looking really good so far! A few comments about some items that immediately popped out to me.
Azure VM runtime
Blocked by
TODO