-
Notifications
You must be signed in to change notification settings - Fork 67
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
terraform, azure and utoronto: fixes and aligning files with terraform state #3578
terraform, azure and utoronto: fixes and aligning files with terraform state #3578
Conversation
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 looks good, one question about max number!
min : 1, | ||
max : 100, | ||
min : 0, | ||
max : 86, |
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 there a specific reason 86 was chosen?
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 updated those values to align with the current configuration, but I don't know the history on why they are configured to 0 - 86.
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.
@consideRatio ah, ok. Can you just add a comment about that here? Once that's done, this is good to go. Thanks for cleaning this up!
@@ -23,7 +23,7 @@ resource "azurerm_storage_share" "homes" { | |||
name = "homes" | |||
storage_account_name = azurerm_storage_account.homes.name | |||
quota = var.storage_size | |||
enabled_protocol = var.storage_protocol | |||
enabled_protocol = "NFS" |
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.
In particular, this is great! Let's rip out any other Azure File mounting related stuff we may have too (we use NFS for mounting that too now)
min : 1, | ||
max : 100, | ||
min : 0, | ||
max : 86, |
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.
@consideRatio ah, ok. Can you just add a comment about that here? Once that's done, this is good to go. Thanks for cleaning this up!
Thank you for reviewing and answering questions about this @yuvipanda. This is now applied, and terraform plan now reports |
Thanks for going through this in detail and getting all this set up before the upgrade, @consideRatio! |
This PR represents changes to terraform files that I found relevant to do in order to get utoronto's terraform state to be possible to apply with minimal changes.
terraform plan -var-file=projects/utoronto.tfvars
📚 Documentation preview 📚: https://2i2c-pilot-hubs--3578.org.readthedocs.build/en/3578/