From cbd6464b88a027b535c7ea553ac3f5868490d2e6 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 12:24:42 +0200 Subject: [PATCH 1/8] CLOUDSTACK-8947 - FW_EGRESS should be added only to filter table --- systemvm/patches/debian/config/opt/cloud/bin/configure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py index 55a4b942bd0..a8a7fc0679b 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py @@ -148,7 +148,7 @@ class CsAcl(CsDataBag): " -m %s " % rule['protocol'] + \ " --dport %s" % rnge - self.fw.append(["filter", "front", "%s -j %s" % (fwr, rule['action'])]) + self.fw.append(["filter", "", "%s -j %s" % (fwr, rule['action'])]) logging.debug("EGRESS rule configured for protocol ==> %s, action ==> %s", rule['protocol'], rule['action']) From 052c0dc4c92ee2f586daa600051f7365176f675f Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 12:28:48 +0200 Subject: [PATCH 2/8] CLOUDSTACK-8947 - Open the input chain to IP when loadbalancer is configured - Also remove the chain rule when it is removed. --- .../config/opt/cloud/bin/cs/CsLoadBalancer.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py index 4199d706fd1..4dce95fc804 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py @@ -27,7 +27,7 @@ HAPROXY_CONF_P = "/etc/haproxy/haproxy.cfg" class CsLoadBalancer(CsDataBag): - """ Manage dhcp entries """ + """ Manage Load Balance entries """ def process(self): if "config" not in self.dbag.keys(): @@ -44,3 +44,22 @@ class CsLoadBalancer(CsDataBag): file1.commit() shutil.copy2(HAPROXY_CONF_T, HAPROXY_CONF_P) CsHelper.service("haproxy", "restart") + + add_rules = self.dbag['config'][0]['add_rules'] + remove_rules = self.dbag['config'][0]['remove_rules'] + self._configure_firewall(add_rules, remove_rules) + + def _configure_firewall(self, add_rules, remove_rules): + firewall = self.fw + + for rules in add_rules: + path = rules.split(':') + ip = path[0] + port = path[1] + fw.append(["filter", "", "-A INPUT -p tcp -m tcp -d %s --dport %s -m state --state NEW -j ACCEPT" % (ip, port)]) + + for rules in remove_rules: + path = rules.split(':') + ip = path[0] + port = path[1] + fw.append(["filter", "", "-D INPUT -p tcp -m tcp -d %s --dport %s -m state --state NEW -j ACCEPT" % (ip, port)]) From 4a177031b055f3649e3b4a00c80eddb5cafa1dd7 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 15:50:58 +0200 Subject: [PATCH 3/8] CLOUDSTACK-8947 - Avoid multiple entries in the FW_EGRESS_RULES table --- systemvm/patches/debian/config/opt/cloud/bin/configure.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py index a8a7fc0679b..014e294bd38 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py @@ -141,13 +141,13 @@ class CsAcl(CsDataBag): " -m %s " % rule['protocol'] + " --icmp-type %s -j %s" % (icmp_type, self.rule['action'])]) else: - fwr = " -A FW_EGRESS_RULES" + \ - " -s %s " % cidr + fwr = " -A FW_EGRESS_RULES" if rule['protocol'] != "all": - fwr += "-p %s " % rule['protocol'] + \ + fwr += " -s %s " % cidr + \ + " -p %s " % cidr, rule['protocol'] + \ " -m %s " % rule['protocol'] + \ " --dport %s" % rnge - + self.fw.append(["filter", "", "%s -j %s" % (fwr, rule['action'])]) logging.debug("EGRESS rule configured for protocol ==> %s, action ==> %s", rule['protocol'], rule['action']) From 59bd935f3eb43d06580c737c69d5e5e46d481868 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 15:54:59 +0200 Subject: [PATCH 4/8] CLOUDSTACK-8947 - Configure the firewall when the load balancer is setup - Only restart HAproxy if it's not running yet --- .../config/opt/cloud/bin/cs/CsLoadBalancer.py | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py index 4dce95fc804..a288eac2c49 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsLoadBalancer.py @@ -19,6 +19,7 @@ import os.path import re import shutil from cs.CsDatabag import CsDataBag +from CsProcess import CsProcess from CsFile import CsFile import CsHelper @@ -27,7 +28,7 @@ HAPROXY_CONF_P = "/etc/haproxy/haproxy.cfg" class CsLoadBalancer(CsDataBag): - """ Manage Load Balance entries """ + """ Manage Load Balancer entries """ def process(self): if "config" not in self.dbag.keys(): @@ -43,23 +44,33 @@ class CsLoadBalancer(CsDataBag): if not file2.compare(file1): file1.commit() shutil.copy2(HAPROXY_CONF_T, HAPROXY_CONF_P) - CsHelper.service("haproxy", "restart") - + + proc = CsProcess(['/var/run/haproxy.pid']) + if not proc.find(): + logging.debug("CsLoadBalancer:: will restart HAproxy!") + CsHelper.service("haproxy", "restart") + else: + logging.debug("CsLoadBalancer:: will reload HAproxy!") + CsHelper.service("haproxy", "reload") + add_rules = self.dbag['config'][0]['add_rules'] remove_rules = self.dbag['config'][0]['remove_rules'] self._configure_firewall(add_rules, remove_rules) def _configure_firewall(self, add_rules, remove_rules): - firewall = self.fw - + firewall = self.config.get_fw() + + logging.debug("CsLoadBalancer:: configuring firewall. Add rules ==> %s" % add_rules) + logging.debug("CsLoadBalancer:: configuring firewall. Remove rules ==> %s" % remove_rules) + for rules in add_rules: path = rules.split(':') ip = path[0] port = path[1] - fw.append(["filter", "", "-A INPUT -p tcp -m tcp -d %s --dport %s -m state --state NEW -j ACCEPT" % (ip, port)]) + firewall.append(["filter", "", "-A INPUT -p tcp -m tcp -d %s --dport %s -m state --state NEW -j ACCEPT" % (ip, port)]) for rules in remove_rules: path = rules.split(':') ip = path[0] port = path[1] - fw.append(["filter", "", "-D INPUT -p tcp -m tcp -d %s --dport %s -m state --state NEW -j ACCEPT" % (ip, port)]) + firewall.append(["filter", "", "-D INPUT -p tcp -m tcp -d %s --dport %s -m state --state NEW -j ACCEPT" % (ip, port)]) From 80b51a7972e590c64a97650d7f53dc6431411007 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 15:58:29 +0200 Subject: [PATCH 5/8] CLOUDSTACK-8947 - Adding some logging to better understand what's happening with the rules --- systemvm/patches/debian/config/opt/cloud/bin/cs/CsNetfilter.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsNetfilter.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsNetfilter.py index 99c15018bba..4b5b49231f2 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsNetfilter.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsNetfilter.py @@ -150,6 +150,8 @@ class CsNetfilters(object): new_rule.set_table(fw[0]) if isinstance(fw[1], int): new_rule.set_count(fw[1]) + + logging.debug("Checking if the rule already exists: rule=%s table=%s chain=%s", new_rule.get_rule(), new_rule.get_table(), new_rule.get_chain()) if self.has_rule(new_rule): logging.debug("Exists: rule=%s table=%s", fw[2], new_rule.get_table()) else: From f35a16c19e8d8c851a250a7d713b30ba58dbf5a0 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 15:59:14 +0200 Subject: [PATCH 6/8] CLOUDSTACK-8947 - Adding some logging to better understand whay is happening with the Processes --- systemvm/patches/debian/config/opt/cloud/bin/cs/CsProcess.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsProcess.py b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsProcess.py index 19d030b3496..6155f3031d1 100755 --- a/systemvm/patches/debian/config/opt/cloud/bin/cs/CsProcess.py +++ b/systemvm/patches/debian/config/opt/cloud/bin/cs/CsProcess.py @@ -46,6 +46,8 @@ class CsProcess(object): matches = len([m for m in proc if m in self.search]) if matches == items: self.pid.append(re.split("\s+", i)[1]) + + logging.debug("CsProcess:: Searching for process ==> %s and found PIDs ==> %s", self.search, self.pid) return self.pid def find(self): From 384b6c7cd44b7a42390689a322027d1bcd83a31d Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 16:13:52 +0200 Subject: [PATCH 7/8] CLOUDSTACK-8947 - Fail fast! - If we canno SSH after 5 retries, it means it's not working. Do not wait for 60 attempts --- test/integration/smoke/test_loadbalance.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/smoke/test_loadbalance.py b/test/integration/smoke/test_loadbalance.py index 7da63ed7922..6e639de323e 100644 --- a/test/integration/smoke/test_loadbalance.py +++ b/test/integration/smoke/test_loadbalance.py @@ -133,7 +133,8 @@ class TestLoadBalance(cloudstackTestCase): ip_addr, self.services['lbrule']["publicport"], self.vm_1.username, - self.vm_1.password + self.vm_1.password, + retries=5 ) hostnames.append(ssh_1.execute("hostname")[0]) self.debug(hostnames) From bb3d1cde60f0bb37587da5116e487b13c8f3ad40 Mon Sep 17 00:00:00 2001 From: Wilder Rodrigues Date: Tue, 13 Oct 2015 16:45:46 +0200 Subject: [PATCH 8/8] CLOUDSTACK-8947 - Do not rely on the machine hostname to verify the test - The machine hostname might be different depending on the template. So do not rely on it. - Using the "uname" command instead. --- test/integration/smoke/test_loadbalance.py | 134 ++++++++++----------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/test/integration/smoke/test_loadbalance.py b/test/integration/smoke/test_loadbalance.py index 6e639de323e..2202e559a53 100644 --- a/test/integration/smoke/test_loadbalance.py +++ b/test/integration/smoke/test_loadbalance.py @@ -120,7 +120,7 @@ class TestLoadBalance(cloudstackTestCase): cleanup_resources(cls.apiclient, cls._cleanup) return - def try_ssh(self, ip_addr, hostnames): + def try_ssh(self, ip_addr, unameCmd): try: self.debug( "SSH into VM (IPaddress: %s) & NAT Rule (Public IP: %s)" % @@ -136,8 +136,8 @@ class TestLoadBalance(cloudstackTestCase): self.vm_1.password, retries=5 ) - hostnames.append(ssh_1.execute("hostname")[0]) - self.debug(hostnames) + unameCmd.append(ssh_1.execute("uname")[0]) + self.debug(unameCmd) except Exception as e: self.fail("%s: SSH failed for VM with IP Address: %s" % (e, ip_addr)) @@ -151,7 +151,7 @@ class TestLoadBalance(cloudstackTestCase): # Validate the Following: #1. listLoadBalancerRules should return the added rule #2. attempt to ssh twice on the load balanced IP - #3. verify using the hostname of the VM + #3. verify using the UNAME of the VM # that round robin is indeed happening as expected src_nat_ip_addrs = PublicIPAddress.list( self.apiclient, @@ -255,30 +255,30 @@ class TestLoadBalance(cloudstackTestCase): ) - hostnames = [] - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) + unameResults = [] + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) - self.debug("Hostnames: %s" % str(hostnames)) + self.debug("UNAME: %s" % str(unameResults)) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) self.assertIn( - self.vm_2.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server2" ) #SSH should pass till there is a last VM associated with LB rule lb_rule.remove(self.apiclient, [self.vm_2]) - # making hostnames list empty - hostnames[:] = [] + # making unameResultss list empty + unameResults[:] = [] try: self.debug("SSHing into IP address: %s after removing VM (ID: %s)" % @@ -287,10 +287,10 @@ class TestLoadBalance(cloudstackTestCase): self.vm_2.id )) - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) except Exception as e: @@ -301,7 +301,7 @@ class TestLoadBalance(cloudstackTestCase): with self.assertRaises(Exception): self.debug("Removed all VMs, trying to SSH") - self.try_ssh(src_nat_ip_addr.ipaddress, hostnames) + self.try_ssh(src_nat_ip_addr.ipaddress, unameResults) return @attr(tags = ["advanced", "advancedns", "smoke"], required_hardware="true") @@ -311,7 +311,7 @@ class TestLoadBalance(cloudstackTestCase): # Validate the Following: #1. listLoadBalancerRules should return the added rule #2. attempt to ssh twice on the load balanced IP - #3. verify using the hostname of the VM that + #3. verify using the UNAME of the VM that # round robin is indeed happening as expected #Create Load Balancer rule and assign VMs to rule @@ -372,22 +372,22 @@ class TestLoadBalance(cloudstackTestCase): "Check List Load Balancer instances Rules returns valid VM ID" ) try: - hostnames = [] - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) + unameResults = [] + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) - self.debug("Hostnames: %s" % str(hostnames)) + self.debug("UNAME: %s" % str(unameResults)) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) self.assertIn( - self.vm_2.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server2" ) @@ -399,15 +399,15 @@ class TestLoadBalance(cloudstackTestCase): self.vm_2.id )) # Making host list empty - hostnames[:] = [] + unameResults[:] = [] - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) - self.debug("Hostnames after removing VM2: %s" % str(hostnames)) + self.debug("UNAME after removing VM2: %s" % str(unameResults)) except Exception as e: self.fail("%s: SSH failed for VM with IP Address: %s" % (e, self.non_src_nat_ip.ipaddress.ipaddress)) @@ -419,7 +419,7 @@ class TestLoadBalance(cloudstackTestCase): self.non_src_nat_ip.ipaddress.ipaddress, self.vm_1.id )) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) return @attr(tags = ["advanced", "advancedns", "smoke"], required_hardware="true") @@ -467,29 +467,29 @@ class TestLoadBalance(cloudstackTestCase): ) lb_rule.assign(self.apiclient, [self.vm_1, self.vm_2]) - hostnames = [] - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) + unameResults = [] + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) - self.debug("Hostnames: %s" % str(hostnames)) + self.debug("UNAME: %s" % str(unameResults)) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) self.assertIn( - self.vm_2.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server2" ) #Removing VM and assigning another VM to LB rule lb_rule.remove(self.apiclient, [self.vm_2]) - # making hostnames list empty - hostnames[:] = [] + # making unameResults list empty + unameResults[:] = [] try: self.debug("SSHing again into IP address: %s with VM (ID: %s) added to LB rule" % @@ -497,11 +497,11 @@ class TestLoadBalance(cloudstackTestCase): self.non_src_nat_ip.ipaddress.ipaddress, self.vm_1.id, )) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) except Exception as e: @@ -510,22 +510,22 @@ class TestLoadBalance(cloudstackTestCase): lb_rule.assign(self.apiclient, [self.vm_3]) -# # Making hostnames list empty - hostnames[:] = [] - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, hostnames) - self.debug("Hostnames: %s" % str(hostnames)) +# # Making unameResults list empty + unameResults[:] = [] + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.try_ssh(self.non_src_nat_ip.ipaddress.ipaddress, unameResults) + self.debug("UNAME: %s" % str(unameResults)) self.assertIn( - self.vm_1.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server1" ) self.assertIn( - self.vm_3.name, - hostnames, + "Linux", + unameResults, "Check if ssh succeeded for server3" ) return