etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #887 from schubergphilis/vr_fixes_combined 

[BLOCKER] Combined PRs that fix VR issuesTonight I worked with @wilderrodrigues to figure out what is wrong with the virtual router. As we couldn't test single PRs any more (because of other issues with them causing tests to fail) we added all VR related PRs in a separate branch and started testing from there.

We combined the following PRs into this PR:
#836 #851 #867 #870 #881 #882 #842

After that, one issue remains: the VPC does not get a default gateway. Which is strange, because we already solved it in PR #738. When I look back, it was fixed again in PR #784. It could very well be that either one fixed one specific case, but also breaking the other. We need to investigate this, and make sure there will be a fix that works both for VPCs and VRs.

When we manually add the default gateway on the VPC, most tests pass and also spinning up two VPCs with one tier each, having a VM and them using s2s to VPN them together works fine. See for more details the report Wilder sent earlier.

Tomorrow we'll try to figure out how to fix the default gateway and merge this. Then we should have a base to work from again. Any PR that fixes another blocker, should at least then be rebased against the fixed master so we can run the tests against the PR branch. I'm not saying everything is fixed, I'm just saying that we can spin up a cloud that has working VMs.

When, in the mean time, someone has the time to checkout this branch and make the default route work for both VPC and VR that would be awesome. After that we should double check and verify the test results.

Pinging @karuturi to let her know the current status.

Regards,
Wilder / Remi

* pr/887:
  Fixing the index out of bounds error in the check_if_link_up() function
  small cleanups
  Fixing the defaut route for VPC routers
  Formatting the get_gateway() method in the CsDatabag.py file
  Fixing the dhcpsrvr iptables file
  Formatting the router_proxy.sh script
  CLOUDSTACK-8881: Fixed Static and PF configuration issue
  CLOUDSTACK-8905: Fixed hooking egress rules
  CLOUDSTACK-8891: Fixed default iptables rules on VR  for guest traffic
  Configured dnsmasq to listen on all interfaces so that vpn  client gets dns
  CLOUDSTACK-8864: Not able to add TCP port forwarding rule in VPN for specific ports
  CLOUDSTACK-8863: VM doesn't reconnect to internet post VR RESTART/STOP-START/RECREATE
  CLOUDSTACK-8843: Fixed issue in default iptables rules on shared network VR

Signed-off-by: Remi Bergsma <github@remi.nl>
This commit is contained in:
Remi Bergsma 2015-09-27 14:09:48 +02:00
parent 415631ab58 09e05f2a06
commit 3ded3e9000
11 changed files with 210 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
scripts/network/domr/router_proxy.sh
View File

@ -16,8 +16,6 @@
# specific language governing permissions and limitations
# under the License.
# used as a proxy to call script inside virtual router
#set -x
@ -47,12 +45,3 @@ check_gw "$domRIp"
ssh -p 3922 -q -o StrictHostKeyChecking=no -i $cert root@$domRIp "/opt/cloud/bin/$script $*"
exit $?

3
server/src/com/cloud/network/firewall/FirewallManagerImpl.java
View File

@ -426,7 +426,8 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService,
// we allow port forwarding rules with the same parameters but different protocols
boolean allowPf =
(rule.getPurpose() == Purpose.PortForwarding && newRule.getPurpose() == Purpose.PortForwarding && !newRule.getProtocol().equalsIgnoreCase(
rule.getProtocol()));
rule.getProtocol())) || (rule.getPurpose() == Purpose.Vpn && newRule.getPurpose() == Purpose.PortForwarding && !newRule.getProtocol().equalsIgnoreCase(
rule.getProtocol()));
boolean allowStaticNat =
(rule.getPurpose() == Purpose.StaticNat && newRule.getPurpose() == Purpose.StaticNat && !newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()));

83
server/test/com/cloud/network/firewall/FirewallManagerTest.java
View File

@ -22,20 +22,28 @@ import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.spy;
import java.util.ArrayList;
import java.util.List;
import javax.inject.Inject;
import com.cloud.exception.NetworkRuleConflictException;
import com.cloud.network.NetworkModel;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.vpc.VpcManager;
import com.cloud.user.AccountManager;
import com.cloud.user.DomainManager;
import junit.framework.Assert;
import org.apache.log4j.Logger;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.runners.MockitoJUnitRunner;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
@ -52,9 +60,9 @@ import com.cloud.network.rules.FirewallRule.Purpose;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.utils.component.ComponentContext;
@Ignore("Requires database to be set up")
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations = "classpath:/testContext.xml")
//@Ignore("Requires database to be set up")
@RunWith(MockitoJUnitRunner.class)
//@ContextConfiguration(locations = "classpath:/testContext.xml")
//@ComponentSetup(managerName="management-server", setupXml="network-mgr-component.xml")
public class FirewallManagerTest {
private static final Logger s_logger = Logger.getLogger(FirewallManagerTest.class);
@ -71,6 +79,7 @@ public class FirewallManagerTest {
// super.setUp();
// }
@Ignore("Requires database to be set up")
@Test
public void testInjected() {
@ -100,9 +109,30 @@ public class FirewallManagerTest {
}
@Inject
FirewallManager _firewallMgr;
@Mock
AccountManager _accountMgr;
@Mock
NetworkOrchestrationService _networkMgr;
@Mock
NetworkModel _networkModel;
@Mock
DomainManager _domainMgr;
@Mock
VpcManager _vpcMgr;
@Mock
IpAddressManager _ipAddrMgr;
@Mock
FirewallRulesDao _firewallDao;
@InjectMocks
FirewallManager _firewallMgr = new FirewallManagerImpl();
@Before
public void initMocks() {
MockitoAnnotations.initMocks(this);
}
@Ignore("Requires database to be set up")
@Test
public void testApplyRules() {
List<FirewallRuleVO> ruleList = new ArrayList<FirewallRuleVO>();
@ -123,6 +153,7 @@ public class FirewallManagerTest {
}
}
@Ignore("Requires database to be set up")
@Test
public void testApplyFWRules() {
List<FirewallRuleVO> ruleList = new ArrayList<FirewallRuleVO>();
@ -151,4 +182,38 @@ public class FirewallManagerTest {
}
}
@Test
public void testDetectRulesConflict() {
List<FirewallRuleVO> ruleList = new ArrayList<FirewallRuleVO>();
FirewallRuleVO rule1 = spy(new FirewallRuleVO("rule1", 3, 500, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null));
FirewallRuleVO rule2 = spy(new FirewallRuleVO("rule2", 3, 1701, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null));
FirewallRuleVO rule3 = spy(new FirewallRuleVO("rule3", 3, 4500, "UDP", 1, 2, 1, Purpose.Vpn, null, null, null, null));
ruleList.add(rule1);
ruleList.add(rule2);
ruleList.add(rule3);
FirewallManagerImpl firewallMgr = (FirewallManagerImpl)_firewallMgr;
when(firewallMgr._firewallDao.listByIpAndPurposeAndNotRevoked(3,null)).thenReturn(ruleList);
when(rule1.getId()).thenReturn(1L);
when(rule2.getId()).thenReturn(2L);
when(rule3.getId()).thenReturn(3L);
FirewallRule newRule1 = new FirewallRuleVO("newRule1", 3, 500, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule2 = new FirewallRuleVO("newRule2", 3, 1701, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null);
FirewallRule newRule3 = new FirewallRuleVO("newRule3", 3, 4500, "TCP", 1, 2, 1, Purpose.PortForwarding, null, null, null, null);
try {
firewallMgr.detectRulesConflict(newRule1);
firewallMgr.detectRulesConflict(newRule2);
firewallMgr.detectRulesConflict(newRule3);
}
catch (NetworkRuleConflictException ex) {
Assert.fail();
}
}
}

4
systemvm/patches/debian/config/etc/dnsmasq.conf.tmpl
View File

@ -87,7 +87,7 @@ local=/2.vmops-test.vmops.com/
# specified interfaces (and the loopback) give the name of the
# interface (eg eth0) here.
# Repeat the line for more than one interface.
interface=eth0
#interface=eth0
# Or you can specify which interface _not_ to listen on
except-interface=eth1
except-interface=eth2
@ -108,7 +108,7 @@ no-dhcp-interface=eth2
# want dnsmasq to really bind only the interfaces it is listening on,
# uncomment this option. About the only time you may need this is when
# running another nameserver on the same machine.
bind-interfaces
#bind-interfaces
# If you don't want dnsmasq to read /etc/hosts, uncomment the
# following line.

2
systemvm/patches/debian/config/etc/init.d/cloud-early-config
View File

@ -706,8 +706,6 @@ setup_dnsmasq() {
sed -i -e "s/^dhcp-range_ip6=.*$//" /etc/dnsmasq.conf
fi
sed -i -e "s/^[#]*listen-address=.*$/listen-address=$LOCAL_ADDRS/" /etc/dnsmasq.conf
if [ "$RROUTER" == "1" ]
then
DEFAULT_GW=$GUEST_GW

58
systemvm/patches/debian/config/etc/iptables/iptables-dhcpsrvr Normal file
View File

@ -0,0 +1,58 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
*nat
:PREROUTING ACCEPT [0:0]
:POSTROUTING ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
COMMIT
*filter
:INPUT DROP [0:0]
:FORWARD DROP [0:0]
:OUTPUT ACCEPT [0:0]
:FW_EGRESS_RULES - [0:0]
:FW_OUTBOUND - [0:0]
-A INPUT -d 224.0.0.18/32 -j ACCEPT
-A INPUT -d 225.0.0.50/32 -j ACCEPT
-A INPUT -i eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT
-A INPUT -i eth1 -m state --state RELATED,ESTABLISHED -j ACCEPT
-A INPUT -i eth2 -m state --state RELATED,ESTABLISHED -j ACCEPT
-A INPUT -p icmp -j ACCEPT
-A INPUT -i lo -j ACCEPT
-A INPUT -i eth0 -p udp -m udp --dport 67 -j ACCEPT
-A INPUT -i eth0 -p udp -m udp --dport 53 -j ACCEPT
-A INPUT -i eth0 -p tcp -m tcp --dport 53 -j ACCEPT
-A INPUT -i eth1 -p tcp -m tcp -m state --state NEW,ESTABLISHED --dport 3922 -j ACCEPT
-A INPUT -i eth0 -p tcp -m tcp -m state --state NEW --dport 80 -j ACCEPT
-A FORWARD -i eth0 -o eth1 -m state --state RELATED,ESTABLISHED -j ACCEPT
-A FORWARD -i eth2 -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT
-A FORWARD -i eth0 -o eth0 -m state --state NEW -j ACCEPT
-A FORWARD -i eth0 -o eth0 -m state --state RELATED,ESTABLISHED -j ACCEPT
-A FORWARD -i eth0 -o eth2 -j FW_OUTBOUND
-A FW_EGRESS_RULES -j ACCEPT
-I FW_OUTBOUND -m state --state RELATED,ESTABLISHED -j ACCEPT
-A FW_OUTBOUND -j FW_EGRESS_RULES
COMMIT
*mangle
:PREROUTING ACCEPT [0:0]
:INPUT ACCEPT [0:0]
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
:POSTROUTING ACCEPT [0:0]
-A PREROUTING -m state --state ESTABLISHED,RELATED -j CONNMARK --restore-mark
-A POSTROUTING -p udp -m udp --dport bootpc -j CHECKSUM --checksum-fill
COMMIT

40
systemvm/patches/debian/config/opt/cloud/bin/configure.py
View File

@ -124,6 +124,7 @@ class CsAcl(CsDataBag):
" -m %s " % rule['protocol'] +
" --dport %s -j RETURN" % rnge])
if self.direction == 'egress':
self.fw.append(["filter", "", " -A FW_OUTBOUND -j FIREWALL_EGRESS_RULES"])
if rule['protocol'] == "icmp":
self.fw.append(["filter", "front",
" -A FIREWALL_EGRESS_RULES" +
@ -662,6 +663,20 @@ class CsForwardingRules(CsDataBag):
elif rule["type"] == "staticnat":
self.processStaticNatRule(rule)
#return the VR guest interface ipo
def getGuestIp(self):
ipr = []
ipAddr = None
for ip in self.config.address().get_ips():
if ip.is_guest():
ipr.append(ip)
if len(ipr) > 0:
ipAddr = sorted(ipr)[-1]
if ipAddr:
return ipAddr.get_ip()
return None
def getDeviceByIp(self, ipa):
for ip in self.config.address().get_ips():
if ip.ip_in_subnet(ipa):
@ -725,7 +740,7 @@ class CsForwardingRules(CsDataBag):
)
fw4 = "-j SNAT --to-source %s -A POSTROUTING -s %s -d %s/32 -o %s -p %s -m %s --dport %s" % \
(
self.getGatewayByIp(rule['internal_ip']),
self.getGuestIp(),
self.getNetworkByIp(rule['internal_ip']),
rule['internal_ip'],
self.getDeviceByIp(rule['internal_ip']),
@ -809,6 +824,14 @@ class CsForwardingRules(CsDataBag):
"-A POSTROUTING -o %s -s %s/32 -j SNAT --to-source %s" % (device, rule["internal_ip"], rule["public_ip"])])
self.fw.append(["nat", "front",
"-A OUTPUT -d %s/32 -j DNAT --to-destination %s" % (rule["public_ip"], rule["internal_ip"])])
self.fw.append(["filter", "",
"-A FORWARD -i %s -o eth0 -d %s -m state --state NEW -j ACCEPT " % (device, rule["internal_ip"])])
#configure the hairpin nat
self.fw.append(["nat", "front",
"-A PREROUTING -d %s -i eth0 -j DNAT --to-destination %s" % (rule["public_ip"], rule["internal_ip"])])
self.fw.append(["nat", "front", "-A POSTROUTING -s %s -d %s -j SNAT -o eth0 --to-source %s" % (self.getNetworkByIp(rule['internal_ip']),rule["internal_ip"], self.getGuestIp())])
def main(argv):
@ -818,51 +841,66 @@ def main(argv):
format=config.get_format())
config.set_address()
logging.debug("Configuring ip addresses")
# IP configuration
config.address().compare()
config.address().process()
logging.debug("Configuring vmpassword")
password = CsPassword("vmpassword", config)
password.process()
logging.debug("Configuring vmdata")
metadata = CsVmMetadata('vmdata', config)
metadata.process()
logging.debug("Configuring networkacl")
acls = CsAcl('networkacl', config)
acls.process()
logging.debug("Configuring firewall rules")
acls = CsAcl('firewallrules', config)
acls.process()
logging.debug("Configuring PF rules")
fwd = CsForwardingRules("forwardingrules", config)
fwd.process()
red = CsRedundant(config)
red.set()
logging.debug("Configuring s2s vpn")
vpns = CsSite2SiteVpn("site2sitevpn", config)
vpns.process()
logging.debug("Configuring remote access vpn")
#remote access vpn
rvpn = CsRemoteAccessVpn("remoteaccessvpn", config)
rvpn.process()
logging.debug("Configuring vpn users list")
#remote access vpn users
vpnuser = CsVpnUser("vpnuserlist", config)
vpnuser.process()
logging.debug("Configuring dhcp entry")
dhcp = CsDhcp("dhcpentry", config)
dhcp.process()
logging.debug("Configuring load balancer")
lb = CsLoadBalancer("loadbalancer", config)
lb.process()
logging.debug("Configuring monitor service")
mon = CsMonitor("monitorservice", config)
mon.process()
logging.debug("Configuring iptables rules .....")
nf = CsNetfilters()
nf.compare(config.get_fw())
logging.debug("Configuring iptables rules done ...saving rules")
# Save iptables configuration - will be loaded on reboot by the iptables-restore that is configured on /etc/rc.local
CsHelper.save_iptables("iptables-save", "/etc/iptables/router_rules.v4")
CsHelper.save_iptables("ip6tables-save", "/etc/iptables/router_rules.v6")

34
systemvm/patches/debian/config/opt/cloud/bin/cs/CsAddress.py
View File

@ -97,23 +97,23 @@ class CsAddress(CsDataBag):
def check_if_link_exists(self,dev):
cmd="ip link show dev %s"%dev
result=CsHelper.execute(cmd)
if(len(result)!=0):
result = CsHelper.execute(cmd)
if(len(result) != 0):
return True
else:
return False
def check_if_link_up(self,dev):
cmd="ip link show dev %s | tr '\n' ' ' | cut -d ' ' -f 9"%dev
result=CsHelper.execute(cmd)
if(result[0].lower()=="up"):
result = CsHelper.execute(cmd)
if(result and result[0].lower() == "up"):
return True
else:
return False
def process(self):
route = CsRoute()
found_defaultroute = False
for dev in self.dbag:
if dev == "id":
@ -121,17 +121,12 @@ class CsAddress(CsDataBag):
ip = CsIP(dev, self.config)
for address in self.dbag[dev]:
if(address["nw_type"]!="public"):
continue
#check if link is up
if (not self.check_if_link_exists(dev)):
logging.info("link %s does not exist, so not processing"%dev)
continue
if not self.check_if_link_up(dev):
cmd="ip link set %s up"%dev
CsHelper.execute(cmd)
gateway = str(address["gateway"])
network = str(address["network"])
ip.setAddress(address)
@ -147,17 +142,25 @@ class CsAddress(CsDataBag):
"Address %s on device %s not configured", ip.ip(), dev)
if CsDevice(dev, self.config).waitfordevice():
ip.configure()
route.add_route(dev, network)
# The code looks redundant here, but we actually have to cater for routers and
# VPC routers in a different manner. Please do not remove this block otherwise
# The VPC default route will be broken.
if address["nw_type"] == "public" and not found_defaultroute:
if not route.defaultroute_exists():
if route.add_defaultroute(gateway):
found_defaultroute = True
# once we start processing public ip's we need to verify there
# is a default route and add if needed
if not route.defaultroute_exists():
cmdline=self.config.get_cmdline_instance()
cmdline = self.config.cmdline()
if(cmdline.get_gateway()):
route.add_defaultroute(cmdline.get_gateway())
class CsInterface:
""" Hold one single ip """
@ -516,9 +519,10 @@ class CsIP:
self.fw.append(["", "", "-A NETWORK_STATS -i eth2 -o eth0 -p tcp"])
self.fw.append(["", "", "-A NETWORK_STATS ! -i eth0 -o eth2 -p tcp"])
self.fw.append(["", "", "-A NETWORK_STATS -i eth2 ! -o eth0 -p tcp"])
self.fw.append(["filter", "", "-A INPUT -p icmp -j ACCEPT"])
self.fw.append(["filter", "", "-A INPUT -i eth0 -p tcp -m tcp --dport 3922 -m state --state NEW,ESTABLISHED -j ACCEPT"])
self.fw.append(["filter", "", "-P INPUT DROP"])
self.fw.append(["filter", "", "-P FORWARD DROP"])

1
systemvm/patches/debian/config/opt/cloud/bin/cs/CsDatabag.py
View File

@ -143,6 +143,7 @@ class CsCmdLine(CsDataBag):
md5 = hashlib.md5()
md5.update(passwd)
return md5.hexdigest()
def get_gateway(self):
if "gateway" in self.idata():
return self.idata()['gateway']

6
systemvm/patches/debian/config/opt/cloud/bin/cs/CsNetfilter.py
View File

@ -126,6 +126,7 @@ class CsNetfilters(object):
del_list = [x for x in self.rules if x.unseen()]
for r in del_list:
cmd = "iptables -t %s %s" % (r.get_table(), r.to_str(True))
logging.debug("unseen cmd: %s ", cmd)
CsHelper.execute(cmd)
# print "Delete rule %s from table %s" % (r.to_str(True), r.get_table())
logging.info("Delete rule %s from table %s", r.to_str(True), r.get_table())
@ -150,10 +151,10 @@ class CsNetfilters(object):
if isinstance(fw[1], int):
new_rule.set_count(fw[1])
if self.has_rule(new_rule):
logging.debug("rule %s exists in table %s", fw[2], new_rule.get_table())
logging.debug("Exists: rule=%s table=%s", fw[2], new_rule.get_table())
else:
# print "Add rule %s in table %s" % ( fw[2], new_rule.get_table())
logging.info("Add rule %s in table %s", fw[2], new_rule.get_table())
logging.info("Add: rule=%s table=%s", fw[2], new_rule.get_table())
# front means insert instead of append
cpy = fw[2]
if fw[1] == "front":
@ -185,6 +186,7 @@ class CsNetfilters(object):
if i.startswith('-A'): # Rule
self.del_rule(table, i.strip())
except IOError:
logging.debug("Exception in del_standard, returning")
# Nothing can be done
return

11
systemvm/patches/debian/config/opt/cloud/bin/vr_cfg.sh
View File

@ -93,4 +93,15 @@ done < $cfg
#remove the configuration file, log file should have all the records as well
rm -f $cfg
# Flush kernel conntrack table
log_it "VR config: Flushing conntrack table"
conntrackd -d 2> /dev/null
if [ $? -eq 0 ]; then
conntrackd -F
conntrackd -k
else
conntrackd -F
fi
log_it "VR config: Flushing conntrack table completed"
exit 0