cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From yas...@apache.org
Subject git commit: updated refs/heads/4.2-forward to a55c75b
Date Tue, 27 Aug 2013 19:08:11 GMT
Updated Branches:
  refs/heads/4.2-forward 47fa6d956 -> a55c75bbd


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.

Signed-off-by: Sheng Yang <sheng.yang@citrix.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/a55c75bb
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/a55c75bb
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/a55c75bb

Branch: refs/heads/4.2-forward
Commit: a55c75bbd201e53f0aca3e8f8f9dee08cd132a95
Parents: 47fa6d9
Author: Dave Cahill <dcahill@midokura.com>
Authored: Tue Aug 27 11:55:49 2013 -0700
Committer: Sheng Yang <sheng.yang@citrix.com>
Committed: Tue Aug 27 12:08:04 2013 -0700

----------------------------------------------------------------------
 plugins/network-elements/midonet/pom.xml        |   4 +-
 .../cloud/network/element/MidoNetElement.java   | 153 ++++++++++---------
 .../network/element/SimpleFirewallRule.java     |  20 ++-
 .../network/guru/MidoNetGuestNetworkGuru.java   |  11 --
 .../network/resource/MidoNetVifDriver.java      |  18 +--
 .../network/element/MidoNetElementTest.java     |   4 +-
 6 files changed, 107 insertions(+), 103 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a55c75bb/plugins/network-elements/midonet/pom.xml
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/pom.xml b/plugins/network-elements/midonet/pom.xml
index 69eeb70..c7dbdf2 100644
--- a/plugins/network-elements/midonet/pom.xml
+++ b/plugins/network-elements/midonet/pom.xml
@@ -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.mockito</groupId>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a55c75bb/plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java
b/plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java
index ab6a6de..42a6795 100644
--- a/plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java
+++ b/plugins/network-elements/midonet/src/com/cloud/network/element/MidoNetElement.java
@@ -54,19 +54,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.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -420,7 +422,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();
         }
@@ -529,10 +533,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();
 
@@ -685,52 +687,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());
-
-                // 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();
+                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();
+
+                    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()));
+                    } 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
+                                    // (-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));
+                                }
 
-                            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);
+                                toApply.create();
                             }
-
-                            toApply.create();
                         }
                     }
                 }
@@ -969,8 +975,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);
+            }
         }
 
         /*
@@ -1054,8 +1063,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);
@@ -1119,7 +1127,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>();
@@ -1360,7 +1369,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();
 
@@ -1419,7 +1428,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();
 
@@ -1483,7 +1492,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;
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a55c75bb/plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java
b/plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java
index a6b78d8..e8d81ef 100644
--- a/plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java
+++ b/plugins/network-elements/midonet/src/com/cloud/network/element/SimpleFirewallRule.java
@@ -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;
         }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a55c75bb/plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java
b/plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java
index d57affc..acc574e 100644
--- a/plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java
+++ b/plugins/network-elements/midonet/src/com/cloud/network/guru/MidoNetGuestNetworkGuru.java
@@ -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

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a55c75bb/plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java
b/plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java
index 3c7c23d..9c21636 100644
--- a/plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java
+++ b/plugins/network-elements/midonet/src/com/cloud/network/resource/MidoNetVifDriver.java
@@ -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

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a55c75bb/plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java
b/plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java
index a7d96b0..47b16b8 100644
--- a/plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java
+++ b/plugins/network-elements/midonet/test/com/cloud/network/element/MidoNetElementTest.java
@@ -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.*;


Mime
View raw message