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.

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

provider/azure: added load balancer support. #4343

Closed
wants to merge 1 commit into from

Conversation

aznashwan
Copy link
Contributor

No description provided.

@aznashwan
Copy link
Contributor Author

@jen20 @phinze this one, I feel, requires quite some attention 😄

@jen20
Copy link
Contributor

jen20 commented Dec 16, 2015

👍 Looks like a reasonably complex resource! Thanks for your work! Similarly to local network gateway, could you consider adding an acceptance test to verify basic functionality, and the resource documentation?

@phinze
Copy link
Contributor

phinze commented Dec 16, 2015

Wow yes this is definitely one of the more complicated back-end APIs we've had to convert into a resource! Looks solid at first glance - will wait for the acctests before I review more thoroughly.

In the meantime, I wanted to discuss a resource modeling question. I wonder if we should consider proactively splitting out multiple resources here, to allow, for example, an ELB to be defined in a module and load balancing rules to be attached elsewhere in config.

The config would become much more verbose in the short term, and down the road we could reintroduce nestable config via #2275 once that's done.

If we went down that road, azurerm_load_balancer would be reduced to just handling the presence of the top-level resource, and we'd gain:

  • azurerm_load_balancer_frontend_ip
  • azurerm_load_balancer_backend_address_pool
  • azurerm_load_balancer_rule
  • azurerm_load_balancer_probe
  • azurerm_load_balancer_nat_rule - could select inbound / outbound with an attribute

Each of these resources would have a load_balancer_id reference and interact with their respective subset of the deeply nested parameters. We'd need to add a per-LB mutex to guard against race conditions - there's an implementation of this in aws_security_group_rule we could use for that.

So there are obviously downsides to this way:

  • the config is immensely noisier than the nested route
  • the implementation is more complicated and difficult
  • it does not naturally have the ability to manage the full set of "sub-resources", so e.g. Terraform would not detect an extra manually added probe as drift

But on the flipside, this is the only way I know of to support configurations where the full set of the load balancer's details are not known in one spot in the config.

/cc @mitchellh - I'm basically using this as an example of a larger more abstract modelling question.

Thoughts are welcome! 😀

@iaingblack
Copy link

This looks great! Sorry for the usual "when will this be implemented" query though... But, i was very happy to see azurerm virtual machines are being supported now (#5514) but just realised I also need load balancer support in AzureRM to send traffic to various VMs in the resource group. Without that ability things get quite tricky with multiple VMs as each windows system created really potentially needs RDP, HTTP/HTTPS and WinRM/Powershell ports opened. Not sure how to handle that elegantly in terraform currently. Unless i'm missing something obvious? Trying hard to fit terraform into my azure workflow but it's not quite there. Terraform much easier to use than native ARM templates from Microsoft in my opinion, so trying to avoid them if I can!

Cheers!
Iain

@igable
Copy link
Contributor

igable commented Apr 25, 2016

@aznashwan do you happen to have a HCL example that works with this PR?

@buzztroll
Copy link
Contributor

After many hours of trying to massage the ARM API I agree that @phinze described approach is in the right direction. At the same time I think that drift is inevitably possible and that it will be difficult (if at all possible) to present the full combination of features possible with multiple ARM LB updates and maintain a good user experience. Would it make sense for a simple LB resource to be created that could either live along side of a more feature rich one or be enhanced over time to become a feature rich one?

@jen20
Copy link
Contributor

jen20 commented Oct 3, 2016

@stack72 and I have started working on this, basing it on some later work that was done. We'll open a new pull request later in the week when there is something to report. Thanks for all the work done here!

@jen20 jen20 closed this Oct 3, 2016
@jen20
Copy link
Contributor

jen20 commented Oct 3, 2016

The new pull request for this is #9199.

@ghost
Copy link

ghost commented Apr 21, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants