Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ENH: Added load balancing application in terraform #150

Merged
merged 9 commits into from
Jun 25, 2024
Merged

ENH: Added load balancing application in terraform #150

merged 9 commits into from
Jun 25, 2024

Conversation

max-amb
Copy link
Contributor

@max-amb max-amb commented Jun 21, 2024

Added a terraform script to create load balancing with multiple vm's

DavidFair
DavidFair reviewed Jun 21, 2024
View reviewed changes
Copy link
Collaborator

@DavidFair DavidFair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, I'm really excited to get this in since it's something we've long needed!

Just some nit picks around the edges mostly to make it safer and easier if people are trying it out

terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/vars.tfvars Show resolved Hide resolved
terraform_provisioning/load_balancing_application/vars.tfvars Show resolved Hide resolved
@max-amb max-amb requested a review from DavidFair June 21, 2024 10:46
meoflynn
meoflynn previously approved these changes Jun 21, 2024
View reviewed changes
khalford
khalford requested changes Jun 24, 2024
View reviewed changes
Copy link
Member

@khalford khalford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, we did not have the time to go through this work in detail and so I have left comments about some best practice things that you should change.

terraform_provisioning/README.md Outdated Show resolved Hide resolved
terraform_provisioning/README.md Outdated Show resolved Hide resolved
terraform_provisioning/private_network/private_network.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/vars.tfvars Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/variables.tf Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/variables.tf Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/vars.tfvars Outdated Show resolved Hide resolved
terraform_provisioning/load_balancing_application/load_balancing_application.tf Outdated Show resolved Hide resolved
@max-amb max-amb requested a review from khalford June 24, 2024 19:14
khalford
khalford requested changes Jun 25, 2024
View reviewed changes
Copy link
Member

@khalford khalford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, much more useable/readable now. Only one comment.

terraform_provisioning/load_balancing_application/variables.tf Outdated Show resolved Hide resolved
khalford
khalford approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@khalford khalford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DavidFair
Copy link
Collaborator

Thanks for your great work Max!

@DavidFair DavidFair merged commit d23281b into stfc:master Jun 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khalford khalford khalford approved these changes

@DavidFair DavidFair DavidFair approved these changes

@meoflynn meoflynn meoflynn left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@max-amb @DavidFair @meoflynn @khalford