cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wid...@apache.org
Subject [03/50] [abbrv] git commit: updated refs/heads/kvm-vnc-listen to 895c51d
Date Mon, 16 Sep 2013 09:11:01 GMT
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.


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

Branch: refs/heads/kvm-vnc-listen
Commit: 28af817fcc835c10e4a21f08e971041325ad206d
Parents: 16a1f15
Author: Dave Cahill <dcahill@midokura.com>
Authored: Tue Aug 27 11:55:49 2013 -0700
Committer: Dave Cahill <dcahill@midokura.com>
Committed: Tue Sep 10 08:14:54 2013 +0000

----------------------------------------------------------------------
 plugins/network-elements/midonet/pom.xml        |   4 +-
 .../cloud/network/element/MidoNetElement.java   | 154 ++++++++++---------
 .../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(+), 104 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/28af817f/plugins/network-elements/midonet/pom.xml
----------------------------------------------------------------------
diff --git a/plugins/network-elements/midonet/pom.xml b/plugins/network-elements/midonet/pom.xml
index 9d6588b..19103e0 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.apache.cloudstack</groupId>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/28af817f/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 85814ef..20dbded 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
@@ -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());
-
-                // 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();
                         }
                     }
                 }
@@ -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;
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/28af817f/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/28af817f/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 891496a..2b7f4c9 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/28af817f/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/28af817f/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 9023c6e..ef91872 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