CLOUDSTACK-4466: Fix DHCP capability breaks in 4.2 for MidoNet

A recent code change in NetworkManager causes NullPointerExceptions when DHCP
capability list is null.

The commit which made the NetworkManager change also changed the VirtualRouter
to not use null for the capabilitylist, but didn't make this change for other
network devices, causing DHCP to fail on MidoNet.

This change also updates the MidoNet plugin to use the most recent MidoNet API.
This commit is contained in:
Dave Cahill 2013-08-27 11:55:49 -07:00
parent 16a1f150eb
commit 28af817fcc
6 changed files with 106 additions and 103 deletions

View File

@ -35,9 +35,9 @@
</repositories>
<dependencies>
<dependency>
<groupId>com.midokura</groupId>
<groupId>org.midonet</groupId>
<artifactId>midonet-client</artifactId>
<version>12.12.2</version>
<version>1.1.0</version>
</dependency>
<dependency>
<groupId>org.apache.cloudstack</groupId>

View File

@ -53,20 +53,21 @@ import com.cloud.vm.ReservationContext;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.dao.NicDao;
import com.midokura.midonet.client.MidonetApi;
import com.midokura.midonet.client.dto.DtoRule;
import com.midokura.midonet.client.resource.Bridge;
import com.midokura.midonet.client.resource.BridgePort;
import com.midokura.midonet.client.resource.DhcpHost;
import com.midokura.midonet.client.resource.DhcpSubnet;
import com.midokura.midonet.client.resource.Port;
import com.midokura.midonet.client.resource.ResourceCollection;
import com.midokura.midonet.client.resource.Route;
import com.midokura.midonet.client.resource.Router;
import com.midokura.midonet.client.resource.RouterPort;
import com.midokura.midonet.client.resource.Rule;
import com.midokura.midonet.client.resource.RuleChain;
import org.midonet.client.MidonetApi;
import org.midonet.client.exception.HttpInternalServerError;
import org.midonet.client.dto.DtoRule;
import org.midonet.client.dto.DtoRule.DtoRange;
import org.midonet.client.resource.Bridge;
import org.midonet.client.resource.BridgePort;
import org.midonet.client.resource.DhcpHost;
import org.midonet.client.resource.DhcpSubnet;
import org.midonet.client.resource.Port;
import org.midonet.client.resource.ResourceCollection;
import org.midonet.client.resource.Route;
import org.midonet.client.resource.Router;
import org.midonet.client.resource.RouterPort;
import org.midonet.client.resource.Rule;
import org.midonet.client.resource.RuleChain;
import com.sun.jersey.core.util.MultivaluedMapImpl;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@ -424,7 +425,9 @@ public class MidoNetElement extends AdapterBase implements
sub.subnetLength(cidrInfo.second());
sub.subnetPrefix(cidrInfo.first());
sub.defaultGateway(network.getGateway());
sub.dnsServerAddr(dest.getDataCenter().getDns1());
List<String> dcs = new ArrayList<String>();
dcs.add(dest.getDataCenter().getDns1());
sub.dnsServerAddrs(dcs);
sub.create();
}
@ -533,10 +536,8 @@ public class MidoNetElement extends AdapterBase implements
.nwDstAddress(floatingIp)
.nwDstLength(32)
.nwProto(SimpleFirewallRule.stringToProtocolNumber("icmp"))
.tpSrcStart(0)
.tpSrcEnd(0)
.tpDstStart(0)
.tpDstEnd(0)
.tpSrc(new DtoRange<Integer>(0,0))
.tpDst(new DtoRange<Integer>(0,0))
.position(2)
.create();
@ -689,52 +690,56 @@ public class MidoNetElement extends AdapterBase implements
}
for (FirewallRule rule : rulesToApply) {
IpAddress dstIp = _networkModel.getIp(rule.getSourceIpAddressId());
FirewallRuleTO ruleTO = new FirewallRuleTO(rule, null, dstIp.getAddress().addr());
if (rule.getState() == FirewallRule.State.Revoke || rule.getState() == FirewallRule.State.Add) {
IpAddress dstIp = _networkModel.getIp(rule.getSourceIpAddressId());
FirewallRuleTO ruleTO = new FirewallRuleTO(rule, null, dstIp.getAddress().addr());
// Convert to string representation
SimpleFirewallRule fwRule = new SimpleFirewallRule(ruleTO);
String[] ruleStrings = fwRule.toStringArray();
// Convert to string representation
SimpleFirewallRule fwRule = new SimpleFirewallRule(ruleTO);
String[] ruleStrings = fwRule.toStringArray();
if (rule.getState() == FirewallRule.State.Revoke) {
// Lookup in existingRules, delete if present
for(String revokeRuleString : ruleStrings){
Rule foundRule = existingRules.get(revokeRuleString);
if(foundRule != null){
foundRule.delete();
}
}
} else if (rule.getState() == FirewallRule.State.Add) {
// Lookup in existingRules, add if not present
for(int i = 0; i < ruleStrings.length; i++){
String ruleString = ruleStrings[i];
Rule foundRule = existingRules.get(ruleString);
if(foundRule == null){
// Get the cidr for the related entry in the Source Cidrs list
String relatedCidr = fwRule.sourceCidrs.get(i);
Pair<String,Integer> cidrParts = NetUtils.getCidr(relatedCidr);
// Create rule with correct proto, cidr, ACCEPT, dst IP
Rule toApply = preFilter.addRule()
.type(DtoRule.Jump)
.jumpChainId(preNat.getId())
.position(1)
.nwSrcAddress(cidrParts.first())
.nwSrcLength(cidrParts.second())
.nwDstAddress(ruleTO.getSrcIp())
.nwDstLength(32)
.nwProto(SimpleFirewallRule.stringToProtocolNumber(rule.getProtocol()));
if(rule.getProtocol().equals("icmp")){
// ICMP rules - reuse port fields
toApply.tpSrcStart(fwRule.icmpType).tpSrcEnd(fwRule.icmpType)
.tpDstStart(fwRule.icmpCode).tpDstEnd(fwRule.icmpCode);
} else {
toApply.tpDstStart(fwRule.dstPortStart)
.tpDstEnd(fwRule.dstPortEnd);
if (rule.getState() == FirewallRule.State.Revoke) {
// Lookup in existingRules, delete if present
for(String revokeRuleString : ruleStrings){
Rule foundRule = existingRules.get(revokeRuleString);
if(foundRule != null){
foundRule.delete();
}
}
} else if (rule.getState() == FirewallRule.State.Add) {
// Lookup in existingRules, add if not present
for(int i = 0; i < ruleStrings.length; i++){
String ruleString = ruleStrings[i];
Rule foundRule = existingRules.get(ruleString);
if(foundRule == null){
// Get the cidr for the related entry in the Source Cidrs list
String relatedCidr = fwRule.sourceCidrs.get(i);
Pair<String,Integer> cidrParts = NetUtils.getCidr(relatedCidr);
toApply.create();
// Create rule with correct proto, cidr, ACCEPT, dst IP
Rule toApply = preFilter.addRule()
.type(DtoRule.Jump)
.jumpChainId(preNat.getId())
.position(1)
.nwSrcAddress(cidrParts.first())
.nwSrcLength(cidrParts.second())
.nwDstAddress(ruleTO.getSrcIp())
.nwDstLength(32)
.nwProto(SimpleFirewallRule.stringToProtocolNumber(rule.getProtocol()));
if(rule.getProtocol().equals("icmp")){
// ICMP rules - reuse port fields
// (-1, -1) means "allow all ICMP", so we don't set tpSrc / tpDst
if(fwRule.icmpType != -1 | fwRule.icmpCode != -1){
toApply.tpSrc(new DtoRange(fwRule.icmpType, fwRule.icmpType))
.tpDst(new DtoRange(fwRule.icmpCode, fwRule.icmpCode));
}
} else {
toApply.tpDst(new DtoRange(fwRule.dstPortStart, fwRule.dstPortEnd));
}
toApply.create();
}
}
}
}
@ -973,8 +978,11 @@ public class MidoNetElement extends AdapterBase implements
// Rules in the preNat table
Map<String, Rule> existingPreNatRules = new HashMap<String, Rule>();
for (Rule existingRule : preNat.getRules()) {
String ruleString = new SimpleFirewallRule(existingRule).toStringArray()[0];
existingPreNatRules.put(ruleString, existingRule);
// The "port forwarding" rules we're interested in are dnat rules where src / dst ports are specified
if(existingRule.getType().equals(DtoRule.DNAT) && existingRule.getTpDst() != null){
String ruleString = new SimpleFirewallRule(existingRule).toStringArray()[0];
existingPreNatRules.put(ruleString, existingRule);
}
}
/*
@ -1058,8 +1066,7 @@ public class MidoNetElement extends AdapterBase implements
.flowAction(DtoRule.Accept)
.nwDstAddress(publicIp)
.nwDstLength(32)
.tpDstStart(pubPortStart)
.tpDstEnd(pubPortEnd)
.tpDst(new DtoRange(pubPortStart, pubPortEnd))
.natTargets(preTargets)
.nwProto(SimpleFirewallRule.stringToProtocolNumber(rule.getProtocol()))
.position(1);
@ -1123,7 +1130,8 @@ public class MidoNetElement extends AdapterBase implements
capabilities.put(Service.Gateway, null);
// L3 Support : DHCP
capabilities.put(Service.Dhcp, null);
Map<Capability, String> dhcpCapabilities = new HashMap<Capability, String>();
capabilities.put(Service.Dhcp, dhcpCapabilities);
// L3 Support : SourceNat
Map<Capability, String> sourceNatCapabilities = new HashMap<Capability, String>();
@ -1364,7 +1372,7 @@ public class MidoNetElement extends AdapterBase implements
int pos = 1;
// If it is ARP, accept it
egressChain.addRule().type(DtoRule.Accept)
.dlType((short)0x0806)
.dlType(0x0806)
.position(pos++)
.create();
@ -1423,7 +1431,7 @@ public class MidoNetElement extends AdapterBase implements
// If it is ARP, accept it
inc.addRule().type(DtoRule.Accept)
.dlType((short)0x0806)
.dlType(0x0806)
.position(pos++)
.create();
@ -1487,7 +1495,13 @@ public class MidoNetElement extends AdapterBase implements
String networkUUIDStr = String.valueOf(networkID);
netBridge = api.addBridge().tenantId(accountUuid).name(networkUUIDStr).create();
s_logger.debug("Attempting to create guest network bridge");
try {
netBridge = api.addBridge().tenantId(accountUuid).name(networkUUIDStr).create();
} catch (HttpInternalServerError ex) {
s_logger.warn("Bridge creation failed, retrying bridge get in case it now exists.", ex);
netBridge = getNetworkBridge(networkID, accountUuid);
}
}
return netBridge;
}

View File

@ -21,9 +21,10 @@ package com.cloud.network.element;
import com.cloud.agent.api.to.FirewallRuleTO;
import com.cloud.agent.api.to.NetworkACLTO;
import com.cloud.agent.api.to.PortForwardingRuleTO;
import com.midokura.midonet.client.dto.DtoRule;
import org.midonet.client.dto.DtoRule;
import org.midonet.client.resource.*;
import com.google.common.collect.*;
import com.midokura.midonet.client.resource.*;
import java.util.*;
// Used for translation between MidoNet firewall rules and
@ -147,8 +148,14 @@ public class SimpleFirewallRule {
dstIp = rule.getNwDstAddress();
if("icmp".equals(protocol)){
icmpType = rule.getTpSrcStart();
icmpCode = rule.getTpDstStart();
if(rule.getTpSrc() != null && rule.getTpDst() != null){
icmpType = rule.getTpSrc().start;
icmpCode = rule.getTpDst().start;
} else {
icmpType = -1;
icmpCode = -1;
}
} else {
/*
* If this is port forwarding, we want to take the start
@ -158,9 +165,9 @@ public class SimpleFirewallRule {
if (targets != null) {
dstPortStart = targets[0].portFrom;
} else {
dstPortStart = rule.getTpDstStart();
dstPortStart = rule.getTpDst().start;
}
dstPortEnd = rule.getTpDstEnd();
dstPortEnd = rule.getTpDst().end;
}
// cidr, protocol, dstIp, dstPortStart, dstPortEnd, icmpType, icmpCode);
@ -177,6 +184,7 @@ public class SimpleFirewallRule {
public int getFieldOne(){
if(protocol.equals("icmp")){
return icmpType;
} else {
return dstPortStart;
}

View File

@ -19,8 +19,6 @@
package com.cloud.network.guru;
import com.cloud.network.element.MidoNetElement;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenter.NetworkType;
import com.cloud.deploy.DeployDestination;
import com.cloud.deploy.DeploymentPlan;
@ -33,21 +31,12 @@ import com.cloud.user.Account;
import com.cloud.user.AccountVO;
import com.cloud.user.dao.AccountDao;
import com.cloud.vm.*;
import com.midokura.midonet.client.resource.Bridge;
import com.cloud.utils.net.NetUtils;
import com.cloud.network.Networks.AddressFormat;
import com.midokura.midonet.client.resource.BridgePort;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
import com.cloud.network.dao.NetworkVO;
import com.cloud.network.dao.PhysicalNetworkVO;
import com.cloud.vm.Nic.ReservationStrategy;
import javax.ejb.Local;
import java.util.UUID;
import javax.inject.Inject;
@Component

View File

@ -21,32 +21,24 @@ package com.cloud.network.resource;
import com.cloud.hypervisor.kvm.resource.*;
import com.cloud.agent.api.to.NicTO;
import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource;
import com.cloud.exception.InternalErrorException;
import com.cloud.network.Networks;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.net.NetUtils;
import com.cloud.utils.script.OutputInterpreter;
import com.cloud.utils.script.Script;
import com.midokura.midonet.client.resource.Bridge;
import com.midokura.midonet.client.resource.Router;
import com.midokura.midonet.client.resource.BridgePort;
import com.midokura.midonet.client.resource.RouterPort;
import com.midokura.midonet.client.resource.Host;
import org.apache.log4j.Logger;
import org.libvirt.LibvirtException;
import com.sun.jersey.core.util.MultivaluedMapImpl;
import com.midokura.midonet.client.MidonetApi;
import javax.ws.rs.core.MultivaluedMap;
import javax.naming.ConfigurationException;
import javax.ws.rs.core.MultivaluedMap;
import java.net.URI;
import java.util.Map;
import java.util.UUID;
import org.midonet.client.resource.Bridge;
import org.midonet.client.resource.BridgePort;
import org.midonet.client.resource.Host;
import org.midonet.client.MidonetApi;
public class MidoNetVifDriver extends VifDriverBase {
private static final Logger s_logger = Logger

View File

@ -24,8 +24,8 @@ import com.cloud.user.dao.AccountDao;
import junit.framework.TestCase;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.*;
import com.midokura.midonet.client.MidonetApi;
import com.midokura.midonet.client.resource.*;
import org.midonet.client.MidonetApi;
import org.midonet.client.resource.*;
import com.sun.jersey.core.util.MultivaluedMapImpl;
import com.cloud.network.*;
import com.cloud.vm.*;