Disallowing udp for lb rules for haproxy #4501

davidjumani · 2020-11-24T05:57:10Z

Description

Since CloudStack uses HAproxy as a load balancer which doesn't support udp, adding checks to prevent udp from being passed as the protocol
haproxy/haproxy#62

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

How Has This Been Tested?

(localhost) 🐱 > create loadbalancerrule publicipid=afe66a42-2ebc-476b-bf5f-8954fcc42f32 protocol=udp publicport=80 privateport=80 algorithm=roundrobin name=test
🙈 Error: (HTTP 431, error code 9999) LB service provider cannot support this rule

davidjumani · 2020-11-24T08:06:36Z

@blueorangutan package

blueorangutan · 2020-11-24T08:07:22Z

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

blueorangutan · 2020-11-24T08:56:49Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2427

davidjumani · 2020-11-24T09:20:01Z

@blueorangutan test

blueorangutan · 2020-11-24T09:21:34Z

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

DaanHoogland

code lgtm

ravening · 2020-11-24T11:16:33Z

@davidjumani it does support udp

(local) 🐱 > create loadbalancerrule name=test privateport=80 publicport=8080 protocol=udp account=admin domainid=04dfd1c6-c28e-11ea-9a43-0617bc003377 networkid=d396de4e-0e23-4e3b-a19a-1fe1ac35227c publicipid=6275b645-fac5-4b03-a2f1-40848655ffd0 algorithm=roundrobin
{
  "loadbalancer": {
    "account": "admin",
    "algorithm": "roundrobin",
    "cidrlist": "",
    "domain": "ROOT",
    "domainid": "04dfd1c6-c28e-11ea-9a43-0617bc003377",
    "fordisplay": true,
    "id": "d1883751-9e55-4104-a522-c6c5c09a123b",
    "name": "test",
    "networkid": "d396de4e-0e23-4e3b-a19a-1fe1ac35227c",
    "privateport": "80",
    "protocol": "udp",
    "publicip": "10.10.20.1",
    "publicipid": "6275b645-fac5-4b03-a2f1-40848655ffd0",
    "publicport": "8080",
    "state": "Add",
    "tags": [],
    "zoneid": "8ae93aed-7db1-4a1a-9030-db0efac6ba62",
    "zonename": "mgt122-10"
  }
}

davidjumani · 2020-11-24T11:31:21Z

@ravening CloudStack allows a udp lb rule to be created but haproxy itself which is internally used for load balancing doesn't support udp load balancing
Also cloudstack treats all lb rules as tcp
https://github.com/apache/cloudstack/blob/master/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java#L340
https://github.com/apache/cloudstack/blob/master/core/src/main/java/com/cloud/network/HAProxyConfigurator.java#L48

ravening · 2020-11-24T11:35:13Z

@ravening CloudStack allows a udp lb rule to be created but haproxy itself which is internally used for load balancing doesn't support udp load balancing
Also cloudstack treats all lb rules as tcp
https://github.com/apache/cloudstack/blob/master/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java#L340
https://github.com/apache/cloudstack/blob/master/core/src/main/java/com/cloud/network/HAProxyConfigurator.java#L48

@davidjumani
yeah checked the haproxy docs and it doesnt support udp.
will there be a possibility where we can have different type of loadbalancers with udp support?

davidjumani · 2020-11-24T11:41:27Z

@davidjumani
yeah checked the haproxy docs and it doesnt support udp.
will there be a possibility where we can have different type of loadbalancers with udp support?

@ravening There are options such as nginx which can be explored later on if there's a need for udp load balancing (since no one noticed that it didn't work so far), so raising this PR so users won't be misled

DaanHoogland · 2020-11-24T14:15:14Z

let's link to #4481 here, so we have context when discussing implementation.

ravening · 2020-11-24T14:18:55Z

@davidjumani @DaanHoogland pr for haproxy support is here #4141

DaanHoogland · 2020-11-24T14:44:29Z

@ravening those are extra features for configuring/fine tuning the loadbalancer as it exists for tcp. the issue I linked is about missing support for udp and conflicts between udp and tcp definitions coexisting. (related but still unrelated)

blueorangutan · 2020-11-24T18:32:38Z

Trillian test result (tid-3239)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30945 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4501-t3239-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

rohityadavcloud · 2020-11-25T08:32:52Z

@weizhouapache since you've worked a lot of haproxy/lb, do you agree with this PR?

...main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java

weizhouapache · 2020-11-25T08:54:26Z

@davidjumani fyi,
In my pr for load balancer configurations #4141, there is a check on haproxy rule

https://github.com/apache/cloudstack/pull/4141/files#diff-1fea20ffdf01a98062ef48dfd3afa97d6fb0355f2f8c84acddc5115876f0188dR792-R806

line 792 to 806 in server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java

rohityadavcloud · 2020-11-25T10:04:10Z

That's a good point, some LB providers may actually support UDP but in most real world usecases would LB over tcp.

This reverts commit a5f4301.

weizhouapache · 2020-11-26T09:33:16Z

@weizhouapache Added the check under validateHAProxyLBRule

@davidjumani I will test it

weizhouapache

looks good.

create a udp lb rule, got exception below

2020-11-27 07:41:12,538 DEBUG [c.c.n.r.NetworkHelperImpl] (qtp1430439149-16:ctx-b16df490 ctx-85efa4e7) (logid:0c5425d3) Can't create LB rule as haproxy does not support udp
2020-11-27 07:41:12,539 WARN  [c.c.n.l.LoadBalancingRulesManagerImpl] (qtp1430439149-16:ctx-b16df490 ctx-85efa4e7) (logid:0c5425d3) Failed to create load balancer due to
com.cloud.exception.InvalidParameterValueException: LB service provider cannot support this rule

rohityadavcloud · 2020-11-27T08:53:16Z

@blueorangutan package

blueorangutan · 2020-11-27T08:55:03Z

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

blueorangutan · 2020-11-27T09:28:45Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2441

davidjumani · 2020-11-27T09:32:21Z

@blueorangutan test

blueorangutan · 2020-11-27T09:33:32Z

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

blueorangutan · 2020-11-27T19:22:37Z

Trillian test result (tid-3276)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33453 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4501-t3276-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_vpn.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.14	test_loadbalance.py
test_02_create_lb_rule_non_nat	`Error`	0.04	test_loadbalance.py
test_assign_and_removal_lb	`Error`	0.09	test_loadbalance.py
test_delete_account	`Error`	56.56	test_network.py
test_reboot_router	`Error`	172.78	test_network.py
test_releaseIP	`Error`	55.78	test_network.py
test_network_rules_acquired_public_ip_3_Load_Balancer_Rule	`Error`	3.31	test_network.py
test_01_lb_usage	`Error`	0.06	test_usage.py

davidjumani · 2020-11-30T05:33:19Z

@blueorangutan package

blueorangutan · 2020-11-30T05:33:43Z

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

blueorangutan · 2020-11-30T06:20:56Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2444

davidjumani · 2020-11-30T06:43:20Z

@blueorangutan test

blueorangutan · 2020-11-30T06:44:47Z

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

blueorangutan · 2020-12-01T01:34:29Z

Trillian test result (tid-3285)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 65709 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4501-t3285-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_routers.py
Intermittent failure detected: /marvin/tests/smoke/test_secondary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 62 look OK, 21 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestResetVmOnReboot>:setup	`Error`	0.00	test_reset_vm_on_reboot.py
ContextSuite context=TestRAMCPUResourceAccounting>:setup	`Error`	0.00	test_resource_accounting.py
ContextSuite context=TestRouterDHCPHosts>:setup	`Error`	0.00	test_router_dhcphosts.py
ContextSuite context=TestRouterDHCPOpts>:setup	`Error`	0.00	test_router_dhcphosts.py
ContextSuite context=TestRouterDns>:setup	`Error`	0.00	test_router_dns.py
ContextSuite context=TestRouterDnsService>:setup	`Error`	0.00	test_router_dnsservice.py
ContextSuite context=TestRouterIpTablesPolicies>:setup	`Error`	0.00	test_routers_iptables_default_policy.py
ContextSuite context=TestVPCIpTablesPolicies>:setup	`Error`	0.00	test_routers_iptables_default_policy.py
ContextSuite context=TestIsolatedNetworks>:setup	`Error`	0.00	test_routers_network_ops.py
ContextSuite context=TestRedundantIsolateNetworks>:setup	`Error`	0.00	test_routers_network_ops.py
ContextSuite context=TestRouterServices>:setup	`Error`	0.00	test_routers.py
test_01_sys_vm_start	`Failure`	0.15	test_secondary_storage.py
ContextSuite context=TestCpuCapServiceOfferings>:setup	`Error`	0.00	test_service_offerings.py
ContextSuite context=TestServiceOfferings>:setup	`Error`	0.22	test_service_offerings.py
ContextSuite context=TestSnapshotRootDisk>:setup	`Error`	0.00	test_snapshots.py
test_01_list_sec_storage_vm	`Failure`	0.06	test_ssvm.py
test_02_list_cpvm_vm	`Failure`	0.05	test_ssvm.py
test_03_ssvm_internals	`Failure`	0.06	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.05	test_ssvm.py
test_05_stop_ssvm	`Failure`	0.05	test_ssvm.py
test_06_stop_cpvm	`Failure`	0.05	test_ssvm.py
test_07_reboot_ssvm	`Failure`	0.05	test_ssvm.py
test_08_reboot_cpvm	`Failure`	0.05	test_ssvm.py
test_09_destroy_ssvm	`Failure`	0.05	test_ssvm.py
test_10_destroy_cpvm	`Failure`	0.06	test_ssvm.py
test_02_create_template_with_checksum_sha1	`Error`	65.62	test_templates.py
test_03_create_template_with_checksum_sha256	`Error`	65.63	test_templates.py
test_04_create_template_with_checksum_md5	`Error`	65.61	test_templates.py
test_05_create_template_with_no_checksum	`Error`	65.64	test_templates.py
test_02_deploy_vm_from_direct_download_template	`Error`	1.38	test_templates.py
test_03_deploy_vm_wrong_checksum	`Error`	1.42	test_templates.py
ContextSuite context=TestTemplates>:setup	`Error`	19.32	test_templates.py
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestLBRuleUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestNatRuleUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestPublicIPUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestSnapshotUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestVmUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestVolumeUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=TestVpnUsage>:setup	`Error`	0.00	test_usage.py
ContextSuite context=Test01DeployVM>:setup	`Error`	0.00	test_vm_life_cycle.py
ContextSuite context=Test02VMLifeCycle>:setup	`Error`	0.00	test_vm_life_cycle.py
test_14_secure_to_secure_vm_migration	`Error`	11.56	test_vm_life_cycle.py
test_15_secured_to_nonsecured_vm_migration	`Error`	74.12	test_vm_life_cycle.py
test_16_nonsecured_to_secured_vm_migration	`Error`	1.37	test_vm_life_cycle.py
ContextSuite context=TestVmSnapshot>:setup	`Error`	2.23	test_vm_snapshots.py
ContextSuite context=TestCreateVolume>:setup	`Error`	0.00	test_volumes.py
ContextSuite context=TestVolumes>:setup	`Error`	0.00	test_volumes.py
ContextSuite context=TestVPCRedundancy>:setup	`Error`	0.00	test_vpc_redundant.py
ContextSuite context=TestVPCNics>:setup	`Error`	0.00	test_vpc_router_nics.py
ContextSuite context=TestRVPCSite2SiteVpn>:setup	`Error`	0.00	test_vpc_vpn.py
ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup	`Error`	0.00	test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup	`Error`	0.00	test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:setup	`Error`	0.00	test_vpc_vpn.py
test_disable_oobm_ha_state_ineligible	`Error`	1516.19	test_hostha_kvm.py

davidjumani · 2020-12-01T04:22:05Z

@blueorangutan test

blueorangutan · 2020-12-01T04:22:52Z

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

davidjumani · 2020-12-01T04:37:27Z

@blueorangutan package

blueorangutan · 2020-12-01T04:38:47Z

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

blueorangutan · 2020-12-01T05:09:02Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2449

davidjumani · 2020-12-01T08:38:48Z

@blueorangutan test

blueorangutan · 2020-12-01T08:39:47Z

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

blueorangutan · 2020-12-01T18:18:43Z

Trillian test result (tid-3295)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32294 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4501-t3295-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

* master: (25 commits) integration test: skip vlan of public ip range in get_free_vlan vpc vr: plugin nics by this order: public/private/guest vpc vr: fix Conflicting device id on private gw nic Adding zone name to physicalnetworkresponse (apache#4510) Disallowing udp for lb rules for haproxy (apache#4501) Make global setting non-dynamic (apache#4505) Adding cpuallocated percentage and value to host and hostsformigrationresponse (apache#4499) kvm: fix router.aggregation.command.each.timeout is reset to 600 when update other kvm configs (apache#4496) fix failures with test_multiple_nic_support.py (apache#4495) Fix hosts for migration count (apache#4500) sql: Fix Zones are returned in a random order (apache#3934) (apache#4494) integration test: update steps integration test: add private gateway in test integration test: verify public nics state bugfix apache#9 vpc vr: Add PREROUTING rule for vm with static nat to multiple private gateways bugfix apache#8 vpc: add rule for traffic between vm and private gateway bugfix apache#7 vpc vr: allow servers in private gateway to reach internet via the VPC VR if it is gateway bugfix apache#6 vpc vr: Add iptables rules for ACL of private gateway Revert "Fix Policy Based Routing for private gateway static routes (apache#3604)" Revert "Add private gateway IP to router initialization config" ...

Disallowing udp for lb rules

a5f4301

This was referenced Nov 24, 2020

Disallowing udp as a lb protocol apache/cloudstack-primate#871

Closed

createLoadBalancerRule does not allow for tcp and udp #4481

Closed

davidjumani requested review from rohityadavcloud and DaanHoogland November 24, 2020 06:03

davidjumani changed the base branch from master to 4.14 November 24, 2020 08:04

davidjumani added the type:bug label Nov 24, 2020

DaanHoogland approved these changes Nov 24, 2020

View reviewed changes

rohityadavcloud requested a review from weizhouapache November 24, 2020 11:08

rohityadavcloud added this to the 4.14.1.0 milestone Nov 24, 2020

DaanHoogland linked an issue Nov 24, 2020 that may be closed by this pull request

createLoadBalancerRule does not allow for tcp and udp #4481

Closed

weizhouapache reviewed Nov 25, 2020

View reviewed changes

...main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java Outdated Show resolved Hide resolved

weizhouapache reviewed Nov 25, 2020

View reviewed changes

...main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java Outdated Show resolved Hide resolved

Revert "Disallowing udp for lb rules"

e23537b

This reverts commit a5f4301.

Adding check for udp rules for haproxy

9fee66d

davidjumani force-pushed the fix-lb-tcp branch from bb1d680 to 9fee66d Compare November 26, 2020 09:08

davidjumani changed the title ~~Disallowing udp for lb rules~~ Disallowing udp for lb rules for haproxy Nov 26, 2020

weizhouapache approved these changes Nov 27, 2020

View reviewed changes

Adding defensive check

7227685

rohityadavcloud approved these changes Dec 2, 2020

View reviewed changes

rohityadavcloud merged commit 7f5eb64 into apache:4.14 Dec 2, 2020

rajujith mentioned this pull request Sep 21, 2023

Kubernetes cluster LB dosen't support UDP port #7969

Closed

Disallowing udp for lb rules for haproxy #4501

Disallowing udp for lb rules for haproxy #4501

Conversation

davidjumani commented Nov 24, 2020 • edited

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

How Has This Been Tested?

davidjumani commented Nov 24, 2020

blueorangutan commented Nov 24, 2020

blueorangutan commented Nov 24, 2020

davidjumani commented Nov 24, 2020

blueorangutan commented Nov 24, 2020

DaanHoogland left a comment

Choose a reason for hiding this comment

ravening commented Nov 24, 2020

davidjumani commented Nov 24, 2020 • edited

ravening commented Nov 24, 2020

davidjumani commented Nov 24, 2020

DaanHoogland commented Nov 24, 2020

ravening commented Nov 24, 2020

DaanHoogland commented Nov 24, 2020

blueorangutan commented Nov 24, 2020

rohityadavcloud commented Nov 25, 2020

weizhouapache commented Nov 25, 2020

rohityadavcloud commented Nov 25, 2020

weizhouapache commented Nov 26, 2020

weizhouapache left a comment

Choose a reason for hiding this comment

rohityadavcloud commented Nov 27, 2020

blueorangutan commented Nov 27, 2020

blueorangutan commented Nov 27, 2020

davidjumani commented Nov 27, 2020

blueorangutan commented Nov 27, 2020

blueorangutan commented Nov 27, 2020

davidjumani commented Nov 30, 2020

blueorangutan commented Nov 30, 2020

blueorangutan commented Nov 30, 2020

davidjumani commented Nov 30, 2020

blueorangutan commented Nov 30, 2020

blueorangutan commented Dec 1, 2020

davidjumani commented Dec 1, 2020

blueorangutan commented Dec 1, 2020

davidjumani commented Dec 1, 2020

blueorangutan commented Dec 1, 2020

blueorangutan commented Dec 1, 2020

davidjumani commented Dec 1, 2020

blueorangutan commented Dec 1, 2020

blueorangutan commented Dec 1, 2020

davidjumani commented Nov 24, 2020 •

edited

davidjumani commented Nov 24, 2020 •

edited