Skip to content
This repository has been archived by the owner on Jul 26, 2021. It is now read-only.

Add Load Balancer Rule protocol support #22

Merged
merged 2 commits into from Nov 29, 2017
Merged

Add Load Balancer Rule protocol support #22

merged 2 commits into from Nov 29, 2017

Conversation

stuwil
Copy link
Contributor

@stuwil stuwil commented Nov 26, 2017

This adds support for the Load Balancer Rule protocol field, added in CloudStack 4.3. The primary use of this optional field is to specify a protocol of tcp-proxy, which enables PROXY protocol headers to be sent to the load balanced instance(s).

Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @stuwil! Unfortunately I can't verify that tcp-proxy is a valid input value. It's not in the docs... Can you point me to something that verifies this?

@@ -170,6 +182,11 @@ func resourceCloudStackLoadBalancerRuleRead(d *schema.ResourceData, meta interfa
d.Set("network_id", lb.Networkid)
}

// Only set protocol if user specified it to avoid spurious diffs
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed if you set Computed: true for the protocol field.

@stuwil
Copy link
Contributor Author

stuwil commented Nov 29, 2017

Thanks for the review!

Yes, unfortunately, the CloudStack API docs are pretty vague on it's usage. Here's the code reference for the "Internal Load Balancer" (haproxy) protocol offerings:

https://github.com/apache/cloudstack/blob/4.10/server/src/com/cloud/network/element/VirtualRouterElement.java#L616

https://github.com/apache/cloudstack/blob/330f24117cc5c90b85db291981652a2191417d5a/core/src/com/cloud/network/HAProxyConfigurator.java#L506

The load balancer protocol offerings do differ when using external load balancers (see this search for an example). It might make more sense in this case to be a bit less explicit in the docs... it's unfortunate there's no apparent good reference for the possible values though, given the LB variations. Let me know what you think and I can update accordingly.

@svanharmelen
Copy link
Member

Cool! In that case it's all good 👍 Thanks...

@svanharmelen svanharmelen merged commit 881b4ac into xanzy:master Nov 29, 2017
@stuwil stuwil deleted the load_balancer_protocol branch November 30, 2017 16:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants