[New feature] Load balancer customization (haproxy-based) #4141

weizhouapache · 2020-06-10T19:54:57Z

Description

As discussed in mailing list, create this PR for haproxy config customization.

FS: https://cwiki.apache.org/confluence/display/CLOUDSTACK/VR+haproxy+customization+in+CloudStack

Fixes #3789

It supports (or will support)

Basic haproxy configurations:
haproxy statistics uri, auth, enable/disable
global maxconn and maxpipes
Basic configurations on rule
timeout connection, client ,server globally and per rule
http, httpalive per rule
maxconn, fullconn per rule
maxconn, minconn, maxqueue per server in rule
advanced features
transparent load balancer
SSL offloading
http2 support
Variable SSL configurations

TODO:

UI change on primate
unit tests

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

weizhouapache · 2020-07-21T07:55:03Z

@andrijapanicsb @svenvogel @rhtyd @DaanHoogland @onitake
This PR is ready for review and testing !

Any suggestions and comments are welcome !

onitake · 2020-07-21T08:11:20Z

@weizhouapache Can you provide test packages or should we build the PR ourselves?
I haven't done a custom CloudStack build in a while, I'm not sure if I even have the necessary build/packaging environment any more...

weizhouapache · 2020-07-21T08:40:10Z

@weizhouapache Can you provide test packages or should we build the PR ourselves?
I haven't done a custom CloudStack build in a while, I'm not sure if I even have the necessary build/packaging environment any more...

@onitake we can provide the packages. could you please tell me what os version you use ?

rohityadavcloud · 2020-07-27T09:01:56Z

@blueorangutan package

blueorangutan · 2020-07-27T09:03:46Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-07-27T09:56:01Z

Packaging result: ✖centos7 ✔debian. JID-1606

rohityadavcloud · 2020-08-05T08:26:26Z

@weizhouapache will this require a new systemvmtemplate?

weizhouapache · 2020-08-06T14:17:49Z

@weizhouapache will this require a new systemvmtemplate?

@rhtyd it requires haproxy 1.8+ for http2 support. As I remember haproxy 1.8 is installed in debian10 systemvm template. No other changes is needed in systemvm template

DaanHoogland · 2020-08-11T08:21:52Z

@blueorangutan test

blueorangutan · 2020-08-11T08:22:57Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2020-08-13T00:21:29Z

Trillian test result (tid-2382)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50655 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4141-t2382-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 80 look OK, 3 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_create_lb_rule_src_nat	`Error`	0.10	test_loadbalance.py
test_02_create_lb_rule_non_nat	`Error`	0.04	test_loadbalance.py
test_assign_and_removal_lb	`Error`	0.06	test_loadbalance.py
test_delete_account	`Error`	79.75	test_network.py
test_reboot_router	`Error`	179.46	test_network.py
test_releaseIP	`Error`	60.47	test_network.py
test_network_rules_acquired_public_ip_3_Load_Balancer_Rule	`Error`	3.23	test_network.py
test_01_lb_usage	`Error`	0.05	test_usage.py

rohityadavcloud · 2020-08-13T10:11:35Z

@weizhouapache can you look at failures? Thanks

rohityadavcloud · 2020-08-13T10:49:48Z

@blueorangutan package

blueorangutan · 2020-08-13T10:50:56Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

weizhouapache · 2020-08-13T11:07:12Z

@weizhouapache can you look at failures? Thanks

@rhtyd I will do

weizhouapache · 2020-08-13T16:51:35Z

@weizhouapache can you look at failures? Thanks

@rhtyd pushed a commit to fix it. can you re-kick off the tests ?

rohityadavcloud · 2020-08-14T07:39:27Z

@blueorangutan package

blueorangutan · 2020-08-14T07:39:56Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-08-14T08:50:31Z

Packaging result: ✔centos7 ✔debian. JID-1752

luhaijiao · 2020-08-24T03:05:20Z

It's a nice feature, is it possible to have it in 4.15 ?

rohityadavcloud · 2021-06-17T07:26:34Z

let's revisit this @weizhouapache with other VR agent/template improvements (if this is necessary or something new we should explore for ex https://wiki.nftables.org/wiki-nftables/index.php/Load_balancing)

sureshanaparti · 2021-06-29T12:15:57Z

@weizhouapache can you please fix the conflicts here. thanks.

weizhouapache · 2021-06-29T12:26:40Z

@blueorangutan package

blueorangutan · 2021-06-29T12:27:03Z

@weizhouapache a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-06-29T12:52:26Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 419

GutoVeronezi · 2021-06-29T16:21:37Z

api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java

+    }
+
+    public void setLbConfigs(List<? extends LoadBalancerConfig> lbConfigs) {
+        if (lbConfigs == null || lbConfigs.size() == 0) {


We could use org.apache.commons.lang3.ArrayUtils here:

... if (ArrayUtils.isEmpty(lbConfigs)) { ...

@GutoVeronezi org.apache.commons.lang3.ArrayUtils package don't have isEmpty method which accepts List object. I have to override it like:

... ArrayUtils.isEmpty(new List[]{lbConfigs}) ...

It imposes unnecessary conversion. What do you think?

GutoVeronezi · 2021-06-29T16:26:59Z

...in/java/org/apache/cloudstack/api/command/user/loadbalancer/AssignCertToLoadBalancerCmd.java

@@ -103,4 +110,32 @@ public Long getCertId() {
    public Long getLbRuleId() {
        return lbRuleId;
    }
+
+    public boolean isForced() {
+        return (forced != null) ? forced : false;


We could use org.apache.commons.lang3.BooleanUtils here:

... return BooleanUtils.toBoolean(forced); ...

GutoVeronezi · 2021-06-29T16:27:07Z

...in/java/org/apache/cloudstack/api/command/user/loadbalancer/AssignCertToLoadBalancerCmd.java

+        if (lb == null) {
+            return null;
+        }
+        return lb.getNetworkId();


We could implement a ternary here.

GutoVeronezi · 2021-06-29T16:27:25Z

...in/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerConfigCmd.java

+    }
+
+    public boolean isForced() {
+        return (forced != null) ? forced : false;


We could use org.apache.commons.lang3.BooleanUtils here:

... return BooleanUtils.toBoolean(forced); ...

GutoVeronezi · 2021-06-29T16:28:53Z

...in/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerConfigCmd.java

+        LoadBalancerConfig config = null;
+        try {
+            config = _entityMgr.findById(LoadBalancerConfig.class, getEntityId());


As config is not referenced outside the try block, we could declare it inside.

GutoVeronezi · 2021-06-29T19:05:16Z

server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java

+        if (lbMaxConn != null) {
+            loadBalancingData.append(",lb.maxconn=").append(lbMaxConn);
+        }
+        if (lbFullConn != null) {
+            loadBalancingData.append(",lb.fullconn=").append(lbFullConn);
+        }
+        if (lbTimeoutConnect != null) {
+            loadBalancingData.append(",lb.timeout.connect=").append(lbTimeoutConnect);
+        }
+        if (lbTimeoutServer != null) {
+            loadBalancingData.append(",lb.timeout.server=").append(lbTimeoutServer);
+        }
+        if (lbTimeoutClient != null) {
+            loadBalancingData.append(",lb.timeout.client=").append(lbTimeoutClient);
+        }
+        if (lbBackendHttps != null) {
+            loadBalancingData.append(",lb.backend.https=").append(lbBackendHttps);
+        }
+        if (lbHttp2 != null) {
+            loadBalancingData.append(",http2=").append(lbHttp2);
+        }
+        if (serverMaxconn != null) {
+            loadBalancingData.append(",server.maxconn=").append(serverMaxconn);
+        }
+        if (serverMinconn != null) {
+            loadBalancingData.append(",server.minconn=").append(serverMinconn);
+        }
+        if (serverMaxqueue != null) {
+            loadBalancingData.append(",server.maxqueue=").append(serverMaxqueue);
+        }


This code seems repeated, but changing parameters; We could extract it to a method.

GutoVeronezi · 2021-06-29T19:10:04Z

server/src/main/java/org/apache/cloudstack/network/lb/LoadBalancerConfigManagerImpl.java

+        if (id != null) {
+            sc.addAnd("id", SearchCriteria.Op.EQ, id);
+        }
+        if (scope != null) {
+            sc.addAnd("scope", SearchCriteria.Op.EQ, scope);
+        }
+        if (networkId != null) {
+            sc.addAnd("networkId", SearchCriteria.Op.EQ, networkId);
+        }
+        if (vpcId != null) {
+            sc.addAnd("vpcId", SearchCriteria.Op.EQ, vpcId);
+        }
+        if (loadBalancerId != null) {
+            sc.addAnd("loadBalancerId", SearchCriteria.Op.EQ, loadBalancerId);
+        }
+        if (name != null) {
+            sc.addAnd("name", SearchCriteria.Op.EQ, name);
+        }


This code seems repeated, but changing parameters; We could extract it to a method or create an addAndNotNull to SearchCriteria.

GutoVeronezi · 2021-06-29T19:14:42Z

server/src/main/java/org/apache/cloudstack/network/lb/LoadBalancerConfigManagerImpl.java

+            sc.addAnd("name", SearchCriteria.Op.EQ, name);
+        }
+        List<LoadBalancerConfigVO> configs = new ArrayList<LoadBalancerConfigVO>();
+        if (id != null || networkId != null || vpcId != null || loadBalancerId != null) {


We could use org.apache.commons.lang3.ObjectUtils here:

... if (ObjectUtils.anyNotNull(id, networkId... ...

GutoVeronezi · 2021-06-29T19:15:42Z

server/src/main/java/org/apache/cloudstack/network/lb/LoadBalancerConfigManagerImpl.java

+                }
+                throw new InvalidParameterValueException("networkId is required");
+            }
+            if (vpcId != null || loadBalancerId != null) {


We could use org.apache.commons.lang3.ObjectUtils here:

... ObjectUtils.anyNotNull(... ...

GutoVeronezi · 2021-06-29T19:17:00Z

server/src/main/java/org/apache/cloudstack/network/lb/LoadBalancerConfigManagerImpl.java

+        if (scope == Scope.Network) {
+            if (networkId == null) {
+                if (listAll) {
+                    return;
+                }
+                throw new InvalidParameterValueException("networkId is required");
+            }
+            if (vpcId != null || loadBalancerId != null) {
+                throw new InvalidParameterValueException("vpcId and loadBalancerId should be null if scope is Network");
+            }
+            if (networkId == null) {
+                throw new InvalidParameterValueException("networkId is required");
+            }
+            NetworkVO network = _networkDao.findById(networkId);
+            if (network == null) {
+                throw new InvalidParameterValueException("Cannot find network by id " + networkId);
+            }
+            // Perform permission check
+            _accountMgr.checkAccess(caller, null, true, network);
+            if (network.getVpcId() != null) {
+                throw new InvalidParameterValueException("network " + network.getName() + " is a VPC tier, please add LB configs to VPC instead");
+            }
+        } else if (scope == Scope.Vpc) {
+            if (vpcId == null) {
+                if (listAll) {
+                    return;
+                }
+                throw new InvalidParameterValueException("vpcId is required");
+            }
+            if (networkId != null || loadBalancerId != null) {
+                throw new InvalidParameterValueException("networkId and loadBalancerId should be null if scope is Vpc");
+            }
+            VpcVO vpc = _vpcDao.findById(vpcId);
+            if (vpc == null) {
+                throw new InvalidParameterValueException("Cannot find vpc by id " + vpcId);
+            }
+            // Perform permission check
+            _accountMgr.checkAccess(caller, null, true, vpc);
+        } else if (scope == Scope.LoadBalancerRule) {
+            if (loadBalancerId == null) {
+                if (listAll) {
+                    return;
+                }
+                throw new InvalidParameterValueException("loadBalancerId is required");
+            }
+            if (networkId != null || vpcId != null) {
+                throw new InvalidParameterValueException("networkId and vpcId should be null if scope is LoadBalancerRule");
+            }
+            LoadBalancerVO rule = _lbDao.findById(loadBalancerId);
+            if (rule == null) {
+                throw new InvalidParameterValueException("Cannot find load balancer rule by id " + loadBalancerId);
+            }
+            if (networkId != null) {
+                // Perform permission check
+                checkPermission(Scope.Network, rule.getNetworkId(), null, null);
+            }
+        }


This code seems repeated, but changing parameters; We could extract it to a method.

GutoVeronezi · 2021-06-29T19:54:07Z

There is a lot of code addition, but none unit testing or javadoc. IMHO we should add they.

weizhouapache · 2021-06-30T06:25:34Z

@ravening @soreana
could you have a look at @GutoVeronezi 's remarks ?

soreana · 2021-06-30T08:16:01Z

@weizhouapache Sure! I asked Neven to make a room for it.

…he-main

DaanHoogland · 2021-07-16T08:35:19Z

@soreana can you resolve that confict, please?

…nezi.

sureshanaparti · 2021-08-26T08:03:14Z

ping @soreana can you please fix the conflicting files.

rohityadavcloud · 2021-09-14T06:59:06Z

Ping @weizhouapache I think some more work may be needed on this as now Debian 11 has moved to haproxy 2.x, should this be targeted for next milestone. I worry it may not be stable within two weeks, ie current tentative RC date.

weizhouapache · 2021-09-14T07:15:50Z

Ping @weizhouapache I think some more work may be needed on this as now Debian 11 has moved to haproxy 2.x, should this be targeted for next milestone. I worry it may not be stable within two weeks, ie current tentative RC date.

@rhtyd it is ok to me.
@ravening @soreana what do you think ?

ravening · 2021-09-14T07:22:11Z

Ping @weizhouapache I think some more work may be needed on this as now Debian 11 has moved to haproxy 2.x, should this be targeted for next milestone. I worry it may not be stable within two weeks, ie current tentative RC date.

@rhtyd it is ok to me.
@ravening @soreana what do you think ?

@rhtyd @weizhouapache yes lets move it out of this milestone as this will not make it in time

soreana · 2021-09-14T07:26:44Z

@rhtyd @weizhouapache Sure let's do this.

ravening · 2021-12-21T15:41:43Z

@weizhouapache @rhtyd I have created a new pr for this in #5799

weizhouapache · 2021-12-21T16:25:25Z

@weizhouapache @rhtyd I have created a new pr for this in #5799

@ravening
great. closing this ticket then.

ustcweizhou force-pushed the 4.15-lbconfigs branch from 025d214 to 1447ac4 Compare July 20, 2020 19:29

weizhouapache closed this Jul 21, 2020

weizhouapache reopened this Jul 21, 2020

weizhouapache changed the title ~~[WIP] Load balancer customization (haproxy-based)~~ [New feature] Load balancer customization (haproxy-based) Jul 21, 2020

weizhouapache marked this pull request as ready for review July 21, 2020 07:51

ravening mentioned this pull request Jul 28, 2020

use X-forward-for address instead of the load balancer ip. #4209

Closed

DaanHoogland added component:virtual-router type:new-feature type:enhancement labels Jul 29, 2020

rohityadavcloud mentioned this pull request Aug 12, 2020

Niche feature configuration is always prominent in UI (SSL Offload) #3056

Closed

Merge remote-tracking branch 'apache/master' into 4.15-lbconfigs

0009b8d

Merge remote-tracking branch 'apache/main' into 4.15-lbconfigs

4125854

GutoVeronezi reviewed Jun 29, 2021

View reviewed changes

Sina Kashipazha added 2 commits July 12, 2021 12:28

Merge branch 'main' of https://github.com/apache/cloudstack into apac…

444d37a

…he-main

Merge branch 'apache-main' into 4.15-lbconfigs

317ecbb

Refactored the code, regarding the code change suggetions by GutoVero…

f5e5d0c

…nezi.

soreana force-pushed the 4.15-lbconfigs branch from 2ff1691 to f5e5d0c Compare July 21, 2021 14:50

weizhouapache modified the milestones: 4.16.0.0, 4.17.0.0 Sep 14, 2021

ravening mentioned this pull request Dec 21, 2021

HAproxy based load balancer customization #5799

Closed

12 tasks

weizhouapache closed this Dec 21, 2021

rohityadavcloud removed this from the 4.17.0.0 milestone Mar 24, 2022

GutoVeronezi mentioned this pull request Jun 21, 2022

Add new parameter to createLoadBalancerRule API #6460

Merged

12 tasks

CharlesQueiroz mentioned this pull request May 1, 2023

HAproxy based load balancer Customisation #7484

Open

7 tasks

[New feature] Load balancer customization (haproxy-based) #4141

[New feature] Load balancer customization (haproxy-based) #4141

Conversation

weizhouapache commented Jun 10, 2020 • edited by DaanHoogland Loading

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

weizhouapache commented Jul 21, 2020

onitake commented Jul 21, 2020

weizhouapache commented Jul 21, 2020 • edited Loading

rohityadavcloud commented Jul 27, 2020

blueorangutan commented Jul 27, 2020

blueorangutan commented Jul 27, 2020

rohityadavcloud commented Aug 5, 2020

weizhouapache commented Aug 6, 2020

DaanHoogland commented Aug 11, 2020

blueorangutan commented Aug 11, 2020

blueorangutan commented Aug 13, 2020

rohityadavcloud commented Aug 13, 2020

rohityadavcloud commented Aug 13, 2020

blueorangutan commented Aug 13, 2020

weizhouapache commented Aug 13, 2020

weizhouapache commented Aug 13, 2020

rohityadavcloud commented Aug 14, 2020

blueorangutan commented Aug 14, 2020

blueorangutan commented Aug 14, 2020

luhaijiao commented Aug 24, 2020

rohityadavcloud commented Jun 17, 2021

sureshanaparti commented Jun 29, 2021

weizhouapache commented Jun 29, 2021

blueorangutan commented Jun 29, 2021

blueorangutan commented Jun 29, 2021

Choose a reason for hiding this comment

soreana Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GutoVeronezi commented Jun 29, 2021

weizhouapache commented Jun 30, 2021

soreana commented Jun 30, 2021

DaanHoogland commented Jul 16, 2021

sureshanaparti commented Aug 26, 2021

rohityadavcloud commented Sep 14, 2021

weizhouapache commented Sep 14, 2021

ravening commented Sep 14, 2021

soreana commented Sep 14, 2021

ravening commented Dec 21, 2021

weizhouapache commented Dec 21, 2021

weizhouapache commented Jun 10, 2020 •

edited by DaanHoogland

Loading

weizhouapache commented Jul 21, 2020 •

edited

Loading

soreana Jul 12, 2021 •

edited

Loading